Recursion boxes through a handle, and a value copy is deep - #155
Conversation
A future session reading a spec that says "subject" and stories that say "receiver" would most likely reconcile them the wrong way. Record which tree is which, the one legitimate hit under spec/, and that sweeping stories/ is an append-only violation rather than a cleanup.
A recursive member is now an ordinary hosting member the compiler boxes — a fixed-size handle inline, payload in the scope's dynamic region — so a recursive field owns its child instead of guesting it. `adt.md` §4.1 and its rooted-in-a-field rule are gone; the dynamic region's power-of-two classes reach down to 16 bytes for fixed-size boxed payloads. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017AYiJXjzCPEDfYRW1ZtxH7
`stories/adt.md` gains "A recursive child is owned, not aliased" — why `&` was doing double duty and what the `Leaves` contortion was a symptom of. `stories/memory.md` gains "The region takes the boxes, and the classes reach down" — where a boxed payload lives, why the classes now start at 16 bytes, and how provisional that number is. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017AYiJXjzCPEDfYRW1ZtxH7
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository: zane-lang/coderabbit/.coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe specification changes recursive members from explicit ChangesRecursive hosting model
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant ExprConstructor
participant Operation
participant DynamicRegion
participant DestinationScope
ExprConstructor->>Operation: construct nested Expr hosting values
Operation->>DynamicRegion: allocate boxed recursive payloads
DynamicRegion-->>Operation: return fixed-size handles
Operation->>DestinationScope: rehost the recursive value
DestinationScope->>DynamicRegion: recursively relocate owned payloads
Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@spec/glossary.md`:
- Around line 257-261: Add a living story pointer immediately after the
“Canonical home” line in the “boxed hosting field” glossary entry and before the
separator, linking to the relevant integrated ADT or memory story using the
existing `> **Story:**` format.
- Around line 257-261: Clarify the definition of “boxed hosting field” to
explicitly include variant case payloads such as Expr.op, Expr.flip, and
Expr.parenthesized, or consistently rename the term to “boxed hosting member”
throughout the entry. Keep the ownership, layout, and recursive-type semantics
unchanged.
- Line 188: Update the Meaning definition to limit fixed-size-region allocation
to handles materialized in scope-level slots. Clarify that handles embedded
within boxed or other dynamic payloads remain part of their containing dynamic
blocks, including nested boxed children, while preserving the existing rehosting
behavior.
In `@spec/lifetimes.md`:
- Around line 35-42: Update the variant-case move-source descriptions in section
1.2 so only case forms producing a `#variant` qualify as hosting move sources;
explicitly exclude value-variant cases from both the normative bullet and the
summary explanation, preserving their copy semantics.
In `@spec/syntax.md`:
- Around line 185-189: Clarify the Expr variant definition by adding a precise
reference to the implicit boxing rule for recursive hosting fields such as
Operation.left and Operation.right. Reconcile the §2.10 discussion around stored
references and &Tree so it distinguishes compiler boxing for field storage from
explicit reference aliasing, without implying these fields are inline recursive
values or invalid.
In `@stories/adt.md`:
- Around line 119-150: Restore the previous recursive-storage chapter, including
the explicit-& rules, Leaves workaround, and rooted-in-a-field explanation,
without altering its historical wording. Append the compiler-boxed
hosting-fields decision as a new chapter after the restored content, preserving
both accounts as an append-only story.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: zane-lang/coderabbit/.coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 9cc1c168-13c6-4309-987e-39b8ad83813a
📒 Files selected for processing (11)
CLAUDE.mdREADME.mdspec/adt.mdspec/foundations.mdspec/glossary.mdspec/lifetimes.mdspec/memory.mdspec/syntax.mdspec/types.mdstories/adt.mdstories/memory.md
- The coined term is a boxed hosting *member*: a `#variant` case payload is boxed on the same terms as a `#struct` field, and "field" did not cover it. - Only a `#variant` case form is a move-source. A value `variant` case is copied inline and has no hosting to transfer. - A handle inside a dynamic payload belongs to that block; only handles materialized in scope slots are placed in the fixed-size region. - syntax.md §2.10 still showed recursion as `node &Tree`, which is now the aliasing spelling rather than the recursive one.
The size classes were never a property of the dynamic region — §3.2's stacks are keyed by exact byte size, and powers of two appear only because a list doubles. A boxed payload never grows, so it has nothing for a class to absorb: it asks for exactly its type's size, aligned as that type is aligned anywhere else. This drops the 16-byte floor and the class ladder below 128 entirely, and replaces the derived alignment rule with one that names the two kinds separately.
The maintainer's account: the trigger was being forced to bind children to storage — sometimes to invent a struct for it — purely so a variant could point at them. Inlining being wrong because a type's size varies is the deeper thing behind it, not the thing that started it. The memory chapter follows the spec: a boxed payload has no size class, so the ladder-and-floor account it carried is replaced by the reason there was never a ladder.
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
spec/memory.md (2)
393-393: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winRemove the remaining boxed-payload size classes.
Line 393 still calls shared byte sizes “classes,” and Lines 407-408 state that sub-128 classes exist. This conflicts with Lines 344-346 and 405, which require exact type-size allocation with no class or floor. Keep powers-of-two sizes for growable backing stores only. Describe boxed reuse through exact byte-size stacks.
Also applies to: 405-409
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@spec/memory.md` at line 393, Update the dynamic block-size description to remove “classes” and any implication of sub-128 size buckets or flooring. Require exact type-size allocation for boxed payloads, retain powers-of-two sizing only for growable backing stores, and describe boxed reuse as stacks keyed by exact byte size.
580-586: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winCorrect the boxed-field placement summary.
Line 581 says the handle is inline “with its instance in the dynamic region.” The handle is inline with the enclosing host. Only the boxed payload named by the handle is in the dynamic region. The enclosing host can be a fixed-size scope slot or a dynamic payload. Rewrite this row to match Sections 3.3 and 3.5.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@spec/memory.md` around lines 580 - 586, Rewrite the “Boxed hosting field” row to state that the fixed-size handle is stored inline with its enclosing host, which may be a fixed-size scope slot or a dynamic payload; only the boxed payload referenced by the handle resides in the dynamic region. Preserve the existing recursive-type rationale and compiler-selection behavior, aligning the wording with Sections 3.3 and 3.5.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@spec/memory.md`:
- Line 405: Update Section 3.1’s oversized-span allocation rule to round up to
enough 1 MiB chunks to cover the entire payload, using ceil(block_size / 1 MiB)
rather than truncating integer division. Preserve exact sizing for boxed
payloads while ensuring payloads just over a MiB boundary receive an additional
chunk.
- Around line 342-346: The dynamic-region reuse policy must preserve alignment,
not only exact byte size. Update the size_stack reuse logic described in the
dynamic allocation and return sections to key blocks by both size and alignment,
or validate alignment before reusing a block and fall back to frontier
allocation when it does not satisfy the request.
In `@spec/syntax.md`:
- Around line 284-289: Add a `> **Story:**` pointer immediately before the
section separator in the recursive-storage discussion, linking to the applicable
story chapter heading. Keep the existing explanation of compiler boxing and `&`
aliasing unchanged.
In `@stories/adt.md`:
- Line 150: Rewrite the paragraph’s cycle example to accurately describe the
graph: acknowledge both back edges, explain that removing Expr.op alone breaks
the cycle, and that breaking it through child edges requires removing both
Operation.left and Operation.right; alternatively, replace it with a genuinely
single-back-edge example while preserving the rule that every member edge on a
cycle is boxed.
---
Outside diff comments:
In `@spec/memory.md`:
- Line 393: Update the dynamic block-size description to remove “classes” and
any implication of sub-128 size buckets or flooring. Require exact type-size
allocation for boxed payloads, retain powers-of-two sizing only for growable
backing stores, and describe boxed reuse as stacks keyed by exact byte size.
- Around line 580-586: Rewrite the “Boxed hosting field” row to state that the
fixed-size handle is stored inline with its enclosing host, which may be a
fixed-size scope slot or a dynamic payload; only the boxed payload referenced by
the handle resides in the dynamic region. Preserve the existing recursive-type
rationale and compiler-selection behavior, aligning the wording with Sections
3.3 and 3.5.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: zane-lang/coderabbit/.coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 06769c7c-c45a-435f-9f18-9e1a05dec60c
📒 Files selected for processing (9)
README.mdspec/adt.mdspec/foundations.mdspec/glossary.mdspec/lifetimes.mdspec/memory.mdspec/syntax.mdstories/adt.mdstories/memory.md
Four of these are consequences of dropping size classes that I missed: reuse stacks keyed by size alone can hand a block to a type needing stronger alignment; an oversized span sized by truncating division under-allocates a payload that is not a whole number of chunks; §3.6 still carried the four-classes-below-128 paragraph; and the summary row put the handle inline with its own payload rather than with its enclosing host. Also corrects the story's feedback-edge example: Operation has two back edges, so dropping left alone does not break the cycle.
| - **`Reading a variant member is partial`.** A case may not be live, so a member read is abortable. The primary consumer is exhaustive dispatch. | ||
| - **`A variant is matched in one central block`.** A `match` block (§5) dispatches a variant on its live tag — variant matching, not pattern matching: no nested destructuring, guards, or shape tests — and must cover every case, with no default arm. | ||
| - **`Recursion requires a reference type`.** A recursive sum must be a `#variant`, never a value `variant`, because a value type is transitively value and cannot hold the `&` a recursive member boxes through. | ||
| - **`Recursion requires a reference type`.** A recursive sum must be a `#variant`, never a value `variant`. A recursive member is an ordinary **hosting** member that the compiler **boxes** — a fixed-size handle inline, payload in the dynamic region — and only a reference type may own boxed storage. |
There was a problem hiding this comment.
what does that mean? can value types not store things in variants?
There was a problem hiding this comment.
Value types can absolutely be stored in variants — that bullet was badly worded and read as a much wider restriction than it is. Fixed in 68bc769.
Two separate things were running together:
- What a variant may hold. A
#variantcase payload can be any type. A valuevariantcan hold value payloads. Neither is affected here. - Whether a variant may lead back to itself. Only that is restricted, and only for the value form.
And "only a reference type may own boxed storage" was assertion, not reason. The reason is that a box is ownership. A boxed payload lives in the dynamic region and something has to decide when it dies; that something is a host. A value type has no host, no anchor, and no destruction tracking (memory.md §2.10), and it is copied by a mechanical inline write. Give it a box and the copy has exactly two options, both fatal:
- copy the handle — two values now name one payload, so a value is aliased, which is the thing §2.10 exists to prevent and which
concurrency.md§4 leans on when it lets a value be shared by snapshot and mutated concurrently; and nothing hosts the payload, so nothing frees it; - copy the payload — assignment becomes allocating and costs time proportional to the structure, which is not what "copied as an ordinary inline value" means.
§2.10 now states that reasoning where the rule lives, with an illegal example alongside the existing two, and the bullet here says what is and is not restricted.
Generated by Claude Code
| A `variant` is a **sum mould**. A value of the type it declares holds **exactly one** of the variant's named members at a time. The body uses `{ }` brackets with `;`-terminated members, each a lowercase member name followed by its payload type — the same grammar a `struct` uses. | ||
|
|
||
| A plain `variant` is a **value** sum: copied on assignment, transitively value, non-recursive. A `#variant` is a **reference** sum: it has identity, may hold reference-type and `&` payloads, and may recurse (§4). A recursive sum such as `Expr` — whose members refer back to `Expr` through `&` — must therefore be a `#variant`: | ||
| A plain `variant` is a **value** sum: copied on assignment, transitively value, non-recursive. A `#variant` is a **reference** sum: it has identity, may hold reference-type and `&` payloads, and may recurse (§4). A recursive sum such as `Expr` — whose payloads lead back to `Expr` — must therefore be a `#variant`: |
There was a problem hiding this comment.
would recursive variants not also work with value types? my example only used #variant because it had intLit which is a String which is itself a reference type.
There was a problem hiding this comment.
You're right about the example, and it was doing real damage — fixed in 68bc769.
Expr is a #variant for two independent reasons and the section only claimed one. intLit String forces it on its own: String is a reference type and a value sum may not carry one (memory.md §2.10). So a reader who checks the example against the rule finds it proves nothing about recursion. §3 now says that outright and isolates the rule with a pair whose payloads are all value types:
type Nat = variant { zero Unit; succ Nat; } // ILLEGAL: a value sum cannot lead back to itself
type Peano = #variant { zero Unit; succ Peano; } // legal: `succ` is a boxed hosting member
On the substance — no, recursion does not work for value types, and it is not an accident of how the rule was written. Boxing is the only finite layout for a self-reference, and a boxed payload is owned storage in the dynamic region. A value type has no host, no anchor, no destruction tracking, and is copied by a mechanical inline write. That copy would have to either duplicate the handle, leaving two values naming one payload with nothing hosting it, or duplicate the payload, making assignment allocating and tree-sized. The first aliases a value, which is exactly what concurrency.md §4 relies on being impossible; the second is not a value copy. Full reasoning is in the reply on the §1 bullet.
If you want value recursion anyway, the thing to attack is the copy rule, not this section — a value sum could recurse if a value copy were allowed to be deep. That is a much bigger change (it gives value types a teardown obligation and makes assignment fallible), so I have not touched it. Say the word and I will open an issue rather than bury it here.
Generated by Claude Code
There was a problem hiding this comment.
Correcting my own reply above: "it gives value types a teardown obligation" is wrong and withdrawn, along with a second objection I raised after it. Both are now tracked in #156.
- Teardown. I conflated the anchor tracking a reference type needs — so a guest can tell whether its host died — with knowing when a value dies. The latter is lexical (
lifetimes.md§2.1 plus overwrite), so the teardown walk is code emitted at a known point, andmemory.md§3.2 already specifies that exact recursive block-return for reference hosts. Nothing new to invent, and a loop does not leak: overwrite frees and immediately repops the same exact-size stack. - The concurrent snapshot. I then argued it was use-after-free needing hazard pointers or epoch reclamation. That assumed freeing can unmap memory, which this arena never does while a reader could run — §4.1's water tower keeps the scope alive and §3.2 unmaps only at drain, so a freed block is recycled inside a live mapping. A stale read yields garbage, not an invalid access, which is the same failure class the flat case already retries through.
What actually survives: an allocating O(structure) =, and for spawn readers a bounded walk with offset validation, allocator contention on the per-scope size stacks, and O(structure) retry cost. Real, but ordinary — not a reclamation scheme.
Nothing here changes this PR; adt.md §4 and memory.md §2.10 describe the language as it stands. Flagging it so the thread does not leave a withdrawn argument standing as the reason.
Generated by Claude Code
There was a problem hiding this comment.
You were right, and my "no" is now reversed in the spec. 804631f and d0cd470.
Once both of my objections were withdrawn there was nothing holding the ban up — what was left was a cost list, and I had been reading it as an impossibility. So:
type Nat = variant { zero Unit; succ Nat; } // now legal
type Peano = #variant { zero Unit; succ Peano; } // still legal
A value copy is deep (memory.md §2.3). Copying a value copies its inline bytes plus a fresh allocation and recursive copy of every boxed payload it owns, so two values never share a node. A value with no boxed member — every value type anyone has written — is still copied by copying its bytes, unchanged. Boxing itself is now available on both sides of the # axis (memory.md §3.3): in a reference type the boxed member hosts its child, in a value type the value owns it outright.
The #-field and &-field bans stay, on your reason rather than the one the spec had. "Copying would silently duplicate hosting" was vague; a reference type exists in order not to be copied is the actual argument, and it also explains why a List field stays out. memory.md §2.10 now states it in those terms — duplicate the object and you mint a second identity that guests do not follow, share it and hosting is no longer single.
What # decides is therefore smaller than it was: identity, aliasing, and copy-versus-move. Not whether a type may contain itself — that turned out to be a layout question, and layout answers it the same way for both kinds.
Costs are stated where they land rather than buried. Copying such a value allocates and is O(structure), and a spawn snapshot of one has to validate offsets and stop at a depth bound, allocates from the scope's size stacks, and retries at O(structure) (concurrency.md §4.4). Two chapters record how we got here — "What a copy is for, and the ban that survived it" and "The sum that could not contain itself" — including the fact that the merged claim transitively-value therefore non-recursive was a non sequitur we had not examined.
#156 stays open for the part I did not do: whether a deep-copied List inside a value type should be legal, and whether # reduces to identity and aliasing alone. That reaches foundations.md and wants its own change.
Generated by Claude Code
| A block never grows in place across a chunk boundary, and an oversized span is never extended in place: further growth relocates into a doubled oversized span after checking that exact-size stack first. Relocation moves or copies elements according to their type's ordinary move rules; the old block becomes reusable only after its previous occupants are no longer live. Guests to the list remain valid because they reach the list's host, whose fixed-size handle now names the current backing store. | ||
|
|
||
| Dynamic chunks, ordinary power-of-two blocks, and oversized spans begin at cache-line-aligned addresses. Because the minimum block is 128 bytes and every larger block doubles, frontier allocations, reused blocks, and dedicated spans preserve cache-line alignment without mixing backing stores into fixed-size chunks. | ||
| A **boxed hosting member** (§3.3) uses the same two-part representation with a payload that never grows. Its handle records the payload's segmented offset; the payload is one reference-type instance, so it is allocated at **exactly that type's size**, aligned to that type's alignment requirement, and is returned to the size stack for that byte size when the member's occupant is destroyed or the member is overwritten (§3.2). Nothing is rounded up: a boxed payload has no size class, because a class exists to absorb growth and a boxed payload never grows. A payload larger than 1 MiB is a dedicated oversized span like any other. None of the growth rules above apply to it: a boxed payload is allocated once at its exact size and is only ever relocated by rehosting (§3.5). |
There was a problem hiding this comment.
i feel like we're repeating ourselves often. there should be one source of truth which should simple be referenced when needed
There was a problem hiding this comment.
Agreed, and it was worse than repetition — consolidating it exposed a contradiction. Fixed in 68bc769.
The exact-size rule was stated three times, with the rationale twice:
- §3.2 ¶3 — the rule, plus why a growable store's power-of-two sizes are a consequence of doubling rather than a classification;
- §3.6 ¶ here — the same sizing and alignment restated;
- §3.6 ¶ following — a whole paragraph re-arguing why a boxed payload has no class;
- §3.6 ¶ after that — the alignment rule a third time, plus frontier rounding already given in §3.2 ¶2.
§3.2 is "Allocation, reuse, and teardown", so it owns sizing, alignment and reuse. §3.6 now describes only what is specific to the two-part handle/payload representation and defers: "the payload is one reference-type instance, sized and aligned as §3.2 specifies". The re-argument paragraph is gone — the rationale lives in §3.2 and, at length, in the story chapter.
The contradiction: 2891b02 rekeyed the reuse stacks to (size, alignment) in §3.2 ¶2, but ¶4 still read "The stacks are keyed by byte size alone". Two paragraphs apart, flatly contradicting each other, and I would not have seen it while the rule was scattered over two sections. ¶4 now keys on both.
Also dropped "size class" from the two handle passages (§3.6 ¶1 and growth rule 5) in favour of "block size", since the region has no classes and the word was left over from when it did.
Generated by Claude Code
|
|
||
| ## The bindings that existed only to be pointed at | ||
|
|
||
| The fuse was not a size calculation. It was noticing what we had to write in order to use a variant at all. |
There was a problem hiding this comment.
i feel like this is a confusing introduction. maybe first start with what the issue was and then explain the change
There was a problem hiding this comment.
Fair — it opened by telling you what the fuse wasn't, which only lands if you already know the size argument that comes two paragraphs later. Reordered in 68bc769.
It now opens on the program that did not compile, says it was rejected and why, walks into Leaves, and only then names the fuse:
That is what lit the fuse — not a size calculation, but noticing what we had to write in order to use a variant at all.
So the negation arrives after you have seen both things it distinguishes, instead of before either. Nothing else in the chapter moved; the rest already runs problem → diagnosis → deeper mistake → fix → costs.
(The chapter is unmerged on this branch, so reordering it is not an append-only violation — the diff against main is still additions-only, verified.)
Generated by Claude Code
… rule Four review comments on #155. The reference requirement for a recursive sum was asserted rather than explained, and read as if it restricted value payloads generally. It does not: either form of `variant` carries value payloads freely. What a value sum cannot do is lead back to itself, because a box is owned storage and a mechanical value copy can neither share it nor duplicate it. memory.md §2.10 now gives that reason and carries an illegal recursive example; adt.md §1, §3 and §4 and the glossary entry follow. The `Expr` example never demonstrated the recursion rule on its own — its `#` is already forced by `intLit String`. §3 now isolates the rule with a value-payload-only pair and says so. The exact-size boxed-payload rule was stated three times across memory.md §3.2 and §3.6, with the rationale twice. §3.2 owns it; §3.6 references it. Consolidating surfaced a contradiction introduced in 2891b02: §3.2 keyed the reuse stacks by (size, alignment) in one paragraph and "by byte size alone" in the next. Also drops the "size class" wording from the two handle passages, since the region has no classes. The recursion story chapter opened by naming what the fuse was not. It now opens with the program that did not compile.
The previous wording jumped straight to ownership and asserted "no finite inline layout" without saying why the layout has to be inline. §2.10 now starts where the reader is: a value type is stored inline in its entirety, holds no handle, and names no dynamic-region payload, so its size is fixed by its parts and a self-reference makes the equation unsolvable — size(Nat) = tag + size(Nat). The ownership argument then answers the obvious follow-up rather than standing in for it: a handle would settle the size, but the payload it names needs something to free it, relocate it on a move, and define its copy — which is the hosting machinery that makes a type a reference type.
adt.md §4 said a hosting member is boxed "exactly when" its edge lies on a cycle, and that a member off one "is laid out inline as usual". The biconditional was stronger than anything this change needs: it forbids an implementation from boxing a member it has good reason to box, most obviously a sum whose widest case dwarfs its common ones — the layout that otherwise costs every instance the fat case's footprint. Nothing observes the difference. No operator exposes a type's size (`sizeof` appears only as prose describing `Array<T, n>`), and §3.5 already states that placement is not a language-visible property. The choice is made per type rather than per instance, so uniform stride (generics.md §7) is untouched either way. Boxing is now required where no finite inline layout exists and permitted elsewhere. memory.md §3.3 and the §6 summary row follow, and glossary §3.39 is restated around the required/permitted split rather than the cycle condition alone.
A value type may now own a boxed member, which makes a value `variant`
recursive: `variant { zero Unit; succ Nat; }` is legal. The indirection is
the ordinary boxed member — placement, not a reference-type field — and a
value copy is deep, so a copy allocates its own payloads and two values
never share a node.
The `#`-field and `&`-field bans stay, restated on the reason that actually
carries them: a reference type exists in order *not* to be copied. It has
one host, a stable identity, and is moved rather than duplicated, so a value
containing one could only mint a second identity or leave two values sharing
a host. `List` and `String` are reference types and are covered by that.
`#` therefore decides identity, aliasing, and copy-versus-move — not whether
a type may contain itself.
Costs are stated where they land: copying such a value allocates and is
O(structure); a `spawn` snapshot of one is a walk that must validate offsets
and stop at a depth bound, allocates from the scope's size stacks, and
retries at O(structure).
Generalizes "boxed hosting member" to "boxed member" across the spec, since
boxing is now available on both sides of the `#` axis.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017AYiJXjzCPEDfYRW1ZtxH7
Two new chapters, plus the spec-side integration. stories/memory.md — "What a copy is for, and the ban that survived it". Records the two arguments we withdrew (a teardown obligation values would need; a concurrent snapshot that was supposedly use-after-free), why each imagined machinery this runtime already has, and that what remained was a price list rather than a rule. Then the line that did not move: a reference type exists in order not to be copied, which is a sharper reason for the `#`-field ban than the one it replaces. stories/adt.md — "The sum that could not contain itself". Names the claim in the merged chapter "One body, product or sum" that stopped being true — transitively-value does not imply non-recursive — and says what the sum mould looks like now that the two declarations differ only in copy-vs-move. Also corrects the "exactly when" wording in this branch's own unmerged recursion chapter, which still read as a biconditional after 257c8da relaxed the rule to required-on-a-cycle, permitted-off-one. Story pointers added at memory.md §2.3 and §2.10, concurrency.md §4.4, and adt.md §3 and §4; both README stories-table rows extended. In-prose spec references are commit-pinned to 804631f and verified to resolve. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017AYiJXjzCPEDfYRW1ZtxH7
&|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
spec/lifetimes.md (1)
35-42: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winAdd a
move-sourceglossary entry.spec/glossary.mdhas no entry for this normative term. Define#variantcase forms and link tolifetimes.md§1.2.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@spec/lifetimes.md` around lines 35 - 42, Add a normative “move-source” entry to spec/glossary.md, defining the term and covering direct host symbols, hosting verb results, and `#variant` case forms. Link the entry to lifetimes.md §1.2, including the distinction that only reference-sum `#variant` cases qualify.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@spec/concurrency.md`:
- Around line 129-132: Update the boxed-member snapshot walk to validate each
handle’s complete payload span before any typed dereference, including bounds,
payload size, live-region containment, and alignment. Abort the walk and retry
through the existing version-check path whenever validation fails, rather than
interpreting recycled bytes. Extend the summary near the existing walk rules to
explicitly include these payload-span checks.
In `@spec/foundations.md`:
- Around line 90-92: The descriptions conflate ordinary value-parameter
borrowing with deep-copy operations. In spec/foundations.md lines 90-92, remove
“passed” from the copy-operation list or limit it to explicitly copy-producing
bindings; in spec/adt.md line 169, replace “every argument pass” with stores
into fresh storage and payload construction, preserving ordinary argument
passing as read-only borrowing.
In `@spec/memory.md`:
- Around line 327-328: Update the fixed-size region bullet to limit boxed-member
handles to those materialized in scope-level slots. Clarify in the dynamic
region description that nested boxed handles remain within their owning dynamic
payload block.
- Line 345: Update the oversized-span description to say its handle stores the
exact block size rather than a size class, and include the alignment if that
metadata is stored by the handle. Keep the existing segmented-offset and
exact-size reuse behavior unchanged.
- Around line 389-392: Define boxed payload behavior from the member’s declared
type, not the enclosing type: in spec/memory.md at lines 389-392, preserve
separate reference-typed and value-typed rules for backpointers, guesting,
identity, deep copying, relocation, and destruction; update the corresponding
glossary definition in spec/glossary.md at lines 257-260 to match this canonical
distinction.
In `@stories/adt.md`:
- Line 142: Update the explanation in the “inlining is the wrong default”
passage to preserve the fixed-layout invariant: state that a variant has a fixed
size determined by its tag and largest case, and identify the actual failure as
an inline recursive cycle having no finite layout. Remove wording that implies
sum types generally lack a settled size.
---
Outside diff comments:
In `@spec/lifetimes.md`:
- Around line 35-42: Add a normative “move-source” entry to spec/glossary.md,
defining the term and covering direct host symbols, hosting verb results, and
`#variant` case forms. Link the entry to lifetimes.md §1.2, including the
distinction that only reference-sum `#variant` cases qualify.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: zane-lang/coderabbit/.coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 8057025d-d9bc-4e72-8524-239623a05bfa
📒 Files selected for processing (11)
README.mdspec/adt.mdspec/concurrency.mdspec/foundations.mdspec/glossary.mdspec/lifetimes.mdspec/memory.mdspec/syntax.mdspec/types.mdstories/adt.mdstories/memory.md
- A boxed payload's nature follows the **member's declared type**, not the enclosing type's kind. Off-cycle boxing lets a reference type box a value-typed member, and the box must not give that payload identity. memory.md §3.3 now splits the two questions — what the payload *is* (declared type: backpointer, identity, guestability) from what becomes of it on move/copy/death (enclosing kind: hosted, or owned and deep-copied). glossary.md §3.39 follows. - The snapshot walk must validate a whole **payload span**, not just an offset. An offset landing inside a live region does not prove a payload's worth of bytes fits there; concurrency.md §4.4 now requires base, size, and alignment to be checked before any typed read, and aborts the walk when the check fails. - Passing a value is not a copy. A value-type parameter is a read-only borrow (memory.md §2.9), so "argument pass" is dropped from the cost lists in adt.md §4, memory.md §2.10, and glossary.md §3.2; the copy sites are bindings into fresh storage. - memory.md §3.1: scope the fixed-size region to handles materialized in scope-level slots, and say that a handle nested inside a dynamic payload belongs to that payload's block; an oversized span's handle records its exact block size and alignment, not a "size class" the region no longer has. - stories/adt.md: the "inlining is the wrong default" passage read as if a sum had no settled size, contradicting fixed layout. A variant is its tag plus its widest case — one fixed size; what fails is following that chain around a cycle. - New glossary.md §3.41 `move-source`, for the term §1.2 gained a third form of in this PR. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017AYiJXjzCPEDfYRW1ZtxH7
|
Review addressed in Boxed payload semantics follow the declared member type ( The snapshot walk validates a payload span, not an offset (
Partly declined: the value-parameter conflation. The substance is right and is fixed — All three Generated by Claude Code |
|
It needs to be explained why recursive variants require to be boxed. As far as i understand it, it would also be possible to inline recursive variants, because all recursive variants have leafs at some point; they're not infinitely recursive. |
There was a problem hiding this comment.
(comment by ChatGPT)
I like the overall direction, including the order-independent cycle-boxing rule and allowing recursive value types through deep copy. Reviewing the current head (f6f7ca5), I found three load-bearing gaps that should be settled before merge:
-
Define direct initialization and overwrite ordering for deep values.
memory.md§2.3 says declarations, assignments, stores, and returns copy a value;adt.md§3 saysNat.succ(n)deep-copiesn; and a value-variant case form is explicitly not a move-source. Taken literally,two Nat = Nat.succ(Nat.succ(...))copies each inner temporary into its parent and then copies the completed temporary again intotwo, making ordinary recursive construction quadratic. The overwrite rule also returns the destination's owned blocks before the replacement becomes live, sox = xand an RHS projected from the destination need explicit copy-before-destroy semantics. Please specify direct construction/copy elision for fresh value-producing forms and state that RHS materialization completes before destination teardown, or define the intended alternative explicitly. -
Reconcile snapshot allocation with destination-scope placement.
concurrency.md§4.4 says a deep snapshot allocates from “the same per-scope size stacks the writer is pushing to,” butmemory.md§3.5 says a value copy allocates each boxed payload in the destination scope's dynamic region. Asnapbinding inside spawned work generally belongs to a different scope from the shared value being mutated. Either specify a source-scope staging allocation, or change the contention/synchronization rule to describe the actual destination stack. -
Validate discriminants before traversing stale bytes. The new span, alignment, live-region, and depth checks prevent invalid address walks, but a recursive
variantrequires reading its tag before the walker knows which child handles exist. Until the final version check succeeds, that tag is untrusted and may be torn or come from a recycled occupant. Require validation of every variant/enum discriminant—and any other structural metadata needed to locate child handles—before case dispatch or typed interpretation.
Non-blocking: the unrelated CLAUDE.md terminology guard is already acknowledged in the PR body; splitting it would keep this PR's scope cleaner.
|
Your second point is important and i dont think we should try to determine the destination scope, as that's not always possible. For example, consider an io dependent if statement: I think it should instead just start in the initial scope and just move during runtime, instead of trying to statically determine the destination scope. |
|
(comment by ChatGPT) Regarding Manuel's two surrounding comments: Why recursive members cannot stay inlineThe fact that each runtime value eventually reaches a leaf does not give the type a finite inline size. Layout has to be fixed before any value exists and cannot depend on its eventual depth. For a declaration such as: an inline layout would require the storage for It would be possible only under a different language model where values have runtime-dependent sizes and containers store variable-stride elements. That would contradict Zane's fixed-layout and uniform-stride foundations. The PR should explain this distinction explicitly: the leaf terminates a value, not the recursive type's compile-time layout. Runtime destination scopeI agree that the runtime does not need to predict which control-flow branch will execute. A value can begin in the arena where it is constructed; when a particular destination store executes, its owned blocks can be relocated or deep-copied into that destination's arena at runtime. “Destination scope” should describe the destination of the store that actually executes, not a scope selected by compile-time control-flow prediction. There is a separate conflict with the current lifetime rules, however: the example moves For the snapshot finding in my review, the narrower issue remains: a deep snapshot is a copy into its fresh snapshot binding, so its blocks should belong to that binding's scope. The current statement that the reader allocates from “the same per-scope size stacks the writer is pushing to” appears to place them in the source/writer arena instead. The spec should either state why reader and writer necessarily share an arena or change this to the snapshot destination's arena. Attribution: This comment and review |
Review asked directly why a recursive variant must box, given that every value bottoms out at a leaf. The spec only asserted that an inline self-reference "would have infinite size", which does not answer it. adt.md §4 now separates the two things being conflated. A recursive type's *values* are finite; layout is fixed per type and settled before any value exists, so the question is what one stride must be for every value at once. `size(Nat) = tag + size(Nat)` has no finite solution, and no stride can be taken from the deepest value because there is no deepest value — one `Nat` holds three nodes and the next three million, and uniform stride requires both to be the same size. The leaf terminates a value, not the type's size equation. Also reconciles two passages that #158 left describing the pre-#158 copy rule, now that a fresh non-place result constructs directly in its destination: - adt.md §4 said a recursive value type walks and reallocates on *every* store into fresh storage. That is now true only for binding an existing place; `Nat.succ(Nat.succ(...))` builds each node once where it will live. - memory.md §6 said a value parameter is "copied when bound into a fresh slot", which reads as though passing were the copy. The copy happens when the parameter, an existing place, is itself bound onward. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017AYiJXjzCPEDfYRW1ZtxH7
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
spec/memory.md (1)
419-419: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winQualify when values are copied instead of rehosted.
Section 3.3 makes the enclosing type control movement of a boxed payload. Section 3.5 also relocates every block owned by a reference host. The unqualified sentence at Line 419 can make an off-cycle value-typed boxed member appear to deep-copy during reference-host rehosting. Limit this rule to a value that is itself copied into a new slot.
Proposed clarification
-A value reaches a new scope by being copied rather than rehosted, and the same recursion applies to its blocks: +When a value itself is copied into a new slot, it reaches the new scope by copying rather than rehosting, and the same recursion applies to its blocks:🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@spec/memory.md` at line 419, Revise the value-copying statement in Section 3.3 so it applies only when the value itself is copied into a new slot or scope. Clarify that this rule does not govern boxed members relocated during reference-host rehosting, whose movement follows the enclosing type and ownership rules in Sections 3.3 and 3.5.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@spec/adt.md`:
- Line 179: Clarify the recursive value-copy rule in spec/adt.md at line 179 by
qualifying “a return of one” as returning an existing value or place, while
preserving that fresh non-place results construct directly. Apply the same
clarification to the “return stores” wording in stories/adt.md at line 171,
specifying that these stores originate from existing values.
In `@spec/concurrency.md`:
- Around line 133-134: Update the depth-bound rules so exhaustion is a terminal
snapshot failure after provisional allocations are cleaned up, rather than a
retryable stale-data failure. In spec/concurrency.md lines 133-134, state this
cleanup and terminal behavior; update the retry summary at spec/concurrency.md
lines 191-191 to limit retries to retryable validation or version failures;
align stories/memory.md lines 225-225 with the same terminal depth-failure rule.
---
Outside diff comments:
In `@spec/memory.md`:
- Line 419: Revise the value-copying statement in Section 3.3 so it applies only
when the value itself is copied into a new slot or scope. Clarify that this rule
does not govern boxed members relocated during reference-host rehosting, whose
movement follows the enclosing type and ownership rules in Sections 3.3 and 3.5.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: zane-lang/coderabbit/.coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 4115e959-b5bc-4326-b595-f75886335ef2
📒 Files selected for processing (6)
spec/adt.mdspec/concurrency.mdspec/glossary.mdspec/memory.mdstories/adt.mdstories/memory.md
| - **The walk MUST be bounded, and MUST validate a whole payload span.** Because a recycled block may hold a handle left by its next occupant, a reader may pick up an offset that is not part of the structure it is traversing. Before interpreting what a handle names, the walk **MUST** check that the handle's **complete payload span** — its base offset, plus the size of the member's declared type, at that type's alignment — lies within a live region of the scope, and it **MUST** stop at a depth bound. The depth bound applies per snapshot attempt: every retry starts a new walk with the same bound. An offset that merely lands in a live region is not enough: a recycled block can hold one near a region's end, and reading a payload's worth of bytes from there would run past it. A handle failing either check aborts the attempt rather than being followed. | ||
| - **The reader allocates in the destination scope.** A deep snapshot is an ordinary deep value copy into its fresh destination binding (see [`memory.md`](memory.md) §2.3). Each boxed payload is therefore allocated from the size stacks of the scope that owns that binding, not generally from the writer's or source value's scope. Snapshotting introduces no special source-scope staging and does not by itself make the reader and writer contend on one stack. Allocator synchronization is required only when concurrent work actually shares an underlying arena. Every block allocated by a snapshot attempt remains **provisional** until the final version check accepts that attempt. If metadata validation, span or depth validation, or the final version check rejects the attempt, the runtime **MUST** return every block allocated by that attempt to the destination scope's corresponding size stacks before retrying. The destination binding becomes live only after the attempt is accepted. |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
Make depth exhaustion a terminal result instead of a retry.
All three passages treat a depth-limit failure like stale data. A valid value deeper than the bound can never succeed, so the reader can retry indefinitely.
spec/concurrency.md#L133-L134: define depth exhaustion as a terminal snapshot failure after provisional cleanup.spec/concurrency.md#L191-L191: update the summary so retries apply only to retryable validation or version failures.stories/memory.md#L225-L225: align the story with the terminal depth-failure rule.
📍 Affects 2 files
spec/concurrency.md#L133-L134(this comment)spec/concurrency.md#L191-L191stories/memory.md#L225-L225
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@spec/concurrency.md` around lines 133 - 134, Update the depth-bound rules so
exhaustion is a terminal snapshot failure after provisional allocations are
cleaned up, rather than a retryable stale-data failure. In spec/concurrency.md
lines 133-134, state this cleanup and terminal behavior; update the retry
summary at spec/concurrency.md lines 191-191 to limit retries to retryable
validation or version failures; align stories/memory.md lines 225-225 with the
same terminal depth-failure rule.
Two review findings, both places where an unqualified sentence over-reaches now that a fresh non-place result constructs in its destination. - adt.md §4 and stories/adt.md listed "a return" among the O(structure) operations. A return of an existing value is one; a return of a freshly built result constructs into the return slot and costs nothing extra. Both now say the source must be an existing value. - memory.md §3.5 said "a value reaches a new scope by being copied rather than rehosted", which read as though a value-typed payload owned by a reference host would be deep-copied when that host is rehosted. It is relocated with its host: what becomes of a boxed payload follows the enclosing type's kind (§3.3). Scoped to the value that is itself copied. The third finding of the round — that depth exhaustion should be a terminal snapshot failure rather than a retry — is a design call and is raised on the thread rather than decided here. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017AYiJXjzCPEDfYRW1ZtxH7
|
Round addressed in Taken: the return-store qualification ( Taken: Raised, not taken: depth exhaustion as a terminal failure. The hole is real. A legal value deeper than the bound fails validation on every attempt, so "retry" never converges — that is a livelock, not a stale read, and the three passages do currently lump it in with torn data. But I don't think the proposed fix can be applied as a wording change, because it adds an observable failure mode that Three ways out, as I see them:
I lean toward 2 — it is the only one that does not add a failure where the spec promised none, and the depth count is cheap next to the allocation a deep copy already does. But this changes what All three Generated by Claude Code |
TheLazyCat00
left a comment
There was a problem hiding this comment.
please make sure that the spec doesnt repeat itself. if there are two places where something is explained, it could happen that they drift apart
| A plain `variant` is a **value** sum: copied on assignment and transitively value. A `#variant` is a **reference** sum: it has identity, and may hold reference-type and `&` payloads. **Both may recurse** (§4); what separates them there is not whether the recursive child is allowed but what owning it means: | ||
|
|
||
| ```zane | ||
| type Nat = variant { zero Unit; succ Nat; } // legal: `succ` is a boxed member the value owns |
There was a problem hiding this comment.
why do we use Nat as the name for this example?
| A value type is copied on assignment, has no identity, and — the load-bearing restriction — is *transitively* a value: it may contain only other value types, never a reference-type or `&` field. Nothing reachable from a value can be aliased, which is why a value can be copied and shared by snapshot with no bookkeeping, and why a value type cannot recurse (a self-reference would need indirection, and indirection is a reference). A reference type is the opposite in each respect: it has stable identity, may be aliased through `&`, may hold reference-type and `&` fields, and may recurse. | ||
| A value type is copied on assignment, has no identity, and — the load-bearing restriction — is *transitively* a value: it may contain only other value types, never a reference-type or `&` field. Nothing reachable from a value can be aliased, which is why a value can be copied and shared by snapshot with no bookkeeping. A reference type is the opposite in each respect: it has stable identity, may be aliased through `&`, may hold reference-type and `&` fields, and reaches a new place by being moved rather than duplicated. | ||
|
|
||
| The two halves of that are the same fact seen twice. A value is copied, so it cannot hold a reference type — a type that exists in order *not* to be copied, since copying one would either mint a second identity or leave two values sharing a host ([`memory.md`](memory.md) §2.10). What the closure does **not** decide is recursion. Either kind may contain itself, because the indirection that makes a self-reference finite is a **boxed member** — out-of-line placement of the member's own type, not a reference-type field — and a value's copy is **deep**, so a recursive value still shares nothing with its copies ([`adt.md`](adt.md) §4, [`memory.md`](memory.md) §2.3). The price is that copying such a value allocates and walks its structure, where copying a flat value is one fixed-size write. |
There was a problem hiding this comment.
i feel like we're saying too much stuff here, there should simply be one source of truth that explains variant and its boxing behaviour instead of explaining it in multiple places
|
|
||
| ### 3.2 value-downstream enforcement | ||
| - **Meaning:** A value type may contain only primitives and other value types, never a reference (`#`) or `&` field anywhere downstream in nested value-type fields. | ||
| - **Meaning:** A value type may contain only primitives and other value types, never a reference (`#`) or `&` field anywhere downstream in nested value-type fields. The reason is copying: when an existing place is bound into a different slot, its value is copied whole; a fresh non-place expression instead constructs directly in its destination. A reference type exists in order *not* to be copied — it has one host, a stable identity, and reaches a new place by being moved. Copying a value that held one could only mint a second identity or leave two values sharing a host; an `&` field would put aliasing inside the value world directly. The closure does **not** bar recursion: a value type may lead back to itself through a boxed member (§3.39), because a box is placement rather than a reference-type field and a value copy is deep (§3.40). |
There was a problem hiding this comment.
here too: is the glossary the right place to explain it or would it be better to simply link it?
Review: the spec was explaining the same two rules in several places, which is how two statements drift apart. An audit found the boxing rule and the deep-copy rule each argued at length in four or five files. Canonical homes, unchanged from what the glossary already declared: recursion and boxing in adt.md §4, the deep value copy in memory.md §2.3, value-downstream enforcement in memory.md §2.10, representation in memory.md §3.3 and §3.6. Everywhere else now states its own point and points there. - foundations.md §7: the re-argument of the closure and the boxed-member reasoning is gone. This document says outright that it introduces no rules of its own, so it now records only that the axis does not decide recursion, and links adt.md §4. - glossary.md §3.2, §3.39, §3.40: cut to summaries. The file's own contract is "a short summary, not the full rule", and all three had grown into full explanations — §3.39 to over 1200 characters. - adt.md §4: keeps the recursion rule, which is its own, and stops re-deriving what a deep copy does. - memory.md §2.10: keeps why a boxed member does not violate the closure, which is this rule's business, and drops the boxing representation. - types.md §2.2: recursion sentence reduced to a pointer. No rule changed; each is now stated once. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017AYiJXjzCPEDfYRW1ZtxH7
|
All three duplication points taken, in The general rule first. Canonical homes, which the glossary had already declared and the prose had quietly stopped respecting: recursion and boxing in
Also trimmed for the same reason: On
Two things are wrong with it though, and the second is the real one:
So I'd rather change it than defend it. Options, and it is your call on taste: I lean toward A: it drops the background assumption and the two names no longer pretend to be different concepts. Tell me which and I'll change it in Guards clean at Generated by Claude Code |
|
yes, i option A seems to be the best. also, the rule for one canonical home is in the contribution guidelines right? otherwise we'd have to state it there too |
Review asked why the example was called Nat. It was the Peano encoding,
picked because it is the smallest recursive type whose payloads are all
value types — the shape the example has to isolate, since Expr cannot
demonstrate the recursion rule at all.
Two problems with it, and the second is the real one. The name assumes
type-theory background, and `Nat` and `Peano` are the same concept under
two names — "Peano" being the mathematician attached to `Nat`'s encoding —
so the pair read as two different ideas when the point is that they are one
structure differing only in `#`.
Now:
type Countdown = variant { done Unit; more Countdown; }
type Chain = #variant { done Unit; more Chain; }
Same minimal shape, no background assumed, and the two names no longer
pretend to be unrelated. Renamed across adt.md §3 and §4, memory.md §2.3
and §2.10, and both story chapters on this branch.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017AYiJXjzCPEDfYRW1ZtxH7
|
Option A applied in Renamed across On the guidelines — it is already there, so nothing to add.
and §1 says the same from the other side: "If you need to mention a concept that is canonically specified elsewhere, keep the mention brief and add a cross-reference rather than restating it." So I broke a rule that was already written down, rather than one that needed writing. Worth saying plainly, because it changes what the fix is: adding a fresh statement of the rule to the guidelines would be its own small instance of the problem — the same instruction in two places, free to drift. The guide is fine; the prose drifted away from it over many edits, each of which looked locally reasonable. If you want a belt-and-braces guard, the thing that would actually catch it is mechanical rather than prose — something like flagging any passage outside a rule's canonical home that grows past a length threshold while restating it. That is not something a grep does well, and I would not add it to this PR, but it is the shape of what would have caught this drift. Guards clean at Still open for you: the depth-bound question from two comments up, the four original review threads, and whether to split Generated by Claude Code |
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
spec/memory.md (1)
402-402: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winDefine guest creation from boxed variant payloads.
spec/adt.mddefines variant-member reads as partial and abortable. The boxed-member rule says a reference-typed boxed payload may be guested from an enclosing field access, but it does not say whether a live live-case read such asExpr.opcan mint&Operation, or whether that abortable access is a valid guest source. Specify this contract before using boxed variant payloads as guest sources.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@spec/memory.md` at line 402, Clarify the boxed variant payload rule in the guest-creation section around the member payload semantics: define whether a live-case variant-member read such as Expr.op may create a guest reference to a reference-typed boxed payload, and whether the partial/abortable read is an admissible guest source. State the required behavior for absent or invalid cases, and align the wording with the variant-member read rules in adt.md.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@spec/adt.md`:
- Around line 174-177: Define the containment-cycle predicate using only
owning/hosting edges, excluding explicit & guest members because their
fixed-size storage does not require boxing. Update the boxing rules in
spec/adt.md lines 174-177, spec/glossary.md lines 257-260, and spec/memory.md
lines 398-399 to consistently use this predicate; all three sites require the
corresponding wording change.
In `@stories/memory.md`:
- Line 225: Update the canonical snapshot rule in concurrency.md §4.4 to
explicitly define the observable result when the per-attempt depth bound is
reached: abort the attempt, roll back its allocations, and retry rather than
returning an incomplete snapshot. Align the wording in the memory story’s
snapshot description with that rule.
---
Outside diff comments:
In `@spec/memory.md`:
- Line 402: Clarify the boxed variant payload rule in the guest-creation section
around the member payload semantics: define whether a live-case variant-member
read such as Expr.op may create a guest reference to a reference-typed boxed
payload, and whether the partial/abortable read is an admissible guest source.
State the required behavior for absent or invalid cases, and align the wording
with the variant-member read rules in adt.md.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: zane-lang/coderabbit/.coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 5b7e7bd1-a507-43b2-809c-376cfa9c89d5
📒 Files selected for processing (7)
spec/adt.mdspec/foundations.mdspec/glossary.mdspec/memory.mdspec/types.mdstories/adt.mdstories/memory.md
| - **Boxing is required on a cycle and permitted off one.** A member **MUST** be boxed when its declared type can lead back to the enclosing type — when its edge lies on a cycle in the containment graph — because no finite inline layout exists for it. Every such edge is boxed, so nothing depends on declaration order or on choosing where to cut the cycle: above, `Expr.op`, `Operation.left`, and `Operation.right` are all boxed, and every type on the cycle has a finite, statically known size. Off a cycle, an implementation **MAY** box or inline as it judges best. Inline is the ordinary choice, but a sum whose widest case dwarfs its common ones is the case an implementation may want to place out of line, and nothing here forecloses that. The choice is made per type, not per instance, so every value of a type is still the same size and uniform stride holds (see [`generics.md`](generics.md) §7); and a program cannot observe which was chosen, because placement is not a language-visible property (see [`memory.md`](memory.md) §3.5) and no operator exposes a type's footprint. | ||
| - **Nothing is written for it.** The programmer writes `left Expr`; the compiler boxes it because inline placement is impossible, exactly as it places list elements out of line. There is no `Box<T>` type and no marker, because placement was never a language-visible property (see [`memory.md`](memory.md) §3.5). | ||
| - **Both kinds may recurse.** A recursive value type is legal, because a box is placement rather than a reference-type field. The body syntax is symmetric across all four kinds, and so is recursion: `#` decides identity, aliasing, and copy-versus-move, not whether a type may contain itself. What a copy of a recursive value costs, and why it shares no node with its original, is the deep-copy rule (see [`memory.md`](memory.md) §2.3). | ||
| - **`&` is for aliasing, not for recursion.** A guest expresses non-hosting access — a parent back-pointer, a symbol table naming nodes, a genuine graph edge — and a cycle of guests is a shape hosting could not express in the first place. An `&` member follows the ordinary guest rules, including the guest-source restriction (see [`memory.md`](memory.md) §2.8); an owning member, boxed or not, does not. |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== files =="
git ls-files | rg '(^|/)spec/(adt|memory|glossary)\.md$|(^|/)stories/.*\.md$' || true
echo "== adt relevant =="
if [ -f spec/adt.md ]; then
sed -n '1,240p' spec/adt.md | nl -ba | sed -n '130,210p'
fi
echo "== glossary boxed member =="
if [ -f spec/glossary.md ]; then
sed -n '220,280p' spec/glossary.md | nl -ba | sed -n '1,80p'
fi
echo "== memory boxing relevant =="
if [ -f spec/memory.md ]; then
sed -n '350,440p' spec/memory.md | nl -ba | sed -n '1,120p'
fi
echo "== search ownership/guest terms =="
rg -n '"(owning|guest|hosting|boxed|&T|&)"|ownership|guest-source|guest source|&T|& ' spec/adt.md spec/glossary.md spec/memory.md spec/*.md 2>/dev/null | head -200 || trueRepository: zane-lang/spec
Length of output: 578
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== adt relevant =="
sed -n '130,210p' spec/adt.md
echo "== glossary boxed member =="
sed -n '220,280p' spec/glossary.md
echo "== memory boxing relevant =="
sed -n '350,440p' spec/memory.md
echo "== search ownership/guest terms =="
rg -n '"(owning|guest|hosting|boxed|&T|&)"|ownership|guest-source|guest source|&T|& ' spec/adt.md spec/glossary.md spec/memory.md spec/*.md 2>/dev/null | head -200 || true
echo "== adt outline if available =="
ast-grep outline spec/adt.md --view compact 2>/dev/null || trueRepository: zane-lang/spec
Length of output: 50372
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
files = [Path("spec/adt.md"), Path("spec/glossary.md"), Path("spec/memory.md")]
needles = [
"declared type can lead back to the enclosing type",
"where the member's declared type can lead back to the enclosing type",
"A boxed member is laid out the same way. A member whose declared type can lead back to the enclosing type",
"contains back to",
"&",
"guest",
"hosting",
"owning",
"boxed member",
]
lines = {p: tuple(p.read_text().splitlines()) for p in files}
for p in files:
print(f"\n-- {p} --")
for i, text in enumerate(lines[p], start=1):
if any(needle in text for needle in needles):
print(f"{i}: {text}")
for p in files:
txt = "\n".join(lines[p])
print(f"\n{p}: boxed-member occurrence count =", txt.count("boxed member") + txt.count("boxed Member") + txt.count("BOXED MEMBER"))
print(f"{p}: ownership/containment phrased as containing guest edges? any guest containment cycle rule?")
for needle in ["cycle of guests", "cycle of guests", "guest containment", "guest graph", "containment graph", "containment cycle"]:
print(" ", needle, "=>", txt.count(needle))
PYRepository: zane-lang/spec
Length of output: 50370
Define boxing by owning/hosting edges only.
& storage is fixed-size guest-only storage, so an explicit & back-edge must not trigger the required boxing rule. Define one containment-cycle predicate that covers owning/hosting members, then use it in:
spec/adt.md#L174-L177spec/glossary.md#L257-L260spec/memory.md#L398-L399
📍 Affects 3 files
spec/adt.md#L174-L177(this comment)spec/glossary.md#L257-L260spec/memory.md#L398-L399
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@spec/adt.md` around lines 174 - 177, Define the containment-cycle predicate
using only owning/hosting edges, excluding explicit & guest members because
their fixed-size storage does not require boxing. Update the boxing rules in
spec/adt.md lines 174-177, spec/glossary.md lines 257-260, and spec/memory.md
lines 398-399 to consistently use this predicate; all three sites require the
corresponding wording change.
|
|
||
| That does not mean every value-producing expression first creates a temporary and then copies it. A place expression denotes an existing value and is copied when bound elsewhere; a fresh non-place result constructs directly in its eventual destination, recursively through product members, value-variant payloads, function results, and `match` arms. Without that distinction, `Countdown.more(Countdown.more(...))` would repeatedly copy every completed prefix and ordinary construction would become quadratic. The same destination rule is what makes `Vector2(Int(3), Int(4))` construct the vector and both integers once, where they finally live. Overwrite carries the matching order rule: if the right-hand side reads the destination or anything reached through it, the replacement is completed before the old occupant and its blocks are destroyed. `x = x` and `x = x.child` therefore cannot erase their own source. | ||
|
|
||
| The residuals we do accept, and they are stated where they land rather than buried. Copying an existing boxed value allocates and walks its structure. A `spawn` reader taking a snapshot of one must treat every byte read before the final version check as untrusted: it validates a sum discriminant before dispatch, validates any count or length that directs traversal, validates every complete payload span, and stops at a per-attempt depth bound. Every retry starts a new walk with the same bound. An invalid value aborts the attempt; a valid-but-stale combination remains memory-safe and is discarded by the final version check. The snapshot is an ordinary deep copy into its fresh binding, so its blocks come from that destination scope's stacks, not generally from the writer's; the two contend on one allocator only when they actually share an arena. Those allocations stay provisional until the final version check accepts the attempt. If validation or that final check rejects it, every block allocated by the attempt is returned to the destination scope's corresponding exact-size stacks before the reader retries, so failed walks cannot accumulate dead snapshot storage. A retry still redoes the whole walk, so a fast writer can starve a reader in a way it cannot for a flat value ([`concurrency.md` §4.4](https://github.com/zane-lang/spec/blob/b1abb87712607119a4f6d3962e1baeb22a2367ca/spec/concurrency.md#44-reads-take-a-coherent-snapshot)). That cleanup is ordinary allocator rollback, not a reclamation scheme for writer memory; the concurrency rule is still validating an optimistic walk. |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Files matching relevant names:"
fd -a '^(memory|concurrency)\.md$' . | sed 's#^\./##'
echo
echo "Check git tracked matching files:"
git ls-files '*/memory.md' '*concurrency.md' | sed 's#^\./##'
echo
echo "Context around stories/memory.md line 225:"
if [ -f stories/memory.md ]; then
nl -ba stories/memory.md | sed -n '205,245p'
fi
echo
echo "Search per-attempt depth bound references:"
rg -n "depth|bound|exhaust|snapshot|concurrency|§4\.4|4\.4" stories/memory.md spec/concurrency.md 2>/dev/null || true
echo
if [ -f spec/concurrency.md ]; then
echo "concurrency.md outline-like line numbers with relevant sections:"
nl -ba spec/concurrency.md | sed -n '1,260p' | rg -n -C 2 "depth|bound|snapshot|reader|version|retry|alloc|allocation|rollback|reject|stale|aborted|invalid|concurrent|reads|takes|coherent" || true
fi
echo
if [ -f spec/concurrency.md ]; then
echo "grep exact line locations:"
grep -n -i "depth\|bound\|snapshot\|reader\|version\|retry\|alloc\|rollback\|reject\|stale\|invalid\|concurrent\|read" spec/concurrency.md || true
fiRepository: zane-lang/spec
Length of output: 531
🌐 Web query:
zane-lang spec concurrency pinned anchor reads take a coherent snapshot
💡 Result:
There is no evidence in the Zane programming language specification (zane-lang) [1][2] regarding "pinned anchor reads" or "coherent snapshots" as part of its concurrency model. The terms "pinned," "coherent snapshot," and "concurrency" appearing together in technical contexts appear to be related to other domains, such as distributed systems snapshot protocols (e.g., in agent-coherence/agent-coherence [3][4]) or unrelated discussions regarding Rust/Zig pinning [5][6]. Zane is a recently created language (established March 2026) [1] and its documentation does not currently contain these specific concurrency-related primitives.
Citations:
- 1: https://github.com/zane-lang
- 2: https://github.laiyagushi.com/zane-lang
- 3: feat(coordinator): consistent multi-artifact snapshot sessions (read-side transaction layer) Cohexa-ai/agent-coherence#132
- 4: https://github.com/Cohexa-ai/agent-coherence/blob/main/formal/tla/Snapshot.tla
- 5: https://rust-lang.github.io/async-book/part-reference/pinning.html
- 6: Proposal: Pinned Structs ziglang/zig#7769
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Context around stories/memory.md line 225:"
if [ -f stories/memory.md ]; then
sed -n '205,245p' stories/memory.md | cat -n
fi
echo
echo "Search per-attempt depth bound references:"
rg -n "depth|bound|exhaust|snapshot|concurrency|4\.4|§4\.4" stories/memory.md spec/concurrency.md stories/concurrency.md spec/memory.md 2>/dev/null || true
echo
echo "Concise relevant concurrency.md sections:"
if [ -f spec/concurrency.md ]; then
sed -n '1,280p' spec/concurrency.md | cat -n
fi
echo
echo "Verify pinned GitHub anchor resolution:"
python3 - <<'PY'
import urllib.request, re, urllib.parse
sha = "b1abb87712607119a4f6d3962e1baeb22a2367ca"
url = f"https://raw.githubusercontent.com/zane-lang/spec/{sha}/spec/concurrency.md"
try:
text = urllib.request.urlopen(url, timeout=15).read().decode("utf-8", "replace")
except Exception as e:
print(f"fetch_error: {type(e).__name__}: {e}")
raise SystemExit(1)
anchors = re.findall(r'<a href="#([^"]+)">', text, flags=re.I)
targets = [m.group(1) for m in re.finditer(r'(?<=[ \t<\[])?(?:4\.4|section)[:\s]*(.+?)(?:\]|$)', text, flags=re.I+re.S)]
targets = [a.strip(" -:–—_.\"'\n") for a in targets if a.strip()]
print("sha:", sha)
print("found_anchors:", len(anchors))
print("contains_44:", "44-reads-take-a-coherent-snapshot" in anchors or any(re.search(r'(?i)reads\s+t[aT]ke\s+a\s+coherent\s+snapshot', a) for a in anchors))
print("anchor_candidates:", [a for a in anchors if "reader" in a.lower() or "snapshot" in a.lower() or "concurrent" in a.lower() or a.startswith("4")][:20])
PYRepository: zane-lang/spec
Length of output: 50370
🌐 Web query:
github zane-lang spec concurrency.md 4.4 reads take a coherent snapshot depth bound exhausted retry
💡 Result:
There is no official documentation or specific specification file named concurrency.md in the Zane programming language repository (zane-lang/spec) that contains a section "4.4 reads take a coherent snapshot depth bound exhausted retry" [1][2]. The phrase "reads take a coherent snapshot... depth bound exhausted retry" describes a common pattern in concurrent system design and database implementation, often found in projects handling atomic state snapshots [3][4][5]. In such systems: 1. Coherent Snapshot: To avoid reading inconsistent (torn) state when multiple values must be read together, a system captures an immutable snapshot or uses a validation protocol to ensure all data belongs to the same state version [3][4][6]. 2. Depth Bound: Because concurrent writers may frequently update the state, a reader might repeatedly detect an inconsistency. A "depth bound" (or attempt limit) is implemented to prevent indefinite spinning or livelock, forcing the operation to return an error if it cannot successfully capture a coherent snapshot within a set number of tries [3][4]. 3. Retry: The system attempts to re-read the data upon detecting a concurrent modification, typically by validating a sequence counter or revision number before and after the read [7][4][5]. While this logic appears in various technical contexts—such as the Zakura blockchain core [3][4], Linux kernel block drivers [5], and distributed data stores [7][8]—it is not a recognized feature of the Zane language specification. Zane is a developing language [9] with repositories for its compiler and specification [1], but it does not currently feature this specific concurrency pattern in its official documentation [1].
Citations:
- 1: https://github.com/zane-lang
- 2: https://github.laiyagushi.com/zane-lang/zane-cpp
- 3: bug: make multi-read RPC responses use atomic state snapshots zakura-core/zakura#360
- 4: fix(state)!: make gettxout use one coherent state view zakura-core/zakura#421
- 5: https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1561203.html
- 6: https://docs.rs/poise-discovery/latest/src/poise_discovery/snapshot.rs.html
- 7: https://github.com/Thejuampi/memris/blob/main/docs/CONCURRENCY.md
- 8: https://github.com/rqlite/rqlite/blob/f9d5a860/store/DESIGN.md
- 9: https://zane-lang.org/
Specify the result of snapshot depth exhaustion.
spec/concurrency.md §4.4 requires the snapshot walk to stop at a depth bound and treats handle offsets outside the payload span as aborts, but it does not define whether hitting the depth bound aborts and retries, fails the snapshot entirely, or returns a partial value. Add that observable behavior to the canonical concurrency rule so this story is not ambiguous about an unsafe or incomplete snapshot.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@stories/memory.md` at line 225, Update the canonical snapshot rule in
concurrency.md §4.4 to explicitly define the observable result when the
per-attempt depth bound is reached: abort the attempt, roll back its
allocations, and retry rather than returning an incomplete snapshot. Align the
wording in the memory story’s snapshot description with that rule.
The required-boxing predicate said a member must be boxed when its declared type "can lead back to the enclosing type", over "a cycle in the containment graph". Read literally that catches an `&` member too, since an `&Node` field's declared type does lead back — which would require boxing a guest. That contradicts the bullet three lines below it, where a cycle of guests is named as a legal graph and precisely the shape `&` exists for. The graph is now stated as one of **owning** edges: members that store an instance of their type, inline or boxed. A guest is fixed-size storage whatever it points at, so a cycle closed through `&` never triggers the rule. Stated in adt.md §4 and matched in glossary.md §3.39; memory.md §3.3 now defers to §4 for the predicate instead of restating it. Also narrows a claim in memory.md §3.3: a reference-typed boxed payload was said to be guestable "from the enclosing instance's field access under §2.8", which asserts more than §2.8 settles for a `#variant` case, whose read is partial and abortable. It now says such a payload may be guested wherever §2.8 admits the access that reaches it — boxing neither grants nor withholds that. Whether an abortable case read is itself a guest source is a pre-existing gap and is raised on the PR rather than decided here. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017AYiJXjzCPEDfYRW1ZtxH7
|
Taken: boxing must read owning edges only. This was a genuine self-contradiction, not a wording preference. The predicate said a member must be boxed when its declared type "can lead back to the enclosing type", over "a cycle in the containment graph" — and an The graph is now stated as one of owning edges — members that store an instance of their type, inline or boxed. A guest is fixed-size storage whatever it points at, so a cycle closed through Narrowed: guests from a boxed variant payload. The finding is right that the contract is undefined, but the fix is not mine to invent. The underlying question stands and predates this PR: is an abortable Already yours: depth exhaustion. This is the same issue I raised two comments up, now arriving from the story side. CodeRabbit proposes abort-rollback-retry, which is what §4.4 already implies — and that is precisely the livelock: a legal value deeper than the bound fails every attempt, so retrying never converges. Still the three-way choice I laid out (terminal failure and an abortable Guards clean at Generated by Claude Code |
|
both stories currently jump right into "the decision that x is..." and "heres an example:". i think they should have a better introduction or say why they are there in the first place |
Review: the new chapters start cold — one on "the decision that X is", another straight into "here is an example". The story guide asks a chapter to open "on the situation that forced a choice", and to carry the reader across the seam from the chapter before it. These four did neither; they opened on the conclusion or on code. Each now names the pressure first: - "The bindings that existed only to be pointed at" — the sum mould was settled long before anyone tried to build a tree with it; every rule involved looked reasonable alone and together made the ordinary thing unwritable. - "The sum that could not contain itself" — boxing settled layout but left one restriction standing, and it looked settled too until review asked why. - "The region takes the boxes" — a decision taken on the ADT side arrived here as a bill nobody had costed, and says what the chapter has to settle: address, size, and the invariant it disturbs. - "What a copy is for" — a rule can look finished and be resting on nothing; this one rested on two mistakes, and something was left standing after both were withdrawn. Also now links back to the chapter it follows. Prose only; no rule changed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017AYiJXjzCPEDfYRW1ZtxH7
|
Taken, in Each now leads with the pressure:
Worth noting the first of those had already been reordered once, after your earlier "start with what the issue was" comment. That fix moved the code to the front of the problem rather than the solution — but it still opened on an example, so it did not actually answer what you were asking. This one does. Prose only; no rule changed. Story append-only check against Generated by Claude Code |
TheLazyCat00
left a comment
There was a problem hiding this comment.
stories are ok, i dont really care about the spec although i looked at it a bit. the most common way to find contradiction is when working with the spec, reviewing wont do much anyway and isnt worth the time, this is a fast moving spec
The open question was what happens when the snapshot walk hits its depth bound. Review proposed making exhaustion terminal, which would give `snap` an error path §4.4 promises it never has. The alternative I raised needed the writer to maintain a depth count, which is its own cost. Neither is necessary, because a bound already exists that no correct walk can reach. A legal walk descends through a distinct live block at every step, so the number of live blocks in the scope's dynamic region when the attempt begins bounds any real structure's depth. Fix the bound there and exceeding it *always* means the walk is following recycled bytes — never that the value is honestly too deep — so discarding the attempt and retrying is correct, converges, and needs no new failure mode. The livelock the review was pointing at was real: an arbitrary bound makes a legal deep value fail every attempt. A derived one cannot. §4.4, its summary row, and the memory story now say this. Nothing the runtime does not already know is required, and no observable behaviour of `snap` changes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017AYiJXjzCPEDfYRW1ZtxH7
|
Understood — and on that basis I've closed the last open question myself rather than leaving it parked for a review round that isn't going to happen. The depth bound is now derived, so exhaustion stays retryable. Both options I put to you were worse than they needed to be: terminal failure gives The livelock the finding pointed at was real — an arbitrary bound does fail a legal deep value forever. A derived one cannot, which is why this resolves it without inventing a failure mode. That leaves nothing blocked on you. For the record, the two items I'm not doing unless you say otherwise:
On your broader point: I think you're right that reviewing prose is a poor contradiction detector, and this PR is evidence for it — the contradictions that mattered surfaced from working the rules against each other, not from reading. The Guards clean at Generated by Claude Code |
* docs: a bare symbol is a guest source again, and the borrow mode goes The ban in `memory.md` §2.8.1 closed a question — what a guest means after the symbol it was minted from is moved from — by deleting the source. The companion it needed, the `'T` borrow mode, cost a sigil, a third passing mode, and an asymmetry between the value and reference subject. Recursion stopped depending on the ban when #155 boxed recursive members through a handle, and what remained was not worth the price. So a bare symbol is a guest source again, and `'T` is removed. §2.8.1 now answers the question the ban avoided: a guest names the object hosted at its source, travels with that object when the object is moved, and carries forward to the replacement when the object is destroyed in place by an overwrite. A move and an overwrite are different statements, so the two cases never compete. A reference-type parameter has two modes, `T` and `&T`. A reference-type subject is an implicit guest and takes no marker, matching the value subject's bare form. Binding a swallowed parameter into `&` storage stays illegal on scope grounds rather than for want of a guest source, and `lifetimes.md` §1.7 loosens: any parameter may root a returned `&`, because every parameter belongs to the call-site scope. Restores `'[A-Z]` to the retired-forms guard and retires the bare-symbol guard, which now matches only correct Zane. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017AYiJXjzCPEDfYRW1ZtxH7 * docs(stories): the ban that cost more than the question it closed Two chapters: one in `stories/memory.md` retiring the bare-symbol guest source ban and the borrow mode that was built to route around it, and one in `stories/lifetimes.md` for the return-root rule that went back to "any parameter" once both were gone. The memory chapter names what the ban's own ledger left out — it counted one rejected program and not the sigil, the third passing mode, or the value/reference split in `this` that the next chapter spent to make the ban survivable. It then answers the five-liner instead of outlawing it, using the ban chapter's own argument turned around: a guest to a bare symbol buys no reach, so nobody minting one meant "watch the slot". Both costs we are accepting are stated — merging stays reachable, and a signature no longer promises non-escape. Records what we declined: the proposal that prompted this also made a moved-from symbol spent. The two readings coincide at the move rather than compete, so the downgrade stays. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017AYiJXjzCPEDfYRW1ZtxH7 * docs: fix a stale summary row and three overclaims from review - `functions.md` summary still called `this` a mutable borrow for both subject kinds, contradicting §2.4's implicit guest. The exact drift CLAUDE.md warns about, missed on self-review. - `memory.md` §1 said a guest is minted from a place that "names hosted storage", which an `&T` parameter does not; list the three forms. - `memory.md` §2.9 glossed the guest-source column as "any place but a `[]` expression", which reads as forbidding `inspect(weapons[1])` on a stored guest. §2.8 excludes `[]` from *minting*, not from being read; drop the gloss rather than restate it badly. - `CLAUDE.md` claimed every line the retired bare-symbol guard matched is now correct Zane. It isn't — §1.1 still rejects one whose target host is declared deeper than the `&`. The point is that a match proves nothing either way, which is why the pattern is gone. Also renumbers the guard list, which still said "both greps" and "a third guard" after the bare-symbol guard was removed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017AYiJXjzCPEDfYRW1ZtxH7 * docs(stories): the ban never removed the merge it was standing next to Adds the argument that makes the revert obvious rather than merely defensible: the bare-symbol ban did not eliminate the situation the anchor-merge machinery exists for. #152's canonical example reaches two live anchor identities on one payload using only field accesses, which the ban explicitly kept legal — so every line of it was writable on the day the ban shipped, and forwarding anchors were permanent regardless. The ban bought a narrower explanation, not a smaller runtime. That corrects this chapter's own cost list, which had booked "merging stays reachable" as something the revert spends. It was already reachable. What the revert actually forecloses is the class of proposals that would have deleted the machinery by restricting the source language further, which is a real decision but a different one. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017AYiJXjzCPEDfYRW1ZtxH7 * docs: a swallowed temporary has no symbol to downgrade The `functions.md` summary row read "caller may supply a temporary and downgrades to a guest", which describes something that cannot happen — `lifetimes.md` §1.6 says a hosting verb result has no symbol to downgrade, and the double-move question never arises for one. Name both move-source forms and what each leaves behind. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017AYiJXjzCPEDfYRW1ZtxH7 --------- Co-authored-by: Claude <noreply@anthropic.com>
Closes #154. Implements #156 (narrow scope — see Decision 3). Absorbs #158.
A recursive member is now an ordinary member that the compiler boxes — a fixed-size handle stored inline, payload in the scope's dynamic region, exactly as a
List's backing store is placed. A recursive member therefore owns its child instead of guesting it, and the motivating program from the issue compiles as written:There is no new syntax. The programmer writes
left Expr; boxing happens because inline placement does not exist for that member, and placement was already an unobservable implementation decision (memory.md§3.5).&now means aliasing and only aliasing.Boxing is available on both sides of the
#axis, so a value sum may recurse too. That half arrived late, from review, and is Decision 3.Decisions
1. A boxed payload has no size class
The dynamic region's blocks looked like power-of-two classes beginning at 128 bytes, and the first version of this PR extended that ladder downward to a 16-byte floor so small nodes would not waste most of a block.
That was the wrong reading of the region. §3.2's free stacks are keyed by exact byte size, created on demand; powers of two appear only because a list doubles. They were never a classification imposed on what the region holds.
A boxed payload never grows, so there is nothing for a class to absorb. It now asks for exactly its type's size, aligned to that type's alignment requirement — a twelve-byte node occupies twelve bytes. The floor, the ladder, and the question of how many classes belong under 128 all disappear.
Alignment is stated per kind instead of derived from size: a growable backing store is cache-line aligned as before, a boxed payload takes its type's own alignment, and the frontier is rounded up before it is bumped.
The one cost: exact sizes never pool across different sizes, where a class would have. That lands the right way here — a sum is laid out at its widest case plus tag, so every node of one type is the same size, and a discarded subtree returns precisely the block the next node wants. Classes pool heterogeneous churn; a tree is not that.
Three consequences of dropping classes were caught in review and fixed in
2891b02: reuse stacks keyed by size alone could hand a block to a type needing stronger alignment (now keyed by(size, alignment)); an oversized span sized by truncating division would under-allocate a payload that is not a whole number of chunks (nowceil); and §3.6 still carried the retired four-classes-below-128 paragraph.2. Which members get boxed — required on a cycle, permitted off one
The containment graph here has three edges:
Expr.opreachesOperation, and bothOperation.leftandOperation.rightreach back toExpr. Several different cuts make every type finite, so "box the recursive member" does not name one answer. Minimal cutting is not unique, which would make a type's layout depend on an ordering nobody wrote down.Boxing is therefore required wherever an edge lies on a cycle: order-independent, decidable from the declarations, no search. The cost is that a two-type cycle boxes on both hops, so descending one level of an
Exprtree costs two indirections where a hand-tuned scheme would charge one. Named as a cost in bothadt.md§4 and the story.The rule first said boxed "exactly when" an edge lies on a cycle. Review pushed on that, and the biconditional was stronger than this change needs — it forbids an implementation from boxing a member it has good reason to box. The clearest case is a sum whose widest case dwarfs its common ones: one fat case costs every instance that footprint, and an array of a thousand such values pays it a thousand times over.
257c8darelaxes it: required where no finite inline layout exists, permitted elsewhere, chosen per type. Nothing observes the difference — no operator exposes a type's footprint, and §3.5 already states that placement is not language-visible. Because the choice is per type rather than per instance, uniform stride (generics.md§7) holds either way.Why a leaf does not rescue inline layout (
2342b53, answering a direct review question). Every value of a recursive type is finite — aNatbottoms out atzero— and it is tempting to conclude the type could be laid out inline. It cannot, and the spec previously only asserted "infinite size" without engaging the objection.adt.md§4 now separates the two: layout is fixed per type, settled before any value exists, so the question is what one stride must be for every value at once.size(Nat) = tag + size(Nat)has no finite solution, and no stride can be taken from the deepest value because there is no deepest value — oneNatholds three nodes, the next three million, and uniform stride requires both to be the same size. The leaf terminates a value, not the type's size equation.3. A value copy is deep, so a value type may recurse
Review asked directly whether recursive variants would not also work with value types, and pointed out that the example the spec used to justify the ban proves nothing:
Expr's#is already forced byintLit String. That was right, and isolating the rule with an all-value-payload pair left nothing behind it.The ban rested on two objections, and
804631ffollows from both being withdrawn:lifetimes.md§2.1 plus overwrite), andmemory.md§3.2 already specifies the recursive block-return walk for reference hosts.What remained was a cost list, not a blocker. So
type Nat = variant { zero Unit; succ Nat; }is now legal.A value copy is deep (
memory.md§2.3). Copying an existing value copies its inline bytes plus a fresh allocation and recursive copy of every boxed payload it owns, so two values never share a node. For a value with no boxed member — every value type written so far — this is exactly the inline byte copy it has always been.The
#-field and&-field bans stay, on the maintainer's reason, which is sharper than the one it replaces. A reference type exists in order not to be copied: it has one host, a stable identity guests resolve through, and it reaches a new place by being moved. A value holding one could only mint a second identity (so guests silently fail to follow) or share it (so hosting is no longer single).ListandStringare covered by the same sentence. This answers #156's question 4 in the negative and leaves question 5 open there.4. Construction is not a copy, and overwrite has an order (from #158)
Merged from #158, which fixed three findings from the review on
f6f7ca5.Direct construction. §2.3 originally said a value is copied whenever "bound into a fresh slot," which read as: build
Nat.succ(...)as a temporary, copy it into its parent, copy the finished thing into the binding — quadratic. The rule now splits on place-ness. A place expression denotes existing storage and is copied when bound elsewhere; a non-place expression produces a fresh value and MUST construct directly in its eventual destination, recursively through product construction, value-variant case forms, function results, andmatcharms.Overwrite ordering. An overwrite evaluates its right-hand side against the destination's pre-overwrite state. If the source is the destination or anything reached through it, the replacement is completed before the old occupant dies and its blocks are returned, so
x = xandx = x.childcannot erase their own source. §3.2 previously returned blocks "before the replacement becomes live," which had the ordering backwards for overlapping cases; it now defers to §2.3.Snapshot allocation. §4.4 said the reader allocates from the writer's size stacks while
memory.md§3.5 says a copy allocates in the destination scope — a straight contradiction. Destination wins: a snapshot is an ordinary deep copy into its fresh binding, and reader and writer contend only when they actually share an arena. Blocks stay provisional until the final version check, and a rejected attempt returns every block it allocated before retrying.Discriminant validation. A fourth walk rule ahead of the span check: all structure-directing metadata is untrusted before the final version check. A
variantdiscriminant must name a declared case before dispatch; likewise any count or length deciding which child handles exist. The depth bound is per attempt.2342b53then reconciled two passages #158 left describing the pre-#158 rule:adt.md§4 claimed a recursive value type walks and reallocates on every store into fresh storage (true only for binding an existing place), andmemory.md§6 read as though passing a value parameter were the copy.5. Why the
#axis got smaller#now decides identity, aliasing, and copy-versus-move. It no longer decides whether a type may contain itself, because that question belongs to layout, and layout answers it the same way for both kinds.foundations.md§7,types.md§2.1–§2.2, andadt.md§1/§3 are reworded accordingly, andboxed hosting memberis generalized toboxed member.A boxed payload's nature follows the member's declared type, not the enclosing kind — off-cycle boxing lets a
#structbox a value-typed member, and that payload must not gain a backpointer or become guestable.memory.md§3.3 splits the two questions: what the payload is follows the declared type; what becomes of it on move, copy, or death follows the enclosing kind.6. One rule, one home
The exact-size rule was stated three times across
memory.md§3.2 and §3.6, with the rationale twice. §3.2 now owns sizing, alignment and reuse; §3.6 keeps only the two-part handle/payload representation. Consolidating exposed a contradiction from2891b02: §3.2 rekeyed the reuse stacks to(size, alignment)in one paragraph while the next still read "keyed by byte size alone." Fixed.One thing in the issue that did not survive contact with the spec
The issue says "a constructor result is a hosting verb result — a legal move-source (
lifetimes.md§1.2)". That is right forOperation(...), butExpr.intLit("3")is not a verb result:adt.md§3.2 is emphatic that naming a variant case is built-in syntax with no verb behind it, and §1.2 listed only two move-source forms. §1.2 now lists a#variantcase form as a third, on the same terms as a hosting verb result. A valuevariantcase form is still excluded, and for a reason that survives Decision 3: a value sum is copied rather than hosted, so there is no hosting to transfer.Changes
spec/memory.mdspec/adt.mdNat/Peanoboth legal) plus value-sum construction by copy; §4 rewritten — why a leaf does not rescue inline layout, boxing available to both kinds, ownership as hosting or value-ownership, costs corrected for direct construction; §7 comparison row; §8 summary rowsspec/concurrency.mdspec/foundations.md#is identity, aliasing, and copy-versus-move; recursion available to both kinds; deep-copy bullet; summary rowspec/types.mdspec/lifetimes.md#variantcase form as a move-source and the reworded value-form exclusion; §2.1 value death points and recursive block returnspec/syntax.mdspec/glossary.mdboxed hosting member→boxed member; new §3.40deep value copy; new §3.41move-sourcestories/memory.mdstories/adt.mdREADME.mdValidation
All three
CLAUDE.mdguards, run with-Ron the directory, re-run at2342b53:CLAUDE.md: three deliberate// ILLEGAL:examples (memory.md:172,memory.md:181,lifetimes.md:27) and one grammar metavariable (syntax.md:20)receiver— the single expected hit, the chapter-title pointer atfunctions.md:37Story append-only check against
origin/main, both files: no output, additions only — including across the #158 merge, since the chapters it edited are this branch's own unmerged drafts.Anchors: every spec→story pointer, story→story link, and pinned in-prose permalink was verified to resolve by script.
bench/No change, and none made speculatively.
bench/zane_bench.cmodels the allocator, not the type system. The shapes this change produces are already in it: Test 3 exercises mixed 8/16/32/64-byte allocation with random-order free, and Test 10 is an ownership tree of ~4000 individually allocated nodes torn down post-order.Still open
lifetimes.md§1.3 already forbids independently — so making it legal is a change to the move model, not a clarification here. Worth its own issue.#reduces to identity and aliasing alone, and whether a deep-copiedListinside a value type should be legal.d06d2e0 docs(meta): guard the receiver-to-subject rename, which predates this work and only adds a validation guard toCLAUDE.md. Happy to split it out if preferred.Summary by CodeRabbit
Operationand ordinary recursive members instead of explicit reference fields.