Commit Graph

8887 Commits

Author SHA1 Message Date
Araq
06b1bf8f9a IC: take the seam into trees and ccgutils
`trees.nim` can import `bnode` — nothing in `bnode`'s import closure reaches
`trees`, checked rather than assumed — so the shared helpers move to `AnyNode`
instead of being reimplemented behind the seam: `getMagic`, `whichPragma`,
`getRoot`, `isDeepConstExpr`, plus `ccgutils.stmtsContainPragma`. That unblocks
three more codegen procs, `canMove`, `notYetAlive` and `ifSwitchSplitPoint`,
which needed them and nothing else.

`stmtsContainPragma` could not simply stay `getPragmaStmt(n, w) != nil`, and
the reason is worth recording because it will recur: a proc that returns a node
OR NIL is the one shape the seam cannot serve. `.bif` spells a missing child as
a `DotToken` *inside* a tree; there is no nil token to hand back as a return
value and a `Cursor` is not nilable. So the predicate is split out — and,
because that leaves two copies of one traversal, `grindPredicates` now asserts
the two agree at every node instead of trusting them to.

Measuring the answers, not just the agreement, again earned its keep. Six of
the new checks came back with a wide spread (`getMagic` 7780 non-`mNone` over
many magics, `getRoot` 19506 non-nil syms compared by identity, `isDeepConstExpr`
7917 true, `notYetAlive` 9653 true). Two came back CONSTANT — `stmtsContainPragma`
false at all 67_721 nodes and `ifSwitchSplitPoint` zero at all 24 — because
nothing in the closure uses `{.linearScanEnd.}` or `{.computedGoto.}`. Both are
now exercised on both answers by shapes added to `tools/icgrind`. A check that
grades a constant is indistinguishable from a passing check in the output, so
this only shows up if the distribution is looked at.

Verified: grind clean over the whole `--ic:on` closure (67_857 nodes, 0
disagreements); the target's `--ic:on` output matches its `nim c` output;
215/215 byte-identical `.c` against HEAD on the default path; all four build
configurations compile.

Sabotaging `bnode.secondSon` — an accessor the lockstep walk does NOT itself
use, since it descends by index — is caught only by this layer, and is: it
fires on `getRoot`, `isDeepConstExpr`, `reifiedOpenArray` and
`skipTrivialIndirections`.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XEF7FJvUkGKvG9LSGuEaNR
2026-08-30 10:23:43 +02:00
Araq
837082eb89 IC: grade every migrated predicate at every node, and migrate nine more
Nine `PNode`-typed codegen procs become `AnyNode`, all pure readers:
`bodyCanRaise` (the consumer of the `canRaise` work), `isAssignedImmediately`,
`branchHasTooBigRange`, `hasNoInit`, `reifiedOpenArray`,
`skipTrivialIndirections`, `isSimpleExpr`, `isConstClosure` and `fewCmps`.

The signatures are the small part. A migrated proc that nothing calls with a
`BNode` is not even type-checked, so the substance is `grindPredicates`: every
one of them runs on BOTH spellings of the SAME node, at EVERY node of every
graded body, inside the walk `grindLockstep` was already doing. 67_721 nodes
on the reference target, 0 disagreements.

Two things had to be gated, and neither by widening a guard until the run went
green.

The tolerated `(ht . <sym>)` type difference is benign for the vocabulary check
and NOT benign for a predicate that reads `typ` — and because the predicates
recurse, one excused node poisons every ancestor's answer too. `grindLockstep`
now reports whether a subtree is free of it, and only clean subtrees are
graded.

`isAssignedImmediately` and `fewCmps` hand `n.typ` to `getSize`/`mapType`,
which are total only over types the C backend can lay out. Asked at an
arbitrary node they meet a `tyGenericParam` or a `tyAnything` and abort — that
is a question with no answer in either spelling, not a disagreement between
them. Both are graded FROM THE PARENT, at the position production calls them
from. Declarative subtrees are skipped for the same reason, along the boundary
`bodyCanRaise` already draws.

Both exclusions are counted and printed beside the graded count, so a run that
grades nothing cannot pass for a run that grades everything.

`tools/icgrind` versions the grind target, because two ways of silently
getting no coverage turned up while writing it: the main module's routines are
never graded, and `ast2nif` defers only `nkStmtList` bodies, so a one-line
`proc f(x: int): int = case x ...` is invisible to the oracle. Shapes added
for `branchHasTooBigRange` and `fewCmps` produced exactly zero coverage until
both were found by counting rather than assumed.

Verified: 215/215 byte-identical `.c` against HEAD on the default path;
sabotaging `isAtom` and sabotaging the parent-driven node selection each make
the grinder fail on the first body it reaches.

Recorded rather than papered over: `isConstClosure` is graded only on its false
side — the whole closure contains one `nkClosure` node.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XEF7FJvUkGKvG9LSGuEaNR
2026-08-30 10:08:07 +02:00
Araq
512d2a8f26 IC: revert the (ht . <sym>) lazy-type pin — it broke sem
The previous commit made `(ht . <sym>)` — a sym node the writer gave an
EXPLICITLY nil type — load back with `nfLazyType`, so `ast.typ` answered
`sym.typ` instead of nil. The stated reason was to remove a load-order
dependence, and the direction was wrong: that nil is load-bearing.

`writeSymNode` only emits the wrapper when the node's own type DIFFERED from
its symbol's, so a nil there says the node genuinely had no type while the
symbol had one. A type symbol used as a VALUE is exactly that shape:
`newException(KeyError, ...)` passes a typedesc, whose node carries no type
while the symbol carries the object type. Handing it `sym.typ` makes sem read
the typedesc as an expression of the type it denotes, and `--ic:on`
compilation of anything instantiating `tables.[]` dies with "only a 'ref
object' can be raised". A four-line program is enough:

    import std/tables
    var t = initTable[string, int]()
    t["a"] = 1
    echo t["a"]

The load-order dependence is real but is not fixed by pinning the flag EITHER
way — setting it breaks sem as above, clearing it would strip the fallback
from the not-yet-loaded-stub population that `nifcBackendActive` exists to
serve. Left alone deliberately, with the reasoning recorded at the site.
`bnode.typ` answers the faithful nil, and the grinder excludes this one shape
via `hasExplicitNilType` — narrowly, only when the cursor says nil and the AST
is saying exactly the symbol's type.

Why the suite did not catch it: `tests/ic` passed 39/39 throughout. The same
four-line program reproduces from the scratchpad and from the repo root, and
PASSES under `tests/ic` — `--skipParentCfg --skipProjCfg` makes it fail there
too, so `tests/config.nims` is what masks it, most plausibly because
evaluating a NimScript config runs the VM and perturbs the very load order the
bug depends on. A test file under `tests/` therefore cannot guard this class,
and no test is added rather than one that passes on the buggy compiler.

Also in this commit, and the reason the bug was found at all:

* `effectsOf` / `raisesNothing` replace the raw subscripting of `fn.typ.n` in
  `canRaiseImpl`, so the effect-list layout is written down in one place and
  the templates carry no knowledge of it. `raisesNothing` is stated as the
  NEGATIVE on purpose — the safe default is "can raise", so the one narrow
  shape that licenses dropping an exception check is the one spelled out, and
  an unanticipated shape falls conservative by construction.

* `-d:icCanRaiseLog` logs every `canRaiseDisp` verdict keyed by name, disamb
  and OWNING MODULE, with the deciding branch. What "the canRaise helpers work
  on a `.bif`" means is that the type the decoder materialises carries the same
  effect list the from-source one did — a claim about the WRITER that the
  BNode/PNode grinder structurally cannot make, since both spellings ask the
  same `PType` and agree however wrong it is. The only oracle is the same
  program built without IC: 234 callees comparable, 0 disagreeing, 23 of them
  reaching the effect-list branch in both builds.

  Two instrumentation bugs worth recording, because both produced confident
  wrong numbers first: keying by name+disamb alone collided (`len.0` names a
  different routine per module) and reported one false disagreement; and the
  branch marker was a global that `canRaiseDisp` left stale on its early
  return, which inflated effect-list coverage from 23 to a claimed 142.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XEF7FJvUkGKvG9LSGuEaNR
2026-08-30 08:54:46 +02:00
Araq
5799220c98 IC: grind the Cursor vocabulary against the PNode loader, and fix what it found
The `BNode` accessors had no oracle. The self-test in `bnode.nim` checks them
against each other and against the raw token stream, which a uniformly wrong
vocabulary satisfies — it passed 5.2M assertions on an off-by-two model. This
adds the missing oracle and lets it drive the next migration step.

`cgen.grindLockstep` (opt-in, `NIM_IC_BNODE_GRIND=1` on an `--ic:on` build)
walks the `.bif` cursor and the materialised `PNode` for the SAME body side by
side and requires `kind`, `len`, `info`, `flags`, the literal payloads, `sym`
and `typ` to agree at every node of every routine body in the dependency
closure. The `PNode` is the oracle, so it compares everything rather than what
someone thought to check. It found two bugs, both silent:

* `BNode.typ` answered `nil` for every bare `Symbol`. `ast.typ` does not: it
  falls back to `n.sym.typ` when the loader set `nfLazyType`, which it does for
  exactly that shape. The consequence is not less information but a DIFFERENT
  answer — `canRaise` asks `fn.typ.kind == tyProc` about a call's callee, so
  nil turns "this call can raise" into "it cannot" and drops the
  goto-exception check after the call.

* `(ht . <sym>)` — an explicitly nil node type — made `n.typ` LOAD-ORDER
  DEPENDENT in the loader itself. `newSymNode` marks the node lazy only if the
  symbol was still an unloaded stub at that moment, so the same `.bif` node
  answered `sym.typ` or `nil` depending on what happened to touch that symbol
  first. Pinned to the lazy reading, so the answer is a property of the file
  rather than of the traversal order.

With `typ` correct, the blocker recorded in `allPathsAsgnResult` is gone.
`ast.canRaise`/`canRaiseConservative` cannot become `AnyNode` procs where they
live — `BNode` is defined in `bnode.nim`, which imports `ast` — so their bodies
move into templates that `bnode` instantiates for its own node type. One source
of truth, no cycle, no second copy. `ccgcalls.canRaiseDisp` and
`cgen.allPathsAsgnResult` follow.

Adds the leaf accessors (`intVal`, `floatVal`, `strVal`, `ident`, `flags`)
because nothing in `ccgexprs` can migrate without them, and `rawDesc` for
diagnosing a disagreement in terms of what the token stream literally says.

`compiler/bodynav.nim` replaces the `BodyScope` snapshot with a scope chain the
traversal maintains — `openScope`/`closeScope`/`registerDefHere`, lookup
falling through to the decoder — ported from Nimony's `typenav`. The scope
becomes a product of the walk, so nothing is copied ahead of time and nothing
can be stale. How much of a live problem the snapshot was is measured rather
than assumed: `-d:icLocalSymStats` reports `localHit=0 fieldStub=2 miss=0
sdReg=5902 extractReg=45` over the stdlib closure, i.e. definitions register
constantly and not one use ever resolved through the table, because
`isLocalSym` is a hardwired `false`. The hazard was latent; this keeps it
latent once that stops being true.

Verified: 0 disagreements over the whole `--ic:on` closure with the walk
driving the nav; deliberately breaking `intVal`, `flags` and the nav key each
make the grinder fire on the first bodies it reaches, so the clean run is not
vacuous; the default path emits 215/215 byte-identical `.c` against the
pre-change compiler and does not compile `bodynav` at all; `tests/ic` 39/39.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XEF7FJvUkGKvG9LSGuEaNR
2026-08-30 08:07:38 +02:00
araq
2447dfdc7d cgen: name the PType child instead of subscripting it
The same treatment the `PNode` side just got, for the reason that applies to
types: `t[0]` is the return type, the base class, the index type or the generic
head depending on the kind, and the subscript says none of that. Every child
access in the cgen files that has a named accessor now uses it — `baseClass`
for the eleven object-hierarchy walks, `elementType` for the seq/openArray
element, `returnType`, `genericHead`, `firstGenericParam` — and the two loops
that walked a type's children become `paramTypes` and `kids`.

Left indexed on purpose: a parameter reached by ARGUMENT position
(`typ[i]` in ccgcalls/ccgstmts), a tuple field, and a generic parameter at an
explicit index. There the index is the clearest thing to write.

Every substitution is exact rather than merely close. `[]` with index 0 is
unconditionally `sonsImpl[0]`, so `baseClass`/`returnType`/`genericHead` cannot
diverge; `elementType` is `sonsImpl[^1]` and is used only where the type has a
single son; `paramTypes` and `kids` are literally the loops they replace.

`ast.sons(t: PType)` gets the warning it has been missing. Despite the name it
is not the counterpart of the `sons` ITERATOR over a `PNode`: it returns the
raw seq, and a `tyProc` keeps its parameter types in `n`, so that seq holds
only the return type while `[]`/`len`/`kids` route parameters through
`n[i].sym.typ`. `for x in t.sons` therefore compiles, reads exactly like the
`PNode` idiom, and visits a different set of types — which is what
`ccgutils.encodeType` would have started doing had it been converted to `sons`
rather than `kids`. Marking the proc deprecated and rebuilding shows one call
site in the whole compiler (`previouslyInferred`), so the trap is latent, not
active.

`bnode.nim` also records that there is deliberately no `BType` beside `BNode`:
types stay `PType`s under `newIcBackend` — `BNode.typ` returns one — because
the backend asks them questions (`skipTypes`, `getSize`, `lengthOrd`, the
record walk over `t.n`) that a raw cursor cannot answer.

Pure refactor: all 216 generated `.c` files byte-identical to the parent commit.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FMyRHByv7hhaQJ4Pa1bHbE
2026-08-29 09:05:44 +02:00
araq
437876efbd IC: say what BNode.sym/typ/info actually need
The stubs guessed. Checking `ast2nif` shows all three need a resolution
context and so cannot stay unary accessors on the cursor:

  * a `Symbol` token holds only a NAME; `loadSymStub` resolves it from a
    `DecodeContext` plus the owning module plus that routine body's `localSyms`,
    because an unsuffixed name is body-local and is in no index;
  * `createTypeStub` likewise takes a `DecodeContext`;
  * `rawLineInfo` yields a `NifLineInfo` in the `.bif`'s own file pool, which
    `oldLineInfo` maps to a `TLineInfo` through a `LineInfoWriter` holding the
    `ConfigRef`.

So the open question is not how to write these three but where codegen gets
that context — a parameter, or module-global state for the span of one module's
`cg` stage, as `ast2nif` already keeps for its writer. Recorded so it is not
rediscovered.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FMyRHByv7hhaQJ4Pa1bHbE
2026-08-29 07:10:59 +02:00
araq
20b70c8b1e IC: implement the Cursor half of the codegen vocabulary
`BNode`'s `Cursor` branch stopped at `{.error.}` stubs. Everything structural
now works, so `-d:newIcBackend` no longer fails on the shape of the tree — the
remaining stubs (`sym`, `typ`, `info`) are the three that need the decoder's
symbol/type/line-info maps, which is the honest next boundary.

Two corrections to the seam, both found by looking at what actually reads a
`.bif`:

  * it was pointed at `nifcursors`, the WRITER cursor over `PackedToken`s.
    `bif.load` produces a `nifcore.TokenBuf` and `ast2nif` decodes it with a
    `nifcore.Cursor`; that is the type the backend will get.
  * the cost model said reading child `i` is O(size of children 0..<i). It is
    O(i): a `TagLit` token stores the width of its whole subtree, so
    `nifcore.skip` is one pointer add no matter how big the subtree is. Indexed
    loops are still quadratic and still worth removing, but in the number of
    children, not in tree size.

`kind` is the accessor the stub called pivotal, and the note on it was wrong in
a way that matters: a `.bif` carries its OWN tag pool, so a tag id means
nothing outside its file and the "build a tag-id -> TNodeKind table once" plan
cannot work. It memoizes per pool and drops the memo when the pool changes.

Since no call site executes any of this yet, `when isMainModule` walks real
`.bif` files and checks the vocabulary against itself and against an uncached
tag lookup. Over 70 files (`.s.bif`, `.t.bif`, `.iface.bif`) from the
testworkspace corpus: 2.05M nodes, 9.19M assertions, all passing. Both halves
of the harness were confirmed live by sabotage — `secondSon` returning child 2
trips it, and so does dropping the tag-pool memo invalidation, which is what
proves ids really do differ between files.

The default build is untouched: all 216 generated `.c` files still byte-
identical to the parent commit.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FMyRHByv7hhaQJ4Pa1bHbE
2026-08-29 07:09:23 +02:00
araq
9e076d74c0 cgen: no [] on a PNode outside tree construction
Completes the vocabulary migration started with `BNode`: every child READ in
the cgen files now goes through an accessor that a `.bif` `Cursor` can also
serve, so flipping `newIcBackend` is a matter of implementing the vocabulary
rather than rewriting call sites.

  * constant indices -> `firstSon` / `secondSon` / `lastSon` / `son(n, k)`,
    including the `namePos`/`paramsPos`/`bodyPos`/... slot reads;
  * indexed loops -> `sons` / `sonsFrom` / `sonsButLast` and the index-yielding
    `isons` / `isonsButLast`. `for i in 0..<n.len: n[i]` is quadratic once
    `BNode` is a `Cursor`, because reaching child `i` costs one `skip` per
    preceding SUBTREE;
  * `n.len == 0` / `> 0` on a node -> `hasSons`, which does not count.

`astdef` gains `sonsButLast(n, count)` and `isonsButLast` — the
`nkOfBranch`/`nkExceptBranch` shape, whose last child is the branch body, and
with `count = 2` the `nkVarTuple`/`nkIdentDefs` shape.

Three places needed more than a rename:

  * the C++/goto/setjmp try generators re-subscripted `t[i]` up to ten times
    per iteration of their `while i < t.len` walk; the branch node is now
    hoisted once per step;
  * `genParams` scans the arguments BACKWARDS to decide which need a temporary,
    which a `Cursor` cannot do at all. It materializes them in one forward pass
    and indexes that — the same order of work, since `needTmp` already
    allocates per call;
  * loops that stop at a computed position (`casePos`, `until`, `splitPoint`)
    walk forward and break instead of counting up to the bound.

What is left is exactly what a `Cursor` backend will not do: writes that build
a fresh `nkProcDef`, and subscripts of a `PType`, `string`, `seq` or `Table`.
`bnode.nim` records the invariant and the `PType` trap — `ast.sons(t: PType)`
is a proc returning `var TTypeSeq`, not the iterator of the same name — which
the type checker enforces, since the `firstSon`/`secondSon`/`lastSon`/`son`
family exists for `PNode` only.

Pure refactor, verified as one: all 216 generated `.c` files byte-identical to
the parent commit; metamorphic IC 16/16; icSuite 19/19 fragments; categories
gc 78, arc 140, destructor 97, closure 23, iter 71, trmacros 6, cpp 50,
exception 47, casestmt 16 with one pre-existing environmental failure
(tests/cpp/tasync_cpp.nim: `cannot open file: jester`).

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FMyRHByv7hhaQJ4Pa1bHbE
2026-08-29 06:57:55 +02:00
araq
8d4ddd5516 IC: introduce BNode, the seam for running codegen off a .bif Cursor
Building `PNode` trees out of a module's `.bif` is the bulk of the `lower` and
`cg` stages: their cost tracks the size of the dependency CLOSURE a stage loads,
not the module it compiles (a 370-byte module costs 0.20s/0.16s in lower/cg, the
main module 3.40s/3.16s). Since the link stage stopped loading the graph those
two are ~85% of the serial backend critical path. The goal is to let the backend
read the `.bif` directly through a `Cursor`, constructing no `PNode` at all.

`BNode` is the seam: a `Cursor` with `-d:newIcBackend`, a `PNode` without, which
is what every build does today. Codegen migrates one area at a time and the
compiler keeps building throughout.

The seam is thin on purpose. `ast`/`astdef` already give `PNode` the whole
vocabulary — `kind`, `len`, `safeLen`, `sym`, `typ`, `info`, `firstSon`,
`secondSon`, `lastSon` and `sons`/`isons`/`sonsFrom` — so bnode deliberately does
NOT redefine them for `PNode`: an identical second overload makes every call site
ambiguous (tried it; `lastSon` breaks first). It adds only `son` and `hasSons`,
and supplies the full vocabulary on the `Cursor` side as `{.error.}` stubs, so
flipping the define names the exact missing accessor AT ITS CALL SITE instead of
collapsing into a cascade of type errors.

Two notes for whoever continues this:

* The migration front is SIGNATURES, not call sites. Converting `n[i]` to the
  iterator vocabulary changes nothing while the value is still `PNode`-typed —
  flipping the define then reports a plain type mismatch. Changing one proc's
  parameter from `PNode` to `BNode` is what moves it, and is a no-op with the
  define off. `containsResult` and `allPathsAsgnResult` are converted as the
  worked example.
* `BNode`'s cost model differs and the vocabulary is shaped around it: reading
  child `i` of a `Cursor` is O(size of children 0..<i), so prefer `sons`/
  `sonsFrom`/`isons` over indexing, and avoid `len` in a loop condition.

Note `type BNode = when defined(x): Cursor else: PNode` does not parse — `when`
is not an expression in type position; it needs a `when` block over two `type`
sections.

Inert: all 219 generated `.c` files and the linked binary are byte-identical to
the parent commit.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-08-29 05:44:39 +02:00
araq
f84f53bff4 cgen: replace indexed child loops with the sons/isons/sonsFrom iterators
`for i in k..<n.len: ... n[i] ...` is the dominant shape for walking a `PNode`'s
children in the code generator: 41 such loops across the cgen files, and 777
indexed node accesses in total. It reads worse than iterating, it bounds-checks
every subscript, and it is quadratic the moment the backend reads children off a
NIF `Cursor` rather than a materialised tree (a child is `firstSon` plus one
`skip` per preceding sibling, and `skip` steps over a whole subtree).

31 of the 41 are converted:

* 20 to `sons`/`sonsFrom` — the index only ever subscripted `n`.
* 9 to `isons`, which now takes a `start` index (defaulting to 0, so its eight
  existing call sites are unchanged). These genuinely need `i`: a parallel index
  into the routine's `PType` (`typ.n[i]`, `typ[i]`), a `needTmp[i-1]` lookup, an
  `i == field.position` test, `$i` in a generated struct name, or the index
  passed straight to `genOtherArg`.
* 2 to `sonsFrom` with a variable start (`firstParam`, `offset`).

`sonsFrom` is new, next to `sons`/`isons` in astdef.

The remaining 10 are deliberate. Eight are not `PNode` at all — `varargs[Snippet]`,
`seq[PSym]`, `string`, and `PType`, where `sons` is a `proc ...: var TTypeSeq`
rather than an iterator, so a blind rewrite would compile into something quite
different. Two iterate `0..<it.len-1`, excluding the last child, which no
iterator expresses cleanly.

Pure refactor, and verified as one: all 219 generated `.c` files of a 219-module
program and the linked binary are byte-identical to the parent commit. That is
the bar that matters here, because the index arithmetic (`i-1`, `i == position`,
`$i`) is the easy thing to get wrong. It also caught a real slip on the way:
`genFieldCheck` reassigns its loop variable, which a `for` binding cannot do.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-08-29 05:44:14 +02:00
araq
e0c0724b62 IC: the link stage no longer loads the module graph
`--icBackendStage:link` called `loadBackendModules` — a whole-program
deserialization — for exactly two things: each module's `.c` path via
`getCFile`, and its recorded C compile/link directives via
`replayBackendActions`. That was 3.7s of the ~11s serial backend critical path
on a 219-module program, spent recovering a list of paths and a handful of
strings.

Both now come from artifacts the earlier stages already produce:

* `.c` paths from the driver's existing `LiveModulesFile` manifest (which
  `merge` already reads); the `.c` sits beside each listed `.c.nif`, so no new
  manifest was needed. The merge-decision fallback for conditionally-imported
  nodes that own a live symbol is kept.
* C directives from a new `<module>.c.cflags` sidecar. The module's own `cg`
  already replays them, so it writes them down too — one tab-separated line per
  `compile`/`link`/`passl`/`passc`/`localpassc`/`cppdefine` action. `localpassc`
  needs the module's source path, which only the writer can resolve, so it is
  baked into the line. Written unconditionally, empty included: it is a declared
  nifmake output of the `cg` rule and a missing output re-fires the rule for
  ever.

link: 3.70s -> 0.44s. A one-line code edit on the corpus goes 15.2s -> 11.8s;
cold 124s -> 120s; no-op unchanged at 0.27s.

Green: 16/16 metamorphic IC tests, the 17-file `koch ic` suite, 13/13
differential edit checks against `nim c`, and `bootic` iteration 1 (the full
fixed-point check was still running when this was committed).

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-08-28 23:13:16 +02:00
araq
d81e764f98 IC: the cg stage must not write emit's .c
`registerModuleCode` writes the module's `.c` and registers it for compilation.
Under `--icBackendStage:cg` it must do neither: `cg`'s product is the `.c.nif`,
the `.c` is rendered by `emit` from that artifact plus the GLOBAL merge
decision, and the compile registration belongs to `link`.

Writing it in `cg` puts a second, differently-filtered `.c` — `cg` can only
filter by the liveness its own process sees — at the exact path `emit` declares
as its nifmake output. Two stages claim one output, and the `.c` ends up newer
than `emit`'s own `.c.nif` input, so nifmake considers `emit` up to date and
skips it: `cg`'s unfiltered text goes to the linker and every emit-everywhere
body is defined twice ("multiple definition of eqdup__u727466987__OOZ…").

Nothing surfaces this today because `merge` rewrites the decision file on every
run and every `emit` lists it as an input, so all of them re-fire and overwrite
the stray file. The fire-all is therefore load-bearing, not the "insurance" it
is documented as — and it is what makes a content-stable merge decision
(219 fewer processes and ~13s less backend CPU per edit on a 219-module
program) impossible: that experiment is exactly how this was found.

No behaviour change on its own, since the fire-all still runs. Green: `bootic`
fixed point, 16/16 metamorphic IC tests, the 17-file `koch ic` suite, and 13/13
differential edit checks against `nim c`.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-08-28 22:38:14 +02:00
araq
3c53629164 IC: don't build interface tables for dep-of-a-dep module loads
`loadTransitiveHooks` loads a module only to register its hooks, macro-cache
replay and generic-instance offers. It is a dep-of-a-dep, not an import, so
none of its symbols are visible to the module being semchecked — yet it went
through the full `loadNifModule`, which builds the interface string tables by
calling `loadSymFromIndexEntry` on every index entry, and `processTopLevel`,
whose `export` branch calls `resolveSym` on every exported symbol. Both write
into an `interf`/`interfHidden` pair that is scratch, shared across all
iterations, and never read. Their only other effect is warming the name-keyed
`c.syms` cache, which `resolveSym` refills lazily from the same index on a miss.

New `LoadFlag.SkipInterfaceTables`, set only by `loadTransitiveHooks`, gates
both. Measured on a 219-module program: a one-line edit rebuild goes 5.58s ->
3.21s (-42%), and the compiler's own cold IC build 202s -> 164s (-19%), since
every `nim m` in a cold build pays this too. Profile of the `nim m` before:
direct imports 17 modules/1.40s, transitive hooks 209 modules/2.99s,
re-exports 39/0.23s.

This is NOT byte-inert, unlike the previous commit: skipping the eager stub
creation shifts the per-module load-order counter, so lazily-created stubs get
different serialized item ids and 44 of 238 `.s.bif` change. The artifacts stay
deterministic (238/238 reproducible across two independent cold builds) and
16/16 metamorphic tests pass, including `clean cache == incremental cache` and
`a no-op edit changes no artifact` — so an existing nimcache re-sems those
modules once and then converges. Also green: the 17-file `koch ic` suite, 13/13
differential edit checks against `nim c`, both `bootic` fixed points (debug and
release), and 482 tests across tests/arc, destructor, closure, iter, gc, async
with a failure set identical to the parent commit's.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-08-28 20:03:42 +02:00
araq
1f504865ed IC: one definition of a backend-minted symbol's disambiguator
The rule deciding which integer identifies a backend-minted symbol —
content-derived `disamb` for a lifted hook (`setHookDisamb`), `itemId.item`
otherwise — was written out at three sites: `mangleProcNameExt` and
`ccgutils.makeUnique` for the C name, `ast2nif.toNifSymName` for the NIF name,
each carrying its own copy of the ten-line rationale.

They drifted, which is exactly cce17461d: `toNifSymName` lacked the hook
exception, so the loader overwrote a content-derived value and two unrelated
`=destroy` hooks collided on one C name (C accepted the mistyped call, C++
rejected it). `astdef.backendMintedDisamb` is now the single definition and all
three call it. `globalName` still reads `disamb` directly — correct for a loaded
symbol, and the round-trip invariant that makes it agree is now stated in the
shared function instead of left implicit.

Pure de-duplication, verified as such: all 219 generated `.c` files of a
219-module corpus and the linked binary are byte-identical to the previous
commit. Plus 16/16 metamorphic IC tests, the 17-file `koch ic` suite, 13/13
differential edit checks against `nim c`, and the debug and release `bootic`
fixed points.

Note for the record: this started as an attempt to replace the per-process
`_c<itemId.item>` counter with a content hash. Instrumenting both mangling
sites showed that branch is never taken — 0 of 166 backend-minted manglings on
the corpus, 0 of 257 on the compiler, all going through the content-derived
path — so rewriting the scheme would have shifted every backend-minted C name
and forced an `icFormatVersion` bump for no demonstrable benefit.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-08-28 20:03:14 +02:00
araq
0c0cc1e496 remove owner-checking-tower 2026-08-28 15:12:56 +02:00
araq
1387093f99 don't redo nifler steps unnecessarily 2026-08-28 14:28:30 +02:00
Andreas Rumpf
546a518b22 Merge branch 'devel' into araq-ic-fixes 2026-08-28 11:10:25 +02:00
Andreas Rumpf
c87926dadf IC: more bugfixes (#26141)
Grinding a small figdraw-based program under `nim ic` and diffing its
output against the classic backend surfaced eight bugs, four of which
silently produced a wrong binary rather than an error.

Frontend / build graph (`deps.nim`):

* Dead `when`-guarded imports were compiled anyway. `when someStrdefine
== "x": import y` is `cvUnknown` to the scanner, which conservatively
keeps the edge — right for an edge, but it also gave `y` its own `nim m`
rule, so a build died on a package the user never installed because they
never selected that backend. Track which edges are speculative and drop
a speculative subtree that cannot compile; if the guard was in fact
live, the discovery fixpoint puts the node back with the honest `cannot
open file`.
* Deleting a still-imported module went unnoticed: no mtime moves, so
nothing re-fires and `nim ic` relinked a stale binary while `nim c`
reported `cannot open file`. Report an unresolvable import from a
non-speculatively reached module during the graph scan.
* Macro-generated imports were discovered once and then forgotten.
Discovery only ran after a failure and the graph is re-derived
statically every run, so on a warm build the discovered module had no
rules at all and editing it changed nothing. Seed the graph from the
`.s.deps` sidecars up front.
* Config changes invalidated nothing. nifmake decides staleness from
file mtimes and never looks at a rule's command line, so `-d:foo=bar` /
`--mm:` / `--threads:` regenerated the build file with the new switches
and re-fired zero rules. Reify the configuration as a file and make it
an input of every rule.
* Command-line switches never reached the children: they replay the
project's config files, never the driver's argv, so `nim ic --opt:speed`
produced a byte-identical debug binary (likewise `--panics`,
`--experimental`, `--passC`). Forward the driver's switches, minus the
ones that must differ per child.

Artifacts and codegen:

* A failed `nim m` still wrote its `.s.bif` and cookies, so nifmake saw
the rule as satisfied on the next run: `nim ic` then reported success
for a program that does not compile, and generated code from
error-bearing AST (or hit an internal error in `ccgexprs`). Never
persist an artifact when `errorCounter > 0`.
* Top-level destructors were never injected. `sfInjectDestructors` lives
on the module symbol, which `moduleFromNifFile` rebuilds from scratch,
so `genTopLevelStmt` skipped `injectDestructorCalls` entirely: a
module-level `block: let h = openHandle()` never ran `=destroy`. Persist
the flag as a `(modflags)` record. `injectdestructors` also has to
tolerate the `nkReplayAction` entries the loader prepends to `topLevel`.
* `nfFirstWrite` / `nfLastRead` were dropped by the serializer. A sym
node is written as a bare NIF `SymUse` token, which has nowhere to put
node flags, so the frontend's move analysis never reached the backend:
EVERY first assignment to a destructor-bearing local compiled as
`=sink`, i.e. `=destroy` on still-zeroed memory followed by a copy, and
no read was ever a move. Wrap a sym use in `(nflags ...)` when it
carries persistent node flags.
2026-08-27 19:35:11 +02:00
Zoom
bd95f88f74 js: fix var openArray write-through for toOpenArray (#26086)
In the JS backend `toOpenArray` used `slice` (a copy), so writes through
a `var openArray` parameter silently vanished.

This emits `subarray` (a live shared-buffer view) for homogeneous
numeric arrays, otherwise such parameters are passed as a `{base, off,
len}` view that always aliases the caller's storage. Sliced seq/array
args become `{base, off, len}`, whole values `{base, off:0, len}`,
re-slices rebase.

Un-skips the JS guard in tests/openarray/topenarray.nim 
Fixes #15952.
2026-08-26 18:01:06 +02:00
araq
cce17461de IC: three closure/environment fixes found by differential grinding
A closure's environment type — and the `=destroy`/`=copy`/`=sink` the compiler
lifts for it — is minted by the BACKEND and exists in no module's semmed NIF.
Grinding a corpus of closure programs against `nim c` as the oracle turned up
three ways the per-module backend gets those wrong. `tests/closure` is now green
under `--ic:on`, and `tests/iter` goes from 13 failures to 9.

* **The env hook is emitted by nobody.** `emitsBodyInThisModule` walks up to the
  outermost enclosing routine to pick the TU that emits a body. For the env
  `=destroy` of a generic closure iterator defined in one module and instantiated
  in another, that walk lands on the module of the ORIGINAL generic — a module
  that never sees the instance — so the routine was emitted nowhere
  (`undefined reference to eqdestroy__c485__…`). A backend-minted routine has no
  owning module NIF at all: it is written into the `.t.bif` of every module that
  references it, re-homed there with `@bk`. Every referencing TU emits it now and
  the merge stage keeps one, like any other content-addressed definition.

* **The `:up` link is stored without an increment.** `env.:up = enclosingEnv` has
  to go through `=copy` (with the cyclic incref) or the parent's refcount is one
  too low, and at teardown the two envs' mutually recursive `=destroy`s each
  believe they hold the last reference and recurse until the stack is gone — a
  SIGSEGV after the program's own output has already been printed
  (`tests/iter/tnestedclosures`, "Test 3"). Whether it becomes a `=copy` depends
  on the up-field type's hooks existing at injection time. Whole-program cgen got
  that for free: a LATER lifting pass creates them and it runs before any
  routine's injection. The per-module backend injects a routine right after
  lifting it (`lower`), long before the module's top level is transformed at all
  (`cg`) — so both up-field assignment sites create the ops themselves.

* **Two backend hooks collide on one C name.** A backend-minted symbol is named
  `_c<itemId.item>`, a PER-PROCESS counter — fine while "nifc lifts, emits and
  compiles them in one run", which is what `mangleProcNameExt` assumed. But
  `lower` mints the env hooks of nested routines and `cg` mints those of the
  module's top level, and both land in the same translation unit: two unrelated
  `=destroy`s became one C function. `mangleProcNameExt` already makes an
  exception for hooks whose `disamb` is content-derived (`HookDisambBit`);
  `toNifSymName` now mirrors it, so the content-derived value survives the round
  trip instead of being overwritten by the loader.

`InstanceDisambBit`/`HookDisambBit` move to `astdef` — `ast2nif` names symbols by
them and cannot import `modulegraphs`.

Two new metamorphic tests, both of which fail on the previous compiler:
`tclosure_hooks` (generic closure iterator instantiated across modules) and
`tclosure_nested_iter` (closure iterator nested in a closure iterator).

`icFormatVersion` 37 -> 38 for the hook naming. `koch bootic` reaches its
byte-identical fixed point; `tests/closure`, `tests/destructor` (3 known
`--newruntime` failures), `tests/cpp` and the classic categories are unchanged.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-08-26 12:12:51 +02:00
araq
f3bdc6c5f2 IC: nim c --ic:on / nim cpp --ic:on replace the nim ic command
`nim ic` was a command of its own, which made it the C backend only and cut it
off from everything the ordinary compile commands accept. `--ic:on` is a switch
on `nim c` / `nim cpp` / `nim objc` instead, so `-r`, `-d:release`,
`--exceptions:`, and a project-wide opt-in from `nim.cfg` / `config.nims` all
work. `nim ic` still resolves to the same driver; `koch bootic`, `koch ic` and
`testament --ic` now go through the switch.

The switch was already parsed into a `conf.ic` nobody read. It is now read in
`passCmd1` as well, because `nim.nim` has to decide whether this run is an IC
DRIVER before config loading (`ensureIcConfig` produces the precompiled config
the driver itself replays); when the switch comes from a config file instead,
`main.nim` produces it late.

**C++.** `tests/cpp` passes under `--ic:on`, matching its classic result. Four
fixes, three of them the shape of every "C++ needs the whole program" problem the
per-module backend has:

* The driver DECLARES each module's translation unit to nifmake without loading a
  module, so it cannot ask `cgen.getCFile` — and it hardcoded `.nim.c`, so the
  merge stage went looking for `.c.nif` next to the `.cpp.nif` the children had
  written. `options.icCFileExt` mirrors the formula at backend granularity.

* C++ has no designated initializers, so the RTTI record is a bare variable that
  `DatInit` fills field by field. A bare `TNimTypeV2 x;` is a tentative
  definition — C's linker merges those, C++'s does not — so every TU that
  demanded the type defined it ("multiple definition of NTIv2__…"). It now gets
  the same extern-declaration + owned-`'d'`-definition split the C flavour has.

* `memberProcsPerType` / `initializersPerType` live only in the sem process, so
  the backend emitted a struct WITHOUT its in-class member declarations and the
  out-of-class definitions did not match ("no declaration matches
  'void Doo::memberProc()'"). They are replayed from a new `(repcppmember …)`
  log entry; `replayCppMember` re-derives the type from the routine's signature
  exactly as `semCppMember` does, so no type key has to survive the round trip.

* Two follow-ons for members: `loc.snippet` is a CALL PATTERN (`#->salute(@)`),
  and only `genMemberProcHeader` derives it — whole-program cgen got it for free
  by generating the defining module first, but the per-module backend emits that
  body in another process, leaving the caller with the mangled Nim name
  (`loo->salute_u0__vireouyks1()`). And that pattern is not a linker name: every
  `salute` member in every class mints the same one, so the merge stage handed
  them all to one artifact and dropped the rest (undefined vtable at link).
  Member definitions are keyed by their NIF name there instead.

`--run` is now dropped when re-invoking for the config artifact: the producer has
no output binary and `nim.nim`'s run step asserted on the empty `outFile`.

testament's `--ic` appends the switch rather than rewriting the compile verb, so
a test that overrides `cmd:` wholesale keeps its verb, and the C++ corpus is
covered too (it never was — the old rewrite only matched `nim c `).

`icFormatVersion` 36 -> 37 for the new log entry. `koch bootic` reaches its
byte-identical fixed point through the new entry point; `koch ic` passes;
`tests/ic`, `tests/destructor` (3 known `--newruntime` failures) and the classic
categories are unchanged.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-08-26 10:32:31 +02:00
araq
7ddfc44c0f IC: six more correctness fixes found by the nim c oracle
`testament --ic` on `tests/destructor` went from 10 failures to 3; the three
left are all `--newruntime`, whose owned-ref RTTI destructor is still missing
(the object type's `=destroy` never reaches the `cg` that emits the type info).

* **global destructors are never run.** `graph.globalDestructors` is filled
  while a module's top level goes through `injectDestructorCalls`, and
  whole-program cgen empties the list into the main module's init proc — which
  IS the program body, so the calls land at exit. Under `nim ic` every module's
  `cg` is a separate process and main's only ever saw its own entries: a
  module-level `var` with a `=destroy` in an imported module was simply never
  destroyed. Each module now wraps its own list in a nullary exported proc and
  announces the name in its `.c.nif` meta head (`CnifVersion` 4 -> 5); main's
  `cg` reads the heads it already reads for init/datInit and calls them in
  reverse dependency order. New test `tests/ic/tglobal_dtors.nim` pins the
  order against the oracle.

* **`spawn` died with "system module needs: nimArgsPassingDone".** A module
  loaded from a NIF is named by its mangled suffix, so `loadCompilerProc`'s
  `module.name.s == "threadpool"` could never match. The backend loads the whole
  program before codegen, so consult every loaded module's index instead; misses
  are final and remembered (`getCompilerProc` doubles as a presence probe).

* **`new(x, finalizer)` died with "environment misses: x".** `semmagic`'s
  finalizer-to-destructor wrapper copies the original's AST but only rewrites
  the parameter, leaving `ast[namePos]` naming the ORIGINAL proc. ast2nif
  re-derives a routine's serialized AST from `ast[namePos].sym.ast`, so the
  wrapper serialized the original's body — whose parameter belongs to the
  original — and lambda lifting saw it as a captured variable of another proc.
  The copy now names itself, the invariant every other routine AST keeps.

* **a top-level `let (a, b) = f()` copied instead of moved.** A module's own
  symbol is the owner of every top-level symbol and is written as a real `(sd)`,
  so the loader minted a SECOND `skModule` PSym for it — and `sym.owner == owner`
  is an identity test in `aliasanalysis.isAnalysableFieldAccess`, which made
  every module-level location un-analysable. Hard error for a type with a
  disabled `=copy`. Bind the NIF name to the one registered module symbol.
  Backend only: doing it under `nim m` costs `times.toDateTimeByWeek` its
  inferred `raises`.

* **a doubly linked list leaked its whole contents.** A field USE serializes as
  a bare `SymUse` with nowhere to put symbol flags, so `trees.isCursor` said
  "not a cursor" for every loaded field and `DoublyLinkedNode.prev` became a
  COUNTED reference: every node held its predecessor alive and no refcount ever
  hit zero. `{.cursor.}` now rides in the NIF name marker (`` `fc `` next to
  `` `f ``), which def and use derive from the same `PSym`.

* **`--expandArc` came out shuffled.** `moduleSymbolStubs` iterated a `Table`,
  i.e. hash order, so the `lower` stage transformed a module's routines in an
  arbitrary order — not even stable between two compilers. Order by index
  offset, which is source order.

Also: `testament`'s `generatedFile` did not include the matrix entry in the
nimcache key its caller uses, so every `ccodeCheck` test with a `matrix:`
reported `reCodeNotFound`.

`koch bootic` still reaches its fixed point; `tests/ic` and the classic
`tests/destructor` are unaffected.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-08-26 08:43:42 +02:00
ringabout
dc242e9027 fixes #26124; internal error: expr: param not init with nested generic procs (#26131)
fixes #26124

The fix preserves the resolved static value, allowing constant folding.

Co-authored-by: Andreas Rumpf <rumpf_a@web.de>
2026-08-26 08:40:57 +02:00
araq
467c911dc5 IC testing: compare against nim c, and run the real corpus under nim ic
Every invariant `tests/ic` checked was IC-against-IC — clean == incremental, a
no-op edit changes nothing, a body edit moves no interface cookie. An IC that is
*consistently* wrong satisfies all of them, and that is exactly how two silent
miscompilations survived. `koch bootic` has the same blind spot: it proves the
compiler reproduces itself, not that it agrees with the reference backend.

Two mechanisms, at different scales.

**The oracle, in the metamorphic runner.** Every successful step is now also
compiled with `nim c` and run, and the two outputs must agree. Unlike the
hand-written `expect:` strings this needs no foresight from the test author: it
compares everything the program does, not only what someone thought to print,
which is what a silently-skipped destructor evades. `no-oracle` opts out.

The format also grew the expressiveness the recent bug hunt showed was missing —
every one of these described a state the suite could not reach:

* `#!DELETE <file>` removes a module. Deleting a still-imported file moves no
  mtime, so nothing re-fires.
* `#!FLAGS <switches>` changes the compiler switches between steps. A config
  change is not a file either.
* `fails: <substring>` asserts that BOTH compilers reject the program with that
  text. Previously every step had to succeed, so the whole error path — and
  recovery from it — was untested.

Six regression tests cover the eight bugs the last round fixed.
`testament r <file>` now dispatches metamorphic tests like `testament cat ic`.

**`testament --ic` runs the whole corpus through the incremental compiler**, so
IC inherits ~10k programs with expected output instead of 30 bespoke tests.
Two things had to change for that to mean anything:

* `nimcacheDir` now keys on the `matrix:` entry too. Two matrix variants of one
  file are two different compilations; sharing a cache meant each run
  invalidated what the previous left — harmless for a backend that caches only
  object files, useless for an incremental one.
* About half the corpus overrides the command wholesale (`cmd: "nim c --gc:arc
  $file"`), bypassing both `$target` and `$options`. Those are rewritten to `ic`
  and given a private cache.

**Warm cache, hastur-style.** A generated warmup program pulling in `system` and
the most-imported stdlib modules is compiled once per distinct compile
configuration into `nimcache/ic_warmup_<hash>`, and each test's empty cache is
seeded from it with mtimes preserved (nifmake compares output-mtime >
input-mtime, so stamping the copies "now" re-fires the whole graph). Only
program-independent artifacts are copied: the frontend NIFs and cookies plus the
per-module `lower`/`cg` output. The `.c`/`.o` are left behind on purpose — the
merge decision is whole-program, so they are re-rendered for every program
anyway.

`tests/destructor` (97 runs): `nim c` 35s cold / 32s warm; `nim ic` ~3m30 cold /
**9.8s warm**.

Compiler changes this required or uncovered:

* `merge` read the live-module list from a manifest the driver writes instead of
  globbing `*.c.nif` off the nimcache. Globbing absorbed artifacts belonging to
  any other program sharing the directory — which is what made a prefilled cache
  produce undefined symbols at link.
* The build-arg signature no longer includes `--icproject:`/`--icPreparsedConfig:`
  (they name where a build lives, not what it produces, so two caches holding
  identical artifacts got different signatures). The precompiled config still
  counts, by content hash, minus its `(nimcache …)` line.
* `.s.deps` seeding is speculative and runs before the prune, so a sidecar entry
  that has gone stale (an import that a `when` no longer takes) can be dropped
  instead of lingering forever; a pruned module's scan artifacts are deleted so
  an edit-accumulated cache still matches a clean one.
* A failed nifmake run no longer prints an `Error:` of its own. The children have
  already reported; adding a build-system status as the LAST error hid the
  compiler's real message from anything reading the final error — every
  reject-style test under `nim ic` said "nifmake failed with exit code: 1".
* `--mm:hooks` fed the mm mode to an on/off switch and failed outright with
  "'on' or 'off' expected, but 'hooks' found". Pre-existing and unrelated to IC;
  only reachable through the explicit switch, since `--newruntime` sets
  `selectedGC` directly.

Running `tests/destructor` under `--ic` currently leaves 10 failures. They are
genuine IC defects, not harness noise (all 97 pass under `nim c`) — the clearest
is `tglobaldestructor`: `graph.globalDestructors` is accumulated while injecting
destructors into a module's top level, but the main module's `cg` — which emits
the teardown — is a different process, so a module-level `var` with a `=destroy`
is never destroyed. Same shape as the init/datInit metas, and it wants the same
fix: record it in the `.c.nif` head.

Validation: `koch bootic` reaches its byte-identical fixed point; `tests/ic` is
36/36; arc, destructor, macros, template, iter, closure, ccg, codegen, types and
effects pass under `nim c`, with generics showing only its pre-existing failure.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-08-25 21:00:13 +02:00
araq
c01e58c146 IC: eight correctness fixes found by differential testing against nim c
Grinding a small figdraw-based program under `nim ic` and diffing its output
against the classic backend surfaced eight bugs, four of which silently produced
a wrong binary rather than an error.

Frontend / build graph (`deps.nim`):

* Dead `when`-guarded imports were compiled anyway. `when someStrdefine == "x":
  import y` is `cvUnknown` to the scanner, which conservatively keeps the edge —
  right for an edge, but it also gave `y` its own `nim m` rule, so a build died
  on a package the user never installed because they never selected that backend.
  Track which edges are speculative and drop a speculative subtree that cannot
  compile; if the guard was in fact live, the discovery fixpoint puts the node
  back with the honest `cannot open file`.
* Deleting a still-imported module went unnoticed: no mtime moves, so nothing
  re-fires and `nim ic` relinked a stale binary while `nim c` reported `cannot
  open file`. Report an unresolvable import from a non-speculatively reached
  module during the graph scan.
* Macro-generated imports were discovered once and then forgotten. Discovery
  only ran after a failure and the graph is re-derived statically every run, so
  on a warm build the discovered module had no rules at all and editing it
  changed nothing. Seed the graph from the `.s.deps` sidecars up front.
* Config changes invalidated nothing. nifmake decides staleness from file mtimes
  and never looks at a rule's command line, so `-d:foo=bar` / `--mm:` / `--threads:`
  regenerated the build file with the new switches and re-fired zero rules.
  Reify the configuration as a file and make it an input of every rule.
* Command-line switches never reached the children: they replay the project's
  config files, never the driver's argv, so `nim ic --opt:speed` produced a
  byte-identical debug binary (likewise `--panics`, `--experimental`, `--passC`).
  Forward the driver's switches, minus the ones that must differ per child.

Artifacts and codegen:

* A failed `nim m` still wrote its `.s.bif` and cookies, so nifmake saw the rule
  as satisfied on the next run: `nim ic` then reported success for a program that
  does not compile, and generated code from error-bearing AST (or hit an internal
  error in `ccgexprs`). Never persist an artifact when `errorCounter > 0`.
* Top-level destructors were never injected. `sfInjectDestructors` lives on the
  module symbol, which `moduleFromNifFile` rebuilds from scratch, so
  `genTopLevelStmt` skipped `injectDestructorCalls` entirely: a module-level
  `block: let h = openHandle()` never ran `=destroy`. Persist the flag as a
  `(modflags)` record. `injectdestructors` also has to tolerate the
  `nkReplayAction` entries the loader prepends to `topLevel`.
* `nfFirstWrite` / `nfLastRead` were dropped by the serializer. A sym node is
  written as a bare NIF `SymUse` token, which has nowhere to put node flags, so
  the frontend's move analysis never reached the backend: EVERY first assignment
  to a destructor-bearing local compiled as `=sink`, i.e. `=destroy` on
  still-zeroed memory followed by a copy, and no read was ever a move. Wrap a sym
  use in `(nflags ...)` when it carries persistent node flags.

`icFormatVersion` 34 -> 35 for the two new NIF records.

Validation: `koch bootic` reaches its byte-identical fixed point; two clean-cache
builds from the same compiler are identical; testament `arc`, `destructor`,
`macros`, `template`, `iter`, `closure`, `ccg`, `codegen`, `types` and `effects`
pass, and `generics`, `ic` and `stdlib` show exactly the pre-existing failures.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-08-25 19:24:08 +02:00
Andreas Rumpf
8ca7b75b8b refactoring: better IC + no unique Id (#26137) 2026-08-25 11:59:19 +02:00
ringabout
2d1412a2ea fixes #26015; Multiple definition error when using codegenDecl regression (#26018)
fixes #26015

Fixes imported global variables with codegenDecl being emitted as
definitions instead of extern declarations.

A variable’s codegenDecl format should customize its definition in the
owning module. Other modules referencing the variable must emit a normal
declaration:

```c
extern NI variable;
```

After the variable-declaration builder refactor, genVarPrototype passed
Extern visibility to addVar. However, the sfCodegenDecl branch returned
before applying that visibility. This caused importing modules to emit
another tentative definition, resulting in duplicate-symbol linker
errors.

The fix restores the previous distinction between the custom definition
and cross-module prototypes. It also adds C and C++ regression coverage
for both direct access and access through an inline procedure.

follows up https://github.com/nim-lang/Nim/pull/24423
2026-08-23 12:37:15 +02:00
ringabout
f1256ddcf4 fixes #26123; Update PathKinds1 to include nkCast (#26126)
fixes #26123

`cast[T](x)` is a transparent path expression for compiler analysis.
Previously, move/alias analysis could fail to see a later use through a
cast and incorrectly mark the source as moved, causing the issue’s
segmentation fault.


for views,
https://nim-lang.org/docs/manual_experimental.html#view-types-path-expressions:
A cast expression cast[T](e) is a path expression.

It also affects skipConvDfa, isAnalysableFieldAccess, and aliases. And I
might narrow it down for the two cases above mentioned if it causes
problems
2026-08-21 21:57:51 +08:00
SirOlaf
901ca7905a IC: Do not serialize nfHasComment to nif (#26127)
It causes non-deterministic behavior because it's process-local.
2026-08-20 18:11:31 +02:00
ringabout
1201c184d7 fix #26112: update variable kinds in isPartOf to include skResult (#26114)
fix #26112
2026-08-17 23:37:46 +02:00
Jacek Sieka
5f5cf8dd03 rm some cruft (#26113)
`XDeclaredButNotUsed` for years in most cases - there's more but this is
the low-hanging fruit
2026-08-17 15:02:49 +02:00
ringabout
16920b56d1 fixes #25992; fix GC tracing of stale bytes in case objects during reset (#26003)
fixes #25992
```nim
type
  Foo = object
    case kind: bool
    of true:
      a: ref Bar   # 8 bytes (pointer)
    of false:
      b: int       # 4 bytes
```
specializeResetT for b emits accessor.b = 0 — writes 4 bytes
But the union is 8 bytes wide (sized by the largest branch)
The remaining 4 bytes where a used to live are untouched
Those stale bytes could contain a heap pointer the GC traces → crash

Add nimZeroMem after specializeResetN for case objects to clear the
entire union including unused branch bytes.
2026-08-15 07:51:13 +02:00
Jacek Sieka
10f0e5e9ac rm genCaseObjDiscMapping (#26097)
No longer used
2026-08-15 07:48:58 +02:00
Jacek Sieka
f489afa7e4 deprecate hotCodeReloading (#26107)
See https://github.com/nim-lang/RFCs/issues/573 - deprecating for
visibility in 2.4, in case a maintainer wants to step up - else it can
be binned for 2.6
2026-08-15 07:45:58 +02:00
ringabout
cbb3b065c7 fixes #26104; prevent compile-time-only typeof from being treated a… (#26105)
…s a runtime alias

fixes #26104

Follows up https://github.com/nim-lang/Nim/pull/25994
2026-08-14 13:36:55 +02:00
Andreas Rumpf
ebfd1c5090 fixes #26025 (#26076) 2026-08-11 22:27:49 +02:00
SirOlaf
2d22f24359 Same-module generic cache for lazy instantiation (#26091)
We defer copying the AST during generic instantiation until a cache miss
and fetch from cache based on bindings. On miss, we fall back to the old
logic and populate the cache.
This gains us a roughly 36% reduction in memory usage during bootstrap,
from `799.242MiB` down to `508.672MiB` on my machine.

For another point of reference, nimbus-eth2 goes from `10.5GB` memory
usage to `7.5GB`.

Independent companion to #26090 which together with this one yields a
bit under 20% faster compiles (or at least `--compileOnly` bootstraps)
on ORC.
2026-08-10 10:18:15 +02:00
ringabout
0ec8682abe fixes #26062; ResultUsed warning behaves inconsistently with manual c… (#26087)
…haracterization with--warning:ResultUsed:on


fixes #26062


> A return statement with no expression is shorthand for return result.

> ResultUsed: Warn about the usage of the built-in result variable.

> A procedure that does not have any return statement and does not use
the special result variable returns the value of its last expression.
2026-08-10 07:38:49 +02:00
Corey Leavitt
f4e8e04cd0 fixes #26092; restore enclosing cast block state when a nested cast block exits (#26093)
`unapplyBlockContext` reset `inEnforcedGcSafe` and
`inEnforcedNoSideEffects` to false whenever a `{.cast(gcsafe).}` or
`{.cast(noSideEffect).}` block ended. When such a block is nested inside
another block of the same cast, the inner block's exit switched
enforcement back on for the rest of the enclosing block, so statements
lexically inside the outer cast were rejected.

The fix saves both flags in `PragmaBlockContext` when the block context
is created and restores the saved values on exit. That matches how every
other piece of block state there (`locked`, `exc`, `tags`, `forbids`) is
already handled; these two bools were the only ones reset to a constant
instead of restored.

No change for non-nested blocks: entering from the non-enforced state
saves false, so exit still clears the flag. A statement after the outer
block is still rejected as before.

Test covers nested `cast(gcsafe)` and nested `cast(noSideEffect)`.
`tests/effects` passes unchanged (49/49, same as stock).
2026-08-09 11:24:02 +02:00
SirOlaf
f0e7969bb0 Miscellaneous frontend optimizations (#26090)
Boostrap without linking is (conservatively) 17% faster on my machine.

Test setup runs the compiler compiling itself under ORC in release mode
from a clean cache and no C compilation (`--compileOnly`.)
Multiple samples are gathered before judging a potential optimization.
The test suite passes locally before push and bootstrap is tested with
strict views (mostly as a sanity check).
Tests are fair in the sense that they compile the same git worktree and
produce identical C output.

Refc will see less or no benefit.


Changes:
- Save tree traversals in `considerGenSyms` when no mappings exist
- Inline `maybeSkipDistinct` into `typeRel` for a safe cursor
- Only skip to static when there is a static type to reach in
`paramTypesMatchAux`
- Disable overflow checks for hashes
- Disable bounds checks for `nextIdentIter`
- Use cursor annotation when judged safe
- Use lent annotation for ast accessors


Spiritual companion to #26084
2026-08-08 23:28:22 +02:00
ringabout
050b38c749 fixes #26036; compiler inference for sfNeverRaises (#26059)
fixes #2603

---------

Co-authored-by: Andreas Rumpf <rumpf_a@web.de>
2026-08-07 13:08:51 +02:00
Emmanuel M. Smith🔸
27763495bc lexer: add seven more unicode operators (#26074)
closes nim-lang/RFCs#571

Adds `⟑ ⟇ ⩓ ⩔ ■ □ ☆` with the same priority as `*`.

Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: Andreas Rumpf <rumpf_a@web.de>
2026-08-05 16:12:17 +02:00
Jacek Sieka
c69cf36610 Simplify C file change detection (#26080)
Remove `moduleHasChanged`
2026-08-05 10:32:57 +02:00
Andreas Rumpf
0206aa334c backend: refactorings so that eventually it can run on BIF directly w… (#25959)
…ithout PNode constructions; also added bif2nif.nim inspection tool
2026-08-04 15:18:34 +02:00
nimamasl114514
1e82deb73d fix #20078: nimpretty --indent applies to keepIndents regions (#25985)
## Summary
- nimpretty with non-default \--indent\ (e.g. 3 or 10) produced invalid
indentation in if/block/try expression regions because layouter kept the
original column when \keepIndents > 0\ and ignored \indWidth\.
- Rebase the column onto \indWidth\ using the relative offset from the
enclosing block baseline (\indentStack[^1]\).

## Root cause
\parser.nim\'s \
imprettyDontTouch\ template sets \keepIndents\ for if/block/try
expressions. layouter in the \keepIndents > 0\ branch used \ ok.indent\
(source column) directly as \indentLevel\ without scaling by \indWidth\,
so lines in these regions kept the original column and misaligned with
the rest of the file when \--indent\ differed from source indent width.

## Fix
\\\
im
em.indentLevel = em.indentStack.high * em.indWidth +
                 (tok.indent - em.indentStack[^1])
\\\

Keeps the relative offset from the enclosing block baseline but rebases
onto \indWidth\. At default \--indent:2\ the offset equals \indWidth\,
so output is unchanged (backwards compatible).

## Testing
- 12 custom cases x 3 indent values (2/3/10) = 36/36 pass
- nimpretty self-test suite 7/7 pass (no regression at default indent)
- 5 keepIndents scenarios (if/block/try expression continuation
alignment) that failed at indent:3/10 now pass

Fixes #20078.
2026-08-03 12:15:44 +02:00
ringabout
23365deef0 fixes #26023; incorrect sink requires a copy (#26028)
fixes #26023
2026-08-03 12:02:29 +02:00
ringabout
234f01510f fixes #25942 #25938; type inference for static container type (#25989)
fixes #25942 
fixes #25938

After a successful match to a concrete static T, normalizes an empty
static container literal to the formal payload type before binding it.
This prevents `static[set[empty]]({})` from leaking into the
instantiated proc body.
2026-08-03 11:52:04 +02:00
Ryan McConnell
5137d273e5 fix #25993; In-place object construction zeroes destination before evaluating self-referencing field values (#25994) 2026-08-03 11:51:16 +02:00
ringabout
f6651e6c70 fixes #26045; #26046; when nimvm leak push options (#26047)
fixes #26045; 
fixes #26046

The fix isolates compiler option state while semantically checking each
when nimvm branch.

compiler/semexprs.nim:2745 snapshots the option stack, compiler options,
diagnostics settings, and enabled features. It analyzes one branch and
restores that state in finally. Both the nimvm and else branches use
this function.

This prevents:

```nim
when nimvm:
  {.push overflowChecks: off.}
```

from disabling overflow checks in following runtime code. It also means
a {.pop.} in the opposite branch correctly reports that it has no
corresponding {.push.}.
2026-08-03 11:49:48 +02:00
Andreas Rumpf
3126a47590 YRC: optimizations (#26042) 2026-07-30 14:54:12 +02:00