Repository navigation
Conversation
fingolfin
force-pushed
the
mh/immutable
branch
from
August 14, 2025 20:41
2dc14c8 to
13e86ac
Compare
fingolfin
force-pushed
the
mh/immutable
branch
from
October 25, 2025 15:36
13e86ac to
86e3522
Compare
For now, we only set this in MakeImmutable and MakeImmutableNoRecurse, and test it in IS_MUTABLE_OBJ. In the future, it should also be set for all objects with TNUM <= LAST_CONSTANT_TNUM (but be careful about immediate integers and FFEs), and eventually the 'IMMUTABLE' TNUMs could be phased out.
Its only checks compared the mutability bit of the old and new tnum. With mutability in an object flag, retyping cannot change it, and all that remained were the early returns for the exceptions. Assisted-by: Claude Code (Opus 5.5)
A tnum no longer says whether an object is mutable, so three places that relied on it lost that information: - RetypeBagSM did nothing beyond RetypeBag. An immutable compressed vector or matrix, which records its mutability in its type, came out of PLAIN_GF2VEC, PLAIN_VEC8BIT, PLAIN_GF2MAT and PLAIN_MAT8BIT as a mutable plain list. - CLONE_OBJ copied tnum and contents but not the flag, so the clone kept the mutability of the object it overwrote. This also affected InstallValue. - SET_TYPE_OBJ turned an immutable plain list into a positional object that stayed immutable whatever its type said. Assisted-by: Claude Code (Opus 5.5)
Removing the immutable tnums renumbers most list tnums, so kernel extensions compiled against an older GAP no longer match. Assisted-by: Claude Code (Opus 5.5)
The tnum names of lists and records no longer mention mutability, so a failed mutability check reported, e.g., "<list1> must be a mutable list (not a plain list of cyclotomics)". Assisted-by: Claude Code (Opus 5.5)
The interpreter and compiled code assign directly to a list with tnum T_PLIST, which used to imply that the list is mutable. Since immutable lists share that tnum, `l[i] := x` inside a function modified them. Assisted-by: Claude Code (Opus 5.5)
Some functions create their result with the tnum of an argument, which used to carry over the mutability as well. Pass it on explicitly in ZeroSameMutability for plain lists of FFEs, in sums and differences of a cyclotomic vector and a scalar, and in REDUCE_LETREP_WORDS_REW_SYS. Assisted-by: Claude Code (Opus 5.5)
The serializer writes the tnum of an object, which no longer tells whether it is mutable, so immutable lists, strings and records came back mutable. Precede them by a tag. Assisted-by: Claude Code (Opus 5.5)
The copies are allocated with the tnum of the original, which used to include its mutability. Assisted-by: Claude Code (Opus 5.5)
OBJ_FLAG_IMMUTABLE records the mutability of lists and records. Any other object keeps asking its IsMutableObjFuncs handler, even after MakeImmutable; for component, positional and data objects that is the type. Otherwise MakeImmutable pinned such an object: code setting the IsMutable filter again afterwards, as homalg does for its matrices, got an object whose type is mutable while IsMutable returns false. Assisted-by: Claude Code (Opus 5.5)
fingolfin
force-pushed
the
mh/immutable
branch
from
October 8, 2026 05:57
86e3522 to
9e55c2d
Compare
This was referenced Oct 8, 2026
fingolfin
added a commit
that referenced
this pull request
Oct 8, 2026
Two places test for an exact tnum and rely on it implying that the object is mutable: the interpreter and compiled code assign directly to a list with tnum T_PLIST, and MigrateObjects in HPC-GAP sorts the components of a record with tnum T_PREC. Test the mutability as well, so that this keeps working once mutability is no longer part of the tnum (see #6025). Assisted-by: Claude Code (Opus 5.5)
fingolfin
added a commit
that referenced
this pull request
Oct 8, 2026
Two places test for an exact tnum and rely on it implying that the object is mutable: the interpreter and compiled code assign directly to a list with tnum T_PLIST, and MigrateObjects in HPC-GAP sorts the components of a record with tnum T_PREC. Test the mutability as well, so that this keeps working once mutability is no longer part of the tnum (see #6025). Assisted-by: Claude Code (Opus 5.5)
fingolfin
added a commit
that referenced
this pull request
Oct 8, 2026
Some functions create their result with the tnum of an argument, and thereby give it the mutability of that argument: ZeroSameMutability for plain lists of FFEs, sums and differences of a cyclotomic vector and a scalar, and REDUCE_LETREP_WORDS_REW_SYS. Say so explicitly, so that this keeps working once mutability is no longer part of the tnum (see #6025). Assisted-by: Claude Code (Opus 5.5)
fingolfin
added a commit
that referenced
this pull request
Oct 8, 2026
Cover places where the mutability of an object travels with its tnum or its type: converting compressed vectors and matrices back to plain lists, CLONE_OBJ, turning a plain list into a positional object, setting the IsMutable filter again after MakeImmutable, and the HPC-GAP serializer. These are the places that need care when mutability moves out of the tnum (see #6025). Assisted-by: Claude Code (Opus 5.5)
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR replaces the
IMMUTABLEbit in the tnums of lists and records by an object flag,OBJ_FLAG_IMMUTABLE. Doing this has been my plan since at least 2018 and I've started and abandoned attempts at this several times.Originally a motivation was to support HPC-GAP but this has faded away. But this PR also makes it easier to ensure immutability is never removed accidentally. It also removes a lot of code (the kernel sources shrink by about 420 lines), and simplifies some stuff.
What changes
FIRST_IMM_MUT_TNUMandLAST_IMM_MUT_TNUMused to come as a mutable/immutable pair. Now there is one tnum per kind of list or record, andIS_MUTABLE_OBJreads the flag for these.IMMUTABLE,MUTABLE_TNUM,IMMUTABLE_TNUMandIS_PLIST_MUTABLEare gone.IS_MUTABLE_OBJstill asksIsMutableObjFuncs, so for component, positional and data objects the type decides. The flag has no meaning for them.FIRST_EXTERNAL_TNUMdrops from 82 to 51. The kernel major version is therefore increased to 12.TNAM_OBJno longer distinguishes mutable from immutable lists. Argument checks compensate: they now report e.g.(not an immutable list (boolean))where they said(not a list (boolean,imm)).Things that silently depended on the tnum
Removing the bit is mostly mechanical, but a few places got the mutability for free from the tnum and had to be changed by hand. Each has a test now.
T_PLIST; that tnum used to imply "mutable".NEW_PLIST(TNUM_OBJ(x), len)used to hand the mutability ofxto the new list:ZeroSameMutabilityfor plain lists of FFEs, sum and difference of a cyclotomic vector and a scalar,REDUCE_LETREP_WORDS_REW_SYS.RetypeBagSMwhen a compressed vector or matrix turns back into a plain list,CLONE_OBJ(henceInstallValue), andSET_TYPE_OBJturning a plain list into a positional object.Fallout outside this repository
T_STRING + IMMUTABLEinsrc/to-cpp.hpp. Fix detection of GAP strings semigroups/Semigroups#1236 removes that.FIRST_EXTERNAL_TNUM = 82insrc/lowlevel.jland asserts it when loading.Parts being merged separately
To shrink this PR, pieces that also work on
masterare split off: #6703 (tests), #6704 and #6705. The refactorings at the start of the series will follow as a fourth PR. Once they are merged, this branch gets rebased and the corresponding commits drop out.Guide to the commits
The commits are meant to be reviewed individually.
The repairs come after the switch, so the commits in between are known to be broken; this needs reordering or squashing before a merge.
Status
testinstallpasses on a regular and on an HPC-GAP build. Before the last rebaseteststandard,testbugfix,testkernel,testlibgapandtestinstallfrom a saved workspace passed as well. In addition I compared the mutability of the results of about 90 list operations on 28 kinds of lists betweenmasterand this branch; they agree.Not tested: packages with kernel extensions,
tst/test-compile, a build with the Julia GC.Open:
DeserializeTypedObjinsrc/hpc/serialize.cstill carries aTODO/FIXMEabout immutable records.AI disclosure
Claude Code (Opus 5.5) rebased the branch, found and fixed the regressions listed under "Things that silently depended on the tnum" (the commits with an
Assisted-bytrailer), made small fixes in three of the older commits (src/range.c,src/stringobj.c,lib/mat8bit.gi) and drafted this description.