From 5fe09f8151a913c5214a28e21d2303740252955c Mon Sep 17 00:00:00 2001 From: Max Horn Date: Fri, 27 Jun 2025 18:43:28 +0200 Subject: [PATCH 1/5] kernel: use MUTABLE_TNUM to avoid some uses of IMMUTABLE --- src/plist.c | 33 +++++++++++++++------------------ 1 file changed, 15 insertions(+), 18 deletions(-) diff --git a/src/plist.c b/src/plist.c index d4781ba49c..1299b2c7ff 100644 --- a/src/plist.c +++ b/src/plist.c @@ -759,20 +759,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, is newest object return new; @@ -1297,6 +1291,7 @@ static Obj ElmsPlistDense(Obj list, Obj poss) Int pos; // as integer Int inc; // increment in a range Int i; // loop variable + Int tnum; // TNUM of // select no element if ( LEN_LIST(poss) == 0 ) { @@ -1312,22 +1307,23 @@ static Obj ElmsPlistDense(Obj list, Obj poss) // get the length of 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 ); @@ -1391,19 +1387,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 ); From f3312c596b46a96b30b2d5a7adf36b2db0eff0a8 Mon Sep 17 00:00:00 2001 From: Max Horn Date: Fri, 27 Jun 2025 18:47:23 +0200 Subject: [PATCH 2/5] kernel: let KTNumPlist & KTNumHomPlist return mutable tnum --- src/plist.c | 41 ++++++++++++++++++----------------------- 1 file changed, 18 insertions(+), 23 deletions(-) diff --git a/src/plist.c b/src/plist.c index 1299b2c7ff..9b3ac02652 100644 --- a/src/plist.c +++ b/src/plist.c @@ -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) { @@ -231,7 +231,7 @@ static Int KTNumPlist(Obj list, Obj * famfirst) #ifdef HPCGAP if (!CheckWriteAccess(list)) { - return TNUM_OBJ(list); + return MUTABLE_TNUM(TNUM_OBJ(list)); } #endif // if list has `OBJ_FLAG_TESTING' keep that @@ -245,8 +245,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; @@ -432,7 +432,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; } @@ -451,7 +450,7 @@ static Int KTNumHomPlist(Obj list) #ifdef HPCGAP if (!CheckWriteAccess(list)) { - return TNUM_OBJ(list); + return MUTABLE_TNUM(TNUM_OBJ(list)); } #endif @@ -560,13 +559,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) @@ -613,7 +611,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 @@ -656,7 +657,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 @@ -669,24 +670,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 From a724d4f717ac216524b76b85c1de54d41efc5fd2 Mon Sep 17 00:00:00 2001 From: Max Horn Date: Fri, 27 Jun 2025 12:44:51 +0200 Subject: [PATCH 3/5] kernel: replace IS_PLIST_MUTABLE by IS_MUTABLE_OBJ --- src/listfunc.c | 4 ++-- src/plist.c | 6 +++--- src/plist.h | 10 ---------- src/pperm.cc | 4 ++-- src/trans.cc | 6 +++--- src/vecgf2.c | 6 +++--- 6 files changed, 13 insertions(+), 23 deletions(-) diff --git a/src/listfunc.c b/src/listfunc.c index e472cbee74..b3e350b4ae 100644 --- a/src/listfunc.c +++ b/src/listfunc.c @@ -67,7 +67,7 @@ void AddPlist3(Obj list, Obj obj, Int pos) { UInt len; - if (!IS_PLIST_MUTABLE(list)) { + if (!IS_MUTABLE_OBJ(list)) { ErrorMayQuit("List Assignment: must be a mutable list", 0, 0); } // in order to be optimistic when building list call assignment @@ -165,7 +165,7 @@ static Obj RemPlist(Obj list) Int pos; Obj removed; - if ( ! IS_PLIST_MUTABLE(list) ) { + if ( ! IS_MUTABLE_OBJ(list) ) { ErrorMayQuit("Remove: must be a mutable list", 0, 0); } pos = LEN_PLIST( list ); diff --git a/src/plist.c b/src/plist.c index 9b3ac02652..de42cee783 100644 --- a/src/plist.c +++ b/src/plist.c @@ -266,7 +266,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); isTable = FALSE; } else { @@ -331,7 +331,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; } @@ -2329,7 +2329,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, "", "must be a mutable plain list"); } diff --git a/src/plist.h b/src/plist.h index 9bddfcce1e..ec68403475 100644 --- a/src/plist.h +++ b/src/plist.h @@ -251,16 +251,6 @@ EXPORT_INLINE BOOL IS_DENSE_PLIST(Obj list) TNUM_OBJ(list) <= LAST_PLIST_TNUM; } -/**************************************************************************** -** -*F IS_PLIST_MUTABLE( ) . . . . . . . . . . . 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( , , ) . . . . . . . . . assign to a plain list diff --git a/src/pperm.cc b/src/pperm.cc index 83c4764c84..9086e7e834 100644 --- a/src/pperm.cc +++ b/src/pperm.cc @@ -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 @@ -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 diff --git a/src/trans.cc b/src/trans.cc index 4506642b72..66d4e4f0e2 100644 --- a/src/trans.cc +++ b/src/trans.cc @@ -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; } @@ -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 { diff --git a/src/vecgf2.c b/src/vecgf2.c index d0113401e0..041c0b60fe 100644 --- a/src/vecgf2.c +++ b/src/vecgf2.c @@ -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; @@ -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); @@ -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; From 16a3fc4564aff64feb3beb76e7c281b703dbbf3a Mon Sep 17 00:00:00 2001 From: Max Horn Date: Thu, 26 Jun 2025 22:51:19 +0200 Subject: [PATCH 4/5] kernel: implement NEW_PLIST_IMM via MakeImmutableNoRecurse --- src/plist.h | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/plist.h b/src/plist.h index ec68403475..e61ae68e86 100644 --- a/src/plist.h +++ b/src/plist.h @@ -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); } /**************************************************************************** From 8c5b6d6add0b95d0b12ff985ec3f9adb5a388d79 Mon Sep 17 00:00:00 2001 From: Max Horn Date: Mon, 12 Oct 2026 00:34:26 +0200 Subject: [PATCH 5/5] kernel: assert plain lists where IS_PLIST_MUTABLE did IS_PLIST_MUTABLE asserted that its argument is a plain list, which IS_MUTABLE_OBJ does not. Keep that check in AddPlist3, RemPlist and KTNumPlist; the latter also covers the list entries it inspects, as only KTNumPlist and TypePlistWithKTNum set OBJ_FLAG_TESTING. Assisted-by: Claude Code (Opus 5.5) --- src/listfunc.c | 2 ++ src/plist.c | 2 ++ 2 files changed, 4 insertions(+) diff --git a/src/listfunc.c b/src/listfunc.c index b3e350b4ae..e400dfe8fb 100644 --- a/src/listfunc.c +++ b/src/listfunc.c @@ -67,6 +67,7 @@ void AddPlist3(Obj list, Obj obj, Int pos) { UInt len; + GAP_ASSERT(IS_PLIST(list)); if (!IS_MUTABLE_OBJ(list)) { ErrorMayQuit("List Assignment: must be a mutable list", 0, 0); } @@ -165,6 +166,7 @@ static Obj RemPlist(Obj list) Int pos; Obj removed; + GAP_ASSERT(IS_PLIST(list)); if ( ! IS_MUTABLE_OBJ(list) ) { ErrorMayQuit("Remove: must be a mutable list", 0, 0); } diff --git a/src/plist.c b/src/plist.c index de42cee783..a96f0735e3 100644 --- a/src/plist.c +++ b/src/plist.c @@ -229,6 +229,8 @@ static Int KTNumPlist(Obj list, Obj * famfirst) Obj loopTypeObj = 0; // typeObj in loop + GAP_ASSERT(IS_PLIST(list)); + #ifdef HPCGAP if (!CheckWriteAccess(list)) { return MUTABLE_TNUM(TNUM_OBJ(list));