Skip to content

kernel: replace IMMUTABLE by OBJ_FLAG_IMMUTABLE - #6025

Draft
fingolfin wants to merge 37 commits into
gap-system:masterfrom
fingolfin:mh/immutable
Draft

fingolfin wants to merge 37 commits into
gap-system:masterfrom
fingolfin:mh/immutable

Conversation

@fingolfin

@fingolfin fingolfin commented Jun 27, 2025 •

Copy link
Copy Markdown
Member

This PR replaces the IMMUTABLE bit 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

  • Every tnum between FIRST_IMM_MUT_TNUM and LAST_IMM_MUT_TNUM used to come as a mutable/immutable pair. Now there is one tnum per kind of list or record, and IS_MUTABLE_OBJ reads the flag for these. IMMUTABLE, MUTABLE_TNUM, IMMUTABLE_TNUM and IS_PLIST_MUTABLE are gone.
  • For all other objects nothing changes: IS_MUTABLE_OBJ still asks IsMutableObjFuncs, so for component, positional and data objects the type decides. The flag has no meaning for them.
  • Most list tnums get new numbers, and FIRST_EXTERNAL_TNUM drops from 82 to 51. The kernel major version is therefore increased to 12.
  • TNAM_OBJ no 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.

  • The interpreter and compiled code assign directly to a list whose tnum is T_PLIST; that tnum used to imply "mutable".
  • NEW_PLIST(TNUM_OBJ(x), len) used to hand the mutability of x to the new list: ZeroSameMutability for plain lists of FFEs, sum and difference of a cyclotomic vector and a scalar, REDUCE_LETREP_WORDS_REW_SYS.
  • RetypeBagSM when a compressed vector or matrix turns back into a plain list, CLONE_OBJ (hence InstallValue), and SET_TYPE_OBJ turning a plain list into a positional object.
  • HPC-GAP: the serializer, which writes tnums, and the copies made by the traversal code.

Fallout outside this repository

  • Kernel extensions must be recompiled. Among the distributed packages only semigroups fails to compile; it uses T_STRING + IMMUTABLE in src/to-cpp.hpp. Fix detection of GAP strings semigroups/Semigroups#1236 removes that.
  • GAP.jl hardcodes FIRST_EXTERNAL_TNUM = 82 in src/lowlevel.jl and asserts it when loading.

Parts being merged separately

To shrink this PR, pieces that also work on master are 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.

  • Refactorings that do not depend on the flag
    • kernel: use MUTABLE_TNUM to avoid some uses of IMMUTABLE
    • kernel: let KTNumPlist & KTNumHomPlist return mutable tnum
    • kernel: replace IS_PLIST_MUTABLE by IS_MUTABLE_OBJ
    • Remove IMMUTABLE: src/plist.h
    • kernel: make implicit mutability check explicit
  • Add the flag
    • kernel: add OBJ_FLAG_IMMUTABLE
  • Remove code that becomes superfluous
    • kernel: remove MakeBagTypePublic calls for IMMUTABLE tnums
    • kernel: remove IMMUTABLE from FiltListTNums handling
    • Remove IMMUTABLE: src/… (11 commits, one per file, all repetitive)
    • kernel: adjust IS_* helpers to ignore IMMUTABLE
  • Switch over
    • Remove (IM)MUTABLE_TNUM
    • kernel: remove IMMUTABLE
    • Fix lib/mat8bit.gi to deal with IMMUTABLE removal
    • WIP: fixup TypePlistHomHelper
    • Adjust some tests
    • Some IMMUTABLE fixes
    • TODOs for the HPC-GAP serializer code
    • HPC-GAP fix
  • Cleanup
    • kernel: remove PrecheckRetypeBag
    • Increase kernel major version
    • kernel: say "immutable" in argument errors
  • Repairs for the places listed above
    • kernel: keep mutability when retyping and cloning
    • kernel: check mutability in plist assignment fast path
    • kernel: keep immutability of lists made from a tnum
    • hpc: serialize mutability of lists and records
    • hpc: keep immutability when copying traversed objects
    • kernel: let the type decide if an object is mutable

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

testinstall passes on a regular and on an HPC-GAP build. Before the last rebase teststandard, testbugfix, testkernel, testlibgap and testinstall from 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 between master and this branch; they agree.

Not tested: packages with kernel extensions, tst/test-compile, a build with the Julia GC.

Open: DeserializeTypedObj in src/hpc/serialize.c still carries a TODO/FIXME about 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-by trailer), made small fixes in three of the older commits (src/range.c, src/stringobj.c, lib/mat8bit.gi) and drafted this description.

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 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

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release notes: not needed PRs introducing changes that are wholly irrelevant to the release notes topic: kernel

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant