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
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)
fingolfin
force-pushed
the
mh/immutable
branch
from
October 15, 2026 07:12
9e55c2d to
65f7cd0
Compare
MakeImmutableNoRecurse sets the new flag in addition to the IMMUTABLE bit of the tnum, so that the code can move over to the flag before the bit goes away. The flag is only meant for lists, records, object sets and object maps; for other objects their type decides. Assisted-by: Claude Code (Opus 5.5)
Lists, records, object sets and object maps no longer come in pairs of a mutable and an immutable tnum; OBJ_FLAG_IMMUTABLE alone records their mutability. This renumbers most list tnums. Consequences handled here: - TypePlistHomHelper computes the index into the type list of a family from the flag, as the tnums are now consecutive. - RetypeBagSM, CLONE_OBJ and SET_TYPE_OBJ carry the mutability over explicitly, as the tnum no longer does. RetypeBag cannot make an object mutable anymore, so PrecheckRetypeBag goes. - SET_TYPE_OBJ and SET_TYPE_COMOBJ reject immutable records, which used to fail for having a tnum of their own. - In HPC-GAP, MakeImmutableNoRecurse makes the bag public; retyping to an immutable tnum used to do that. - Argument errors say "immutable", as tnum names no longer do. - lib/mat8bit.gi and the tests no longer assume the old tnum numbers and names. 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)
fingolfin
force-pushed
the
mh/immutable
branch
from
October 18, 2026 22:47
65f7cd0 to
740c524
Compare
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_TNUMandIMMUTABLE_TNUMare gone (IS_PLIST_MUTABLEwent in Reduce reliance on theIMMUTABLEtnum bit in the plain list code #6707).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. This breaks the kernel ABI; master already raised the kernel major version to 12 in kernel: store nargs and nloc as machine ints; increase kernel major version #6590.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". Fixed onmasterby kernel: check mutability where a tnum implied it #6705.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. Fixed onmasterby kernel: pass mutability on explicitly to new lists #6704.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 merged separately
The pieces that also work on
masterwere split off and are merged: #6703 (tests), #6704, #6705 and #6707 (the refactorings that used to open this series). This branch is rebased on top of them.Guide to the commits
The commits are meant to be reviewed individually. Those between adding the flag and the switch are not meant to be tested on their own.
Status
On the current head
testinstallpasses. On the same series based 11 master commits earlier,testinstall,teststandardandtestbugfixpassed on a regular build, andtestinstallon an HPC-GAP build with #6721 applied (without it HPC-GAP sometimes crashes inmapping.tst, an unrelated bug fixed there). In addition I compared the mutability of the results of about 90 list operations on 28 kinds of lists betweenmasterand this branch; they agree.Kernel extensions built against this branch: the test suites of io, orb, cvec, datastructures, digraphs, ferret, gauss, json, profiling and edim pass. digraphs needs a fix for an unrelated assertion in
DIGRAPH_HASHin debug builds. semigroups, with semigroups/Semigroups#1236 applied, shows 16 differences, all from the new view of plist matrices (#6686), none from this PR.Not tested: kernel extensions needing external libraries,
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.