Skip to content

Reduce reliance on the IMMUTABLE tnum bit in the plain list code #6707

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
fingolfin merged 5 commits into from
Oct 14, 2026
Merged

Reduce reliance on the IMMUTABLE tnum bit in the plain list code #6707

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
5fe09f8
kernel: use MUTABLE_TNUM to avoid some uses of IMMUTABLE
fingolfin Jun 27, 2025
f3312c5
kernel: let KTNumPlist & KTNumHomPlist return mutable tnum
fingolfin Jun 27, 2025
a724d4f
kernel: replace IS_PLIST_MUTABLE by IS_MUTABLE_OBJ
fingolfin Jun 27, 2025
16a3fc4
kernel: implement NEW_PLIST_IMM via MakeImmutableNoRecurse
fingolfin Jun 26, 2025
8c5b6d6
kernel: assert plain lists where IS_PLIST_MUTABLE did
fingolfin Oct 11, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions src/listfunc.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,8 @@ void AddPlist3(Obj list, Obj obj, Int pos)
{
UInt len;

if (!IS_PLIST_MUTABLE(list)) {
Comment thread
james-d-mitchell marked this conversation as resolved.
GAP_ASSERT(IS_PLIST(list));
if (!IS_MUTABLE_OBJ(list)) {
ErrorMayQuit("List Assignment: <list> must be a mutable list", 0, 0);
}
// in order to be optimistic when building list call assignment
Expand Down Expand Up @@ -165,7 +166,8 @@ static Obj RemPlist(Obj list)
Int pos;
Obj removed;

if ( ! IS_PLIST_MUTABLE(list) ) {
Comment thread
james-d-mitchell marked this conversation as resolved.
GAP_ASSERT(IS_PLIST(list));
if ( ! IS_MUTABLE_OBJ(list) ) {
ErrorMayQuit("Remove: <list> must be a mutable list", 0, 0);
}
pos = LEN_PLIST( list );
Expand Down
82 changes: 38 additions & 44 deletions src/plist.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,7 @@ static Obj TYPE_LIST_EMPTY_MUTABLE;
static Obj TYPE_LIST_EMPTY_IMMUTABLE;
static Obj TYPE_LIST_HOM;

static Obj TypePlistWithKTNum( Obj list, UInt *ktnum );
static Obj TypePlistWithKTNum(Obj list, UInt * ktnum);

static Int KTNumPlist(Obj list, Obj * famfirst)
{
Expand All @@ -229,9 +229,11 @@ static Int KTNumPlist(Obj list, Obj * famfirst)

Obj loopTypeObj = 0; // typeObj in loop

GAP_ASSERT(IS_PLIST(list));

#ifdef HPCGAP
if (!CheckWriteAccess(list)) {
return TNUM_OBJ(list);
return MUTABLE_TNUM(TNUM_OBJ(list));
}
#endif
// if list has `OBJ_FLAG_TESTING' keep that
Expand All @@ -245,8 +247,8 @@ static Int KTNumPlist(Obj list, Obj * famfirst)

// special case for empty list
if ( lenList == 0 ) {
res = IS_MUTABLE_OBJ(list) ? T_PLIST_EMPTY : T_PLIST_EMPTY+IMMUTABLE;
RetypeBagIfWritable(list, res);
res = T_PLIST_EMPTY;
RetypeBagSM(list, res);
if (famfirst != (Obj *) 0)
*famfirst = (Obj) 0;
return res;
Expand All @@ -266,7 +268,7 @@ static Int KTNumPlist(Obj list, Obj * famfirst)
#endif
else if (TEST_OBJ_FLAG(elm, OBJ_FLAG_TESTING)) {
isHom = FALSE;
areMut = IS_PLIST_MUTABLE(elm);
areMut = IS_MUTABLE_OBJ(elm);

@james-d-mitchell james-d-mitchell Oct 10, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same comment as before, maybe add an assertion at the start of this function?

@fingolfin fingolfin Oct 12, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

agreed and changed in all instances.

isTable = FALSE;
}
else {
Expand Down Expand Up @@ -331,7 +333,7 @@ static Int KTNumPlist(Obj list, Obj * famfirst)
#endif
else if (TEST_OBJ_FLAG(elm, OBJ_FLAG_TESTING)) {
isHom = FALSE;
areMut = (areMut || IS_PLIST_MUTABLE(elm));
areMut = (areMut || IS_MUTABLE_OBJ(elm));
isTable = FALSE;
isRect = FALSE;
}
Expand Down Expand Up @@ -432,7 +434,6 @@ static Int KTNumPlist(Obj list, Obj * famfirst)
SET_FILT_LIST( list, areMut ? FN_IS_DENSE : FN_IS_RECT );
res = T_PLIST_TAB_RECT;
}
res = res + ( IS_MUTABLE_OBJ(list) ? 0 : IMMUTABLE );
return res;
}

Expand All @@ -451,7 +452,7 @@ static Int KTNumHomPlist(Obj list)

#ifdef HPCGAP
if (!CheckWriteAccess(list)) {
return TNUM_OBJ(list);
return MUTABLE_TNUM(TNUM_OBJ(list));
}
#endif

Expand Down Expand Up @@ -560,13 +561,12 @@ static Int KTNumHomPlist(Obj list)
res = T_PLIST_HOM;

finish:
res = res + ( IS_MUTABLE_OBJ(list) ? 0 : IMMUTABLE );
return res;
}

static Obj TypePlist(Obj list)
{
return TypePlistWithKTNum( list, (UInt *) 0);
return TypePlistWithKTNum(list, 0);
}

static Obj TypePlistNDense(Obj list)
Expand Down Expand Up @@ -613,7 +613,10 @@ static Obj TypePlistEmpty(Obj list)

static Obj TypePlistHomHelper(Obj family, UInt tnum, UInt knr, Obj list)
{
GAP_ASSERT(knr <= tnum);
// make sure tnum reflects mutability
if (!IS_MUTABLE_OBJ(list))
tnum |= IMMUTABLE;

knr = tnum - knr + 1;

// get the list types of that family
Expand Down Expand Up @@ -656,7 +659,7 @@ static Obj TypePlistWithKTNum (
tnum = KTNumPlist( list, &family);
CLEAR_OBJ_FLAG( list, OBJ_FLAG_TESTING );
} else {
tnum = TNUM_OBJ(list);
tnum = MUTABLE_TNUM(TNUM_OBJ(list));
family = 0;
}
#else
Expand All @@ -669,24 +672,18 @@ static Obj TypePlistWithKTNum (
*ktnum = tnum;

// handle special cases
switch (tnum)
{
case T_PLIST_NDENSE:
case T_PLIST_NDENSE+IMMUTABLE:
switch (tnum) {
case T_PLIST_NDENSE:
return TypePlistNDense(list);
case T_PLIST_DENSE_NHOM:
case T_PLIST_DENSE_NHOM+IMMUTABLE:
case T_PLIST_DENSE_NHOM:
return TypePlistDenseNHom(list);
case T_PLIST_DENSE_NHOM_SSORT:
case T_PLIST_DENSE_NHOM_SSORT+IMMUTABLE:
case T_PLIST_DENSE_NHOM_SSORT:
return TypePlistDenseNHomSSort(list);
case T_PLIST_DENSE_NHOM_NSORT:
case T_PLIST_DENSE_NHOM_NSORT+IMMUTABLE:
case T_PLIST_DENSE_NHOM_NSORT:
return TypePlistDenseNHomNSort(list);
case T_PLIST_EMPTY:
case T_PLIST_EMPTY+IMMUTABLE:
case T_PLIST_EMPTY:
return TypePlistEmpty(list);
default: ; // fall through into the rest of the function
default: ; // fall through into the rest of the function
}

// handle homogeneous list
Expand Down Expand Up @@ -759,20 +756,14 @@ static Obj TypePlistFfe(Obj list)
** 'ShallowCopyPlist' only copies up to the logical length, the result is
** always a mutable list.
*/
Obj ShallowCopyPlist (
Obj list )
Obj ShallowCopyPlist(Obj list)
{
Obj new;
UInt len;

// make the new object and copy the contents
len = LEN_PLIST(list);
if ( ! IS_PLIST_MUTABLE(list) ) {
new = NEW_PLIST( TNUM_OBJ(list) - IMMUTABLE, len );
}
else {
new = NEW_PLIST( TNUM_OBJ(list), len );
}
new = NEW_PLIST(MUTABLE_TNUM(TNUM_OBJ(list)), len);
memcpy(ADDR_OBJ(new), CONST_ADDR_OBJ(list), (len + 1) * sizeof(Obj));
// 'CHANGED_BAG(new);' not needed, <new> is newest object
return new;
Expand Down Expand Up @@ -1297,6 +1288,7 @@ static Obj ElmsPlistDense(Obj list, Obj poss)
Int pos; // <position> as integer
Int inc; // increment in a range
Int i; // loop variable
Int tnum; // TNUM of <list>

// select no element
if ( LEN_LIST(poss) == 0 ) {
Expand All @@ -1312,22 +1304,23 @@ static Obj ElmsPlistDense(Obj list, Obj poss)
// get the length of <positions>
lenPoss = LEN_LIST( poss );

// get the (mutable) tnum of list
tnum = MUTABLE_TNUM(TNUM_OBJ(list));

// make the result list
// try to assert as many properties as possible
if (HAS_FILT_LIST(list, FN_IS_SSORT) && HAS_FILT_LIST(poss, FN_IS_SSORT))
{
elms = NEW_PLIST( MUTABLE_TNUM(TNUM_OBJ(list)), lenPoss);
elms = NEW_PLIST(tnum, lenPoss);
RESET_FILT_LIST( elms, FN_IS_NHOMOG); // can't deduce this one
}
else if (HAS_FILT_LIST(list, FN_IS_RECT))
elms = NEW_PLIST( T_PLIST_TAB_RECT, lenPoss );
else if (HAS_FILT_LIST(list, FN_IS_TABLE))
elms = NEW_PLIST( T_PLIST_TAB, lenPoss );
else if (T_PLIST_CYC <= TNUM_OBJ(list) && TNUM_OBJ(list) <=
T_PLIST_CYC_SSORT+IMMUTABLE)
else if (T_PLIST_CYC <= tnum && tnum <= T_PLIST_CYC_SSORT)
elms = NEW_PLIST( T_PLIST_CYC, lenPoss );
else if (T_PLIST_FFE <= TNUM_OBJ(list) && TNUM_OBJ(list) <=
T_PLIST_FFE+IMMUTABLE)
else if (T_PLIST_FFE <= tnum && tnum <= T_PLIST_FFE)
elms = NEW_PLIST( T_PLIST_FFE, lenPoss );
else if (HAS_FILT_LIST(list, FN_IS_HOMOG))
elms = NEW_PLIST( T_PLIST_HOM, lenPoss );
Expand Down Expand Up @@ -1391,19 +1384,20 @@ static Obj ElmsPlistDense(Obj list, Obj poss)
(Int)pos + (lenPoss - 1) * inc, 0);
}

// get the (mutable) tnum of list
tnum = MUTABLE_TNUM(TNUM_OBJ(list));

// make the result list
// try to assert as many properties as possible
if ( HAS_FILT_LIST(list, FN_IS_SSORT) && inc > 0 )
elms = NEW_PLIST( MUTABLE_TNUM(TNUM_OBJ(list)), lenPoss );
elms = NEW_PLIST( tnum, lenPoss );
else if (HAS_FILT_LIST(list, FN_IS_RECT))
elms = NEW_PLIST( T_PLIST_TAB_RECT, lenPoss );
else if (HAS_FILT_LIST(list, FN_IS_TABLE))
elms = NEW_PLIST( T_PLIST_TAB, lenPoss );
else if (T_PLIST_CYC <= TNUM_OBJ(list) && TNUM_OBJ(list) <=
T_PLIST_CYC_SSORT+IMMUTABLE)
else if (T_PLIST_CYC <= tnum && tnum <= T_PLIST_CYC_SSORT)
elms = NEW_PLIST( T_PLIST_CYC, lenPoss );
else if (T_PLIST_FFE <= TNUM_OBJ(list) && TNUM_OBJ(list) <=
T_PLIST_FFE+IMMUTABLE)
else if (T_PLIST_FFE <= tnum && tnum <= T_PLIST_FFE)
elms = NEW_PLIST( T_PLIST_FFE, lenPoss );
else if (HAS_FILT_LIST(list, FN_IS_HOMOG))
elms = NEW_PLIST( T_PLIST_HOM, lenPoss );
Expand Down Expand Up @@ -2337,7 +2331,7 @@ static Obj FuncASS_PLIST_DEFAULT(Obj self, Obj plist, Obj pos, Obj val)
Int p;

p = GetPositiveSmallInt("List Assignment", pos);
if (!IS_PLIST(plist) || !IS_PLIST_MUTABLE(plist)) {
if (!IS_PLIST(plist) || !IS_MUTABLE_OBJ(plist)) {
RequireArgumentEx(0, plist, "<list>", "must be a mutable plain list");
}

Expand Down
18 changes: 4 additions & 14 deletions src/plist.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,14 +50,14 @@ EXPORT_INLINE Obj NEW_PLIST(UInt type, Int plen)

EXPORT_INLINE Obj NEW_PLIST_IMM(UInt type, Int plen)
{
return NEW_PLIST(type | IMMUTABLE, plen);
Obj list = NEW_PLIST(type, plen);
MakeImmutableNoRecurse(list);
return list;
}

EXPORT_INLINE Obj NEW_PLIST_WITH_MUTABILITY(Int mut, UInt type, Int plen)
{
if (!mut)
type |= IMMUTABLE;
return NEW_PLIST(type, plen);
return mut ? NEW_PLIST(type, plen) : NEW_PLIST_IMM(type, plen);
}

/****************************************************************************
Expand Down Expand Up @@ -251,16 +251,6 @@ EXPORT_INLINE BOOL IS_DENSE_PLIST(Obj list)
TNUM_OBJ(list) <= LAST_PLIST_TNUM;
}

/****************************************************************************
**
*F IS_PLIST_MUTABLE( <list> ) . . . . . . . . . . . is a plain list mutable
*/
EXPORT_INLINE BOOL IS_PLIST_MUTABLE(Obj list)
{
GAP_ASSERT(IS_PLIST(list));
return !((TNUM_OBJ(list) - T_PLIST) % 2);
}

/****************************************************************************
**
*F AssPlist( <list>, <pos>, <val>) . . . . . . . . . assign to a plain list
Expand Down
4 changes: 2 additions & 2 deletions src/pperm.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -250,7 +250,7 @@ static inline Obj DOM_PPERM(Obj f)
static inline void SET_IMG_PPERM(Obj f, Obj img)
{
GAP_ASSERT(IS_PPERM(f));
GAP_ASSERT(IS_PLIST(img) && !IS_PLIST_MUTABLE(img));
GAP_ASSERT(IS_PLIST(img) && !IS_MUTABLE_OBJ(img));
GAP_ASSERT(DOM_PPERM(f) == NULL ||
LEN_PLIST(img) == LEN_PLIST(DOM_PPERM(f)));
// TODO check entries of img are valid
Expand All @@ -260,7 +260,7 @@ static inline void SET_IMG_PPERM(Obj f, Obj img)
static inline void SET_DOM_PPERM(Obj f, Obj dom)
{
GAP_ASSERT(IS_PPERM(f));
GAP_ASSERT(IS_PLIST(dom) && !IS_PLIST_MUTABLE(dom));
GAP_ASSERT(IS_PLIST(dom) && !IS_MUTABLE_OBJ(dom));
GAP_ASSERT(IMG_PPERM(f) == NULL ||
LEN_PLIST(dom) == LEN_PLIST(IMG_PPERM(f)));
// TODO check entries of img are valid
Expand Down
6 changes: 3 additions & 3 deletions src/trans.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -234,14 +234,14 @@ static inline Obj EXT_TRANS(Obj f)
static inline void SET_IMG_TRANS(Obj f, Obj img)
{
GAP_ASSERT(IS_TRANS(f));
GAP_ASSERT(img == NULL || (IS_PLIST(img) && !IS_PLIST_MUTABLE(img)));
GAP_ASSERT(img == NULL || (IS_PLIST(img) && !IS_MUTABLE_OBJ(img)));
ADDR_OBJ(f)[0] = img;
}

static inline void SET_KER_TRANS(Obj f, Obj ker)
{
GAP_ASSERT(IS_TRANS(f));
GAP_ASSERT(ker == NULL || (IS_PLIST(ker) && !IS_PLIST_MUTABLE(ker) &&
GAP_ASSERT(ker == NULL || (IS_PLIST(ker) && !IS_MUTABLE_OBJ(ker) &&
LEN_PLIST(ker) == DEG_TRANS(f)));
ADDR_OBJ(f)[1] = ker;
}
Expand Down Expand Up @@ -3365,7 +3365,7 @@ static Obj FuncOnPosIntSetsTrans(Obj self, Obj set, Obj f, Obj n)
}

if (IS_PLIST(set)) {
res = NEW_PLIST_WITH_MUTABILITY(IS_PLIST_MUTABLE(set), T_PLIST_CYC_SSORT, len);
res = NEW_PLIST_WITH_MUTABILITY(IS_MUTABLE_OBJ(set), T_PLIST_CYC_SSORT, len);
SET_LEN_PLIST(res, len);
}
else {
Expand Down
6 changes: 3 additions & 3 deletions src/vecgf2.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1373,7 +1373,7 @@ static void ConvGF2Vec(Obj list)
}
if (SIZE_PLEN_GF2VEC(len) > SIZE_OBJ(list))
ResizeBag(list, SIZE_PLEN_GF2VEC(len));
BOOL mutable = IS_PLIST_MUTABLE(list);
BOOL mutable = IS_MUTABLE_OBJ(list);

block = 0;
bit = 1;
Expand Down Expand Up @@ -1473,7 +1473,7 @@ static Obj NewGF2Vec(Obj list)
}

// mutability should be inherited from the argument
if (IS_PLIST_MUTABLE(list))
if (IS_MUTABLE_OBJ(list))
SetTypeDatObj(res, TYPE_LIST_GF2VEC);
else
SetTypeDatObj(res, TYPE_LIST_GF2VEC_IMM);
Expand Down Expand Up @@ -1529,7 +1529,7 @@ static Obj FuncCONV_GF2MAT(Obj self, Obj list)
SET_ELM_PLIST(list, i + 1, tmp);
}
SET_ELM_PLIST(list, 1, INTOBJ_INT(len));
mut = IS_PLIST_MUTABLE(list);
mut = IS_MUTABLE_OBJ(list);
RetypeBag(list, T_POSOBJ);
SET_TYPE_POSOBJ(list, mut ? TYPE_LIST_GF2MAT : TYPE_LIST_GF2MAT_IMM);
return (Obj)0;
Expand Down
Loading