Skip to content

name and enforce the operand-headed statement shape - #2255

Draft
Redict wants to merge 5 commits into
nim-lang:masterfrom
Redict:operand-headed-stmt-set
Draft

name and enforce the operand-headed statement shape#2255
Redict wants to merge 5 commits into
nim-lang:masterfrom
Redict:operand-headed-stmt-set

Conversation

@Redict

@Redict Redict commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

Stacked on #2240, which adds (comesfrom ^SYM S*). Draft until that merges; the diff here includes its commits.

Two statement tags are transparent wrappers, no scope and no semantics, but their first child is an operand:

(pragmax ^(pragmas ...) X)   the pragma list
(comesfrom ^SYM S*)          the expanded symbol

A pass that doesn't care about the tag should just descend into the body, which makes these look like (stmts ...). They aren't. A generic child walker walks the operand, silently, and the damage surfaces several passes later.

pragmax has 9 hand-written branches today. comesfrom needed 16 more in #2240, four of them found by breakage rather than reading: derefs/vl surfaced as an init error in widestrs.nim, cse hoisted a load above a declaration the expansion makes, lengcgen emitted a global's initializer as a C constructor running before NimMain. Each in a different suite. Plus 4 in arkham (nim-lang/nativenif#95), caught by CI.

Nothing recorded the property. doc/tags.md had pragmax backwards, (pragmax X (pragmas ...)), when every producer, all 7 consumers and all 68 .nif fixtures put pragmas first. So the 9 branches read as folklore, not a pattern.

What's here

OperandHeadedS = {PragmaxS, ComesfromS} in nimony_model, mirrored as {ComesfromS} in leng_model. Transparency is the criterion, not the leading operand: (block .D X) has the same shape and is excluded, being a scope and a break target.

bodyInto is an into-shaped template that steps over the operand, so the right walk is shorter than the wrong one:

n.into:                    n.bodyInto:
  skip n # origin symbol     while n.hasMore:
  while n.hasMore:             trStmt c, n
    trStmt c, n

Row 143 corrected in place, since gen_tags.nim derives tag values from the row index. No ordinal moved.

A ^ slot modifier marks the operand, alongside the existing .D, T?, S*. The validator reads it and reports a case n.stmtKind branch that opens such a tag and walks every child.

Why new table syntax

Keying on slot kind doesn't work. (bind Y) and (mixin Y) lead with the same kind as (comesfrom ^SYM S*); (when ...), (using ...), (if ...) match (pragmax ^(pragmas ...) X). None is transparent. 11 of 15 validator pass files failed that way. (instr SYM X*) is the sharpest case: same shape as comesfrom, not transparent, correctly excluded only because it's unmarked.

Reading a marker sentence from the row prose worked but is fragile the same way the original bug was. Reword the row, the check stops firing.

A 5th column isn't possible, gen_tags.nim requires exactly 5 fields. Per-slot modifiers are already how this table says things about slots. Adding ^ also meant documenting the slot notation, which was written down nowhere.

Scope and honest limits

The 25 existing sites are left alone. One was converted, controlflow.nim, as a smoke test.

The check would not have caught any of the four regressions. All four were permissive else: branches with no branch for the tag at all, which the existing exhaustive-case check already reports. This covers the follow-on shape: an author who writes the branch that check forces, then walks every child.

It fires on 8 branches in existing pass files, including eraiser.nim, whose branch is commented "generic container: copy the head and recurse into the children". Whether each is a live bug depends on what the pass does with what it walks, and the check can't tell a copy from a rewrite. So it warns instead of erroring: pass files stay green, sites get reported, nobody's forced to rewrite working code.

Testing

655/655 on the base branch, 656/656 with this, validator 16/16. The golden tests/check_tags/fake_operand_headed.nim covers the violating form, both conforming forms, and the dispatching form that must stay quiet. Deleting the check drops it to 15/16.

Removing ^ from the comesfrom row drops detection to zero; restoring brings it back.

Full rationale and the alternatives weighed are in openspec/changes/add-operand-headed-stmt-set/.

Redict added 5 commits August 4, 2026 17:20
Registers a transparent statement wrapper marking a template expansion, so
the LLVM debug backend can later emit it as an inlined frame (nim-lang#1987).

Nothing emits the tag yet; this is the plumbing. Every case n.stmtKind
dispatcher is taught to see through it, which the compiler's exhaustiveness
check enumerates for us: 13 sites across nimony/hexer/lengc.

The wrapper opens NO scope, unlike (scope ...). That is the whole point:
a scope would hide {.inject.} symbols. It follows the (par ...) model
Araq pointed at in nim-lang#1946 - a semantics-free syntax record that consumers
descend into and otherwise skip.
fixes nim-lang#1987's frontend half. semTemplateCall now emits
(tmplbody templateSym <expansion>) around void expansions, carrying the
call-site info on the head. Non-void expansions stay unwrapped: they are
expressions and beforeCall arithmetic must keep pointing at the value.

The wrapper is re-sem'd when a generic body instantiates, so semExpr
gets a preserving branch (no scope, {.inject.} keeps working).

Passes that treat statements generically needed the leading symbol
stepped over; the ones with permissive else branches mis-walked it as an
expression and had to be taught explicitly:
- derefs: recursed into the symbol, corrupting the tree the NJ
  definite-assignment analysis then read ('cannot prove initialized')
- vl: would version the symbol like a variable
- nj/finalir/contracts_fir: walk the body inline so writes inside an
  expansion count for definite assignment
- lengcgen trToplevel: must dissolve the wrapper at module toplevel,
  since its children get split between file scope and the init proc.
  Keeping it hoisted global initializers into a C
  __attribute__((constructor)) that ran before NimMain on zeroed
  globals - module plugins crashed with 0xC0000005.

tvarargs/tresemtype golden .nif files re-recorded: toplevel echo is now
wrapped, which is the expected new shape.

tests: templates 9/9, macros 6/6, plugins 16/16, stdlib 51/51.
fixes nim-lang#1987. A template call now shows up as its own stack frame in the
debugger instead of the stepper jumping into the template's definition.

genComesFromLLVM turns each (comesfrom SYM ...) into a synthetic
DISubprogram for the template plus an inlinedAt chain back to the call
site, which is what DWARF models inlining with. The call-site DILocation
is built BEFORE the frame is pushed, so nested expansions chain rather
than flatten:

  !7  = DILocation(line: 5, scope: !4)                    call site
  !9  = DISubprogram(name: "setElem")
  !10 = DILocation(line: 2,  scope: !9,  inlinedAt: !7)
  !12 = DISubprogram(name: "[]=", file: system.nim, line: 34)
  !13 = DILocation(line: 34, scope: !12, inlinedAt: !10)

That !13 is exactly the frame the issue reports as a bare jump into
system.nim:34.

The SP is keyed on the template's SymId so all call sites share one, and
takes its file/line from the expansion's first statement: template decls
do not reach Leng, so the body's own info is the only source.

emitDbgDeclare follows the frame too - a variable declared inside an
expansion is scoped to the synthetic SP and its location carries the same
inlinedAt chain, or the verifier rejects the module. Such variables are
kept out of the physical proc's retainedNodes.

renderer: (comesfrom ...) has no source syntax, so astToStr and error
messages render the expanded statements alone.

shoggoth/cse: the wrapper is transparent, not a hoist anchor, and its
first child is a symbol rather than a statement. Without both, CSE walked
it as an expression and hoisted a load past the declarations the expansion
itself makes - `useCpuRegisters` in nifreader.nim declares an {.inject.}
`p` and then loads `^p`, so the boot bootstrap failed to compile with
`'p_11' undeclared`.

Adds tests/llvmdebug, a golden suite over the DWARF-relevant metadata of
the emitted .ll (DIFile, DISubprogram, DILocation, DILocalVariable). The
rest of the IR is dropped: it churns with every unrelated codegen change.
The absolute directory and the module-hash suffix are normalized so the
goldens are portable across checkouts. `hastur.mode = skip` because the
LLVM backend cannot build the full stdlib yet; run it with
`hastur tests/llvmdebug`, `--overwrite` to regenerate.

Two cases: a nested expansion pinning the two-level inlinedAt chain, and a
`var` declared inside an expansion where the DILocalVariable scope and its
DILocation must agree. Checked that the suite fails when an inlinedAt is
removed from a golden.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant