Skip to content

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

Merged
fingolfin merged 5 commits into
masterfrom
mh-claude/immutable-prep-refactorings
Oct 14, 2026
Merged

fingolfin merged 5 commits into
masterfrom
mh-claude/immutable-prep-refactorings

Conversation

@fingolfin

@fingolfin fingolfin commented Oct 8, 2026 •

Copy link
Copy Markdown
Member

Preparation for #6025, which moves the mutability of lists and records from the tnum into an object flag. These commits stop a few places from handling the IMMUTABLE bit by hand. No change in behaviour.

  • ShallowCopyPlist and ElmsPlistDense use MUTABLE_TNUM instead of subtracting IMMUTABLE or comparing against ... + IMMUTABLE.
  • KTNumPlist and KTNumHomPlist return the mutable tnum. TypePlistHomHelper, which needs the bit to pick the type, adds it back from IS_MUTABLE_OBJ. This continues kernel: avoid some uses of IMMUTABLE in plist.c #6024 and is the one commit that needs a careful look.
  • IS_PLIST_MUTABLE is replaced by IS_MUTABLE_OBJ and removed. I found no use of it in the distributed packages.
  • NEW_PLIST_IMM and NEW_PLIST_WITH_MUTABILITY go through MakeImmutableNoRecurse instead of or-ing the bit into the tnum.

AI disclosure: Claude Code (Opus 5.5)

@fingolfin fingolfin added topic: kernel release notes: not needed PRs introducing changes that are wholly irrelevant to the release notes labels Oct 8, 2026
@codecov

codecov Bot commented Oct 8, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 90.90909% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 79.55%. Comparing base (482ebce) to head (8c5b6d6).
⚠️ Report is 11 commits behind head on master.

Files with missing lines Patch % Lines
src/plist.c 89.28% 3 Missing ⚠️
src/listfunc.c 75.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #6707      +/-   ##
==========================================
- Coverage   79.56%   79.55%   -0.02%     
==========================================
  Files         684      684              
  Lines      295665   295512     -153     
  Branches     8656     8656              
==========================================
- Hits       235259   235100     -159     
- Misses      58582    58590       +8     
+ Partials     1824     1822       -2     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@fingolfin
fingolfin force-pushed the mh-claude/immutable-prep-refactorings branch from 4db271a to 16a3fc4 Compare October 8, 2026 20:15

@james-d-mitchell james-d-mitchell left a comment

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.

Looks good to me, mod the minor comments about re-adding some assertions.

Comment thread src/listfunc.c
Comment thread src/listfunc.c
Comment thread src/plist.c
else if (TEST_OBJ_FLAG(elm, OBJ_FLAG_TESTING)) {
isHom = FALSE;
areMut = IS_PLIST_MUTABLE(elm);
areMut = IS_MUTABLE_OBJ(elm);

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?

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.

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)
@lgoettgens
lgoettgens removed their request for review October 12, 2026 14:07
@fingolfin

Copy link
Copy Markdown
Member Author

@james-d-mitchell @ChrisJefferson any further concerns or requests?

@james-d-mitchell james-d-mitchell left a comment

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.

Looks good, thanks @fingolfin

@fingolfin
fingolfin merged commit 09e5b65 into master Oct 14, 2026
34 checks passed
@fingolfin
fingolfin deleted the mh-claude/immutable-prep-refactorings branch October 14, 2026 19:34
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.

3 participants