mirror of
https://github.com/nim-lang/Nim.git
synced 2026-09-01 19:33:42 +00:00
dbaed3d38aaa91ab5cee33fa654ba1eeb847ceb0
23221 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
araq
|
dbaed3d38a |
IC: measure the whole backend, and record where its time actually is
The profiling added with the cursor path was local to `bnode`, so it could only
answer questions about the cursor path. Moved to `compiler/icprof.nim` — no
compiler imports, so any stage can use it without a cycle — and extended to the
stage boundaries: the closure load and its three phases, `transformBody`,
`handOffBody`, `genProcBody`.
The budget that produces, on a cold `--ic:on` build of a 68-module target
(10.1s wall, summed over 177 backend processes):
loadDepClosure 3306ms of which moduleId 1285ms
processTopLevel 1516ms
interface tbls 1323ms
genProcBody 333ms
handOffBody 60ms
transformBody 28ms
This is worth having written down because it reprices the migration this branch
is doing. Reading a routine body off a cursor rather than a tree is finished and
costs nothing — `genProcBody` is the same either way. But FINISHING the job,
reading a `.t.bif` body directly and never materialising the `PNode`, can only
win back `handOffBody` + `transformBody`: under 1% of the build. The 41% is in
getting the closure's INTERFACE into memory, which no amount of body-reading
touches.
The remaining blockers in `bnode`'s header — `TLoc.lode` above all, 72 call
sites and hard, because a symbol's `loc.lode` outlives the body it was built in
and `lode == nil` is a sentinel — are worth exactly that under 1% until
something else changes. Said so in the header, replacing the older 0.20s/0.16s
figures, since that paragraph is the map read first.
The obvious lever on the real cost was tried and is not taken:
`{SkipInterfaceTables}` for dep-of-a-dep loads in `loadDepClosure` builds and
runs correctly but returns ~200ms, because most of that phase is the target and
system modules rather than the transitive ones. Not worth a name that silently
fails to resolve, so the flag stays restricted to `loadTransitiveHooks`.
Verified: both configurations build; `tests/ic` 40/40; the instrumentation
changes no codegen — 67/67 `.c` identical, cursor still identical to `PNode`.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FMyRHByv7hhaQJ4Pa1bHbE
|
||
|
araq
|
727d70d2ba |
IC: keep the node bridge out of the nimKochBootstrap build
`koch boot` failed with `bodynav.nim(278) Error: undeclared identifier:
'program'`. `transf` imported `nodebridge` unconditionally, which pulls in
`bodynav`, which resolves names through `ast.program` — and `program` does not
EXIST under `-d:nimKochBootstrap`. That define disables the IC subsystem
wholesale: `ast.nim` guards `program` and the loader callbacks with it, and
`koch.bootic` records that it also disables `commandIc`. The bridge was being
compiled into a build with nothing for it to talk to.
Guarded the same way `ast.nim` already guards `program`. `handOffBody` goes
with it; its only caller is `cgen`, under `-d:newIcBackend`.
`koch temp` cannot catch this — it does not set the define — so the break
survived several rounds of verification that all used it. Predates this
branch's recent work: `transf`'s unconditional import and `bodynav`'s use of
`program` are both there at
|
||
|
araq
|
2db27d4826 |
Merge branch 'devel' into araq-ic-fixes2
One conflict, in `canRaiseDisp`, where both sides added to the same guard: * devel (#26145) short-circuits `skMethod` to "can raise", because a base method's inferred effects describe only the base body, not every vtable target; * this branch resets `markCanRaiseBranch` to 5 on entry, so that a `-d: icCanRaiseLog` differential attributes an answer to the branch that actually decided it rather than to whatever the previous call left behind. Both kept: the reset first, then devel's method branch, then the existing flags branch. Marker 5 already means "decided here, neither predicate ran", which is what the new branch does too, so it needs no new number — the comment now says both short-circuits land there. Verified on the merged tree: `tests/ic` 40/40, including devel's new `timportcalias`; the cursor-driven and `PNode`-driven backends still generate byte-identical C (67/67 under `--ic:on`); ccgbugs 146, concepts 48, method 22, destructor 97, arc 140, gc 78, closure 23, iter 71, exception 47, all clean. `tests/generics/tparser_generator.nim` fails, and does NOT come from this merge: it fails the same way on pristine `origin/devel`, built and run in a throwaway worktree to check. The compile succeeds; the spec expects no output and the compiler now emits `typed`-deprecation and unused-import warnings. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FMyRHByv7hhaQJ4Pa1bHbE |
||
|
araq
|
d27e1e2712 |
IC: give the cursor path a way to be measured, and record what it cost
The last commit's finding took three refuted hypotheses to reach. The switches
that produced it are kept, and so is the answer, because the next person will
ask the same question:
* `-d:icBNodeProf` counts every accessor and times the phases (`handOffBody`,
`genProcBody`, the analyses, `sym`/`typ`/`info`/`origin`). Each backend
process appends a line to `$NIM_IC_BNODE_PROF`, so a parallel build still
produces attributable output. Costs nothing when the define is off.
* `-d:icBridgeOnly` builds the buffer but generates off the tree. It is a
MEASUREMENT switch, not a mode, and it is the only way to separate what the
encoder costs from what reading costs — which is how encoding was shown to
be free and the reader identified as the thing to profile.
Cursor-driven generation is now the same speed as tree-driven: over the 2420
routines of a 68-module target, `genProcBody` is 369ms off a cursor against
366ms off a `PNode`. It was 1877ms.
The three suspects that measured out as wrong are written down so they are not
re-guessed: the tag-name string compares in `typ`/`flags` (0.3% of the build),
`Cursor`'s reference-counting lifetime hooks (6M calls, 60ms of 1900ms), and
the structural accessors as a whole — `kind`, `son` and the iterators together
are 32ms of 1877ms.
The one real change here is the tag memo, which now resolves a tag's
`TNodeKind` and its wrapper role together instead of comparing tag NAMES on
every `typ`/`flags`/`hasExplicitNilType` call. Small — it is the 0.3% above —
but it removes 197k string comparisons and lets those three read a `case`.
`tagCachePool` holds the pool by reference for the same reason `readPool` does:
that is what stops a freed pool from being replaced at the same address.
Verified with the parent commit: `tests/ic` 39/39, grinder clean, `.c`
byte-identical both under `--ic:on` and without it.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FMyRHByv7hhaQJ4Pa1bHbE
|
||
|
araq
|
db47363784 |
IC: memoize the FileId -> FileIndex resolution, halving a cold --ic:on build
`oldLineInfo` resolved every decoded token's line info by copying a path out of the buffer's filename pool and hashing it through `msgs.fileInfoIdx`. Every time — no cache, on a step that runs once per node the decoder loads and once per statement the generator emits. On a 68-module target that is 259k calls at 5.2us. Memoized per pool it is 34ms, and the cold build goes from 21.5s to 10.1s. `revTab` could not serve this: it is keyed by a `FileId` in the WRITER's global `pool.files`, while a decoded token's `FileId` indexes the buffer's OWN filename pool — a fresh one per `bif`-loaded module. So the cache is keyed by (pool, FileId), as a `seq` because ids are small and dense within one pool. `readPool` holds a REFERENCE rather than a raw pointer, and that is the whole safety argument: it keeps the pool alive, so a freed pool cannot be replaced by a new one at the same address and answer from the wrong file table. Verified: `tests/ic` 39/39; the cursor-vs-`PNode` differential grinder clean over ~150k graded nodes on `tools/icgrind` — and confirmed live by sabotaging the pool-identity check, which makes it fire on `info` immediately; all 67 `.c` files an `--ic:on` build generates byte-identical to before; all 216 a non-IC build generates likewise. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FMyRHByv7hhaQJ4Pa1bHbE |
||
|
Araq
|
8afd306b0d |
IC: correct the module docs to the state the seam is actually in, and measure it
`bnode`'s header still said the generator "has to move together, and it needs
write-side capability this seam does not have". It has moved, and `origin` was
the write-side answer, so the section described a state two commits out of date —
the kind of stale map that sends the next person looking for a problem that is
already solved. Rewritten to say what runs on a cursor now, that `origin` is what
keeps `TLoc.lode` a `PNode`, and that the generator's own in-place rewrites run
on the origin (with the hazard spelled out: where a mutation is read back,
generation has to continue on the origin, because the buffer is a snapshot).
Two blockers were also described wrongly:
* THE ALIAS FAMILY is no longer blocked by field identity — that is exact on a
bridged buffer. I checked whether `isPartOf` could migrate now and it cannot,
for a different reason: every call site passes `d.lode` as one operand and that
is a `PNode`, so a generic `isPartOf` would still be handed a `PNode` on one
side and buy nothing. It moves when `TLoc.lode` does.
* `sym`'s note said `isPartOf` "CANNOT be migrated as written" without
qualifying that this is a FILE-path property; a bridged buffer hands back the
object it was given, and the grinder asserts exactly that.
And the cost, which I flagged twice as unmeasured and is now measured on a
50-module target:
baseline (PNode) 6.75s
bridge built, not read 6.79s -- encoding is inside the noise
generator driven off it 8.85s -- +31%
So the encoder is as cheap as claimed and the whole cost is in READING: `son` is
O(i), `kind` indexes a memo per call, `sym`/`typ` go through the nav, `origin` is
a hash lookup per location built. None of it is inherent and none of it has been
optimised. Since a compile is mostly frontend, codegen itself is slowed by well
over 31%.
Verified: both configurations build; cursor-driven and `PNode`-driven output
still identical.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XEF7FJvUkGKvG9LSGuEaNR
|
||
|
Araq
|
603953376d |
IC: add -d:icSymCount, and use it to settle the full-compiler comparison
Counts every `newSym` mint, by kind, and reports on exit. A gensym's number is its item id, so ONE extra symbol anywhere shifts every later name in the generated C — which makes a mint count far more sensitive than diffing output, and localises a difference by symbol kind instead of by whichever file happened to show it. It was written to answer a question I had reported wrongly. Comparing the cursor-driven and `PNode`-driven builds over the whole compiler showed 3 of 215 `.c` files differing, and I attributed 2 of them to the compiler being nondeterministic. That control was invalid: I had compared two runs across an edit to `ccgexprs.nim`, so what I read as nondeterminism was line numbers moving inside assertion strings. The compiler is DETERMINISTIC — same binary, same source, twice, 0 of 215 differ. What the count establishes: the two builds mint exactly the same symbols, 515_551 of them, with no per-kind difference at all. So nothing in the cursor path creates or skips a symbol, and the earlier gensym-numbered differences — which had zero non-gensym lines — were ordering, not extra work. On the current tree the comparison is clean: cursor-driven and `PNode`-driven produce BYTE-IDENTICAL `.c` for all 215 files of `compiler/nim.nim`. I have not isolated why the same comparison showed 3 differences one commit earlier; the commit between was behaviour-neutral by inspection, so I am recording the observation rather than a claim about its cause. Verified: both build configurations compile; the diagnostic is behind a define and costs nothing otherwise. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XEF7FJvUkGKvG9LSGuEaNR |
||
|
Araq
|
31a7b6bc85 |
IC: undo gratuitous rewrites the bulk migration made
Reading the previous commit's diff, as promised, turned up edits that were semantically neutral but said something false: a regex pass had rewritten `t.n == nil` (a TYPE's record tree) as `t.n.isNilNode`, done the same inside the grinder where both sides are `PNode`s, and relabelled `asgnComplexity` and `containsOpaqueImportcFieldAux` as `AnyNode` when both walk `PType.n` and can never see a cursor. It also edited a comment. None of it changed behaviour — `isNilNode` on a `PNode` is `== nil` — but it made unrelated code look like it participates in the seam, which is exactly the kind of noise that makes a later reader think the type-record walkers are migration candidates when the module doc says they are not. Reverted, and the two type-record procs now say why they stay. Verified: both builds compile; cursor-driven and `PNode`-driven output still identical (50/50). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XEF7FJvUkGKvG9LSGuEaNR |
||
|
Araq
|
709fd00861 |
IC: migrate expr and the emitters — cgen generates from a cursor
`genProcBody` is handed `BNode(bodyBuf.rootCursor)` under `-d:newIcBackend`, so
`expr` and the ~160 procs under it read the routine body through a cursor rather
than a tree. This had to land as one change: `expr` dispatches to all of them, so
they move together or the dispatch converts at every node.
The evidence that it works is not that it compiles. Cursor-driven and
`PNode`-driven builds emit BYTE-IDENTICAL `.c` (50/50 on an 89k-line target,
12/12 on the grind target), the built program runs and prints the right thing,
and — the part that makes the first number mean something — sabotaging
`bnode.intVal` changes all 12 files. The generator is genuinely reading through
the cursor, not quietly falling back.
Four kinds of site could not simply take `AnyNode`, and each is marked where it
sits rather than left for the next person to rediscover:
* THE GENERATOR REWRITES. `mAppendSeqElem`, `mNewSeq`, `genSetLengthSeq`,
`genWasMoved` and `genArrToSeq` replace a child or a type IN PLACE, and
`genEnumToStr`/`mAsgn`/`spawn` build fresh trees. Those run on `origin(n)` —
the very node the buffer was encoded from — so the mutation lands exactly
where it always did. Where the mutation is then READ (`genArrToSeq` retypes a
bracket, `genArg` replaces a `var` param's type), generation continues on the
origin too, because the buffer does not see the write and a cursor would keep
reading the slot as encoded.
* NILABLE NODES stay `PNode`: a cursor has no standalone nil. That is the
assignment DESTINATION throughout the call family (`genCall` passes nil), the
`check` of an object-constructor field, `exvar`, `stepNode`, the `fin` of a
try statement.
* `PNode`-KEYED TABLES AND ANALYSES take `origin`: `dataCache`, `isPartOf`,
`lhsDoesAlias`, `potentialAlias`, the type-record walkers.
* SHARED PREDICATES in `ast.nim` cannot see `BNode`, so `skipHiddenAddr`,
`isInfixAs` and `getStr` join `canRaise`/`getInt` as templates instantiated
for both. `skipPragmaExpr` is a deliberate exception: it sits above the point
in `ast.nim` where `firstSon` for a `PNode` exists, so `bnode` carries a
one-line spelling with a pointer back.
Two Nim details worth recording. Repeated occurrences of a type class in one
signature share ONE implicit generic, so any proc whose two node parameters can
differ in representation needs explicit params — `genSingleVar`,
`genFieldObjConstr`, `callGlobalVarCppCtor`. And a `{.dirty.}` template inside a
generic resolves its identifiers at instantiation, so `genClosureCall`'s local
`rawProc` had to be bound before the template that uses it or it lost to the
module-level proc of the same name.
Verified: grind clean (1431 bodies, 260_431 nodes, 0 disagreements, origins
exact); the default path is byte-identical to HEAD; all four build
configurations compile.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XEF7FJvUkGKvG9LSGuEaNR
|
||
|
Araq
|
e5bafa48c5 |
IC: make the location primitives representation-agnostic
`initLoc`, `fillLoc`, `putIntoDest` and `putDataIntoDest` take `AnyNode` and store `origin(lode)`. That is the gate the previous commit was for: 99 of the 180 generator procs build a `TLoc` from their node, so until these four accept a cursor none of those 99 could migrate, and with them accepting one they all can — `TLoc.lode` is still a `PNode`, and on a bridged buffer it is the SAME `PNode` a tree-driven build would have stored. `bnode.origin` is the ambient accessor (through `currentNav`, like `sym` and `typ`), so a generator proc does not have to be handed the buffer to build a location. It answers nil for a file-backed body, which is correct — those tokens came from no `PNode` — and asserts on a bridged one, where a node head always has an origin and a miss means the cursor is not where the caller thinks. The origin check in the grinder now goes through that ambient path rather than calling `originOf` directly, because the direct call is not the path the generator will take and testing it would have proved the wrong thing. Verified: grind clean, origins exact by reference at every node of 1431 bodies; the default path is byte-identical to HEAD; all four build configurations compile. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XEF7FJvUkGKvG9LSGuEaNR |
||
|
Araq
|
9f34f5e42b |
IC: give the bridge origin tracking, so TLoc.lode stops blocking the generator
Measured before designing: of the 180 `PNode`-taking procs in `cgen`/`ccgexprs`/ `ccgstmts`/`ccgcalls`, **99 build a `TLoc` from a node**. So the generator cannot move to the seam without an answer for `TLoc.lode`, and the obvious answers are both bad — leaving it a `PNode` means a cursor-driven proc cannot fill it, and changing its representation means editing `TLoc`, which lives in `astdef` at the bottom of the module graph, pushing the seam far below the backend and forcing a flag day. There is a third answer, and nifcore already had the piece it needs. `cursorToPosition` is documented as a stable per-token key, and `TokenBuf.len` is where the next token lands — so the encoder records `position -> PNode` as it walks, and `originOf` inverts it. A cursor-driven generator can then put the ORIGINAL node into a location: `TLoc.lode` stays a `PNode`, and the identity comparisons already in the backend (`preventNrvo`'s `dest != le`, `isPartOf(d.lode, …)`) keep meaning what they meant, because it is the same object and not an equal copy. Asserted, not assumed: the bridge grinder now walks cursor and tree together and requires `originOf(c) == a` by REFERENCE at every node — 1431 bodies, 0 failures. Recording the position one token off makes it fail on the first body, so the check is not vacuous. This unblocks the generator migration without a flag day: `expr` and its ~60 emitters can move to `AnyNode` with `TLoc` untouched. Verified: grind clean (1431 bodies, 260_431 nodes, 0 disagreements); the default path is byte-identical to HEAD; all four build configurations compile. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XEF7FJvUkGKvG9LSGuEaNR |
||
|
Araq
|
e988e0366c |
IC: hand the transformed body to cgen as a buffer, and read the analysis off it
`transf.handOffBody` is the seam between the two halves: rewriting ends, and from there the reading side works off a cursor. `cgen.genProcLvl3` takes the handoff once the body is final and runs `allPathsAsgnResult` — which has been `AnyNode` since the predicates landed — over the CURSOR rather than the `PNode`. Three things this had to get right, none of them obvious from the outside. WHERE the handoff goes. A bridged buffer is a SNAPSHOT, so it has to be taken after the last rewrite. Destructor injection runs after `transformBody` returns, and `easyResultAsgn` sets `nfPreventCg` on the tree later still — a buffer taken at the end of `transformBody` would describe a tree that no longer exists by the time anything read it. So the API lives in `transf`, which owns that invariant, and the call site is in `cgen`, which is where rewriting actually finishes. The one flag `easyResultAsgn` writes afterwards is read only by the `PNode` generator, and it sits in the branch that does not run the analysis at all. WHY it is gated on `-d:newIcBackend`. `expr` cannot migrate a piece at a time: it dispatches to ~60 emitters, so either all of them take `AnyNode` or the dispatch converts at every node. Until that happens the generator still needs a `PNode`, and building a buffer per routine in a default build would cost a tree walk and buy nothing. The analyses are the part that moves now. THAT IT IS ACTUALLY LOAD-BEARING. "Byte-identical output" is worth nothing if the new path never ran, so: cursor-driven and `PNode`-driven builds produce identical `.c` (50/50 on an 89k-line target, 12/12 on the grind target), and forcing the cursor-driven answer wrong changes 10 of 12 files. The first number alone would have been consistent with the code being dead. Verified: grind clean (1431 bodies, 260_431 nodes, 0 disagreements, neither tolerance taken); the default path is byte-identical to HEAD; all four build configurations compile. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XEF7FJvUkGKvG9LSGuEaNR |
||
|
Araq
|
47144c7962 |
IC: a PNode <-> TokenBuf bridge, so rewrites stay on PNode
The generator does not migrate to a `Cursor` and should not: transf, destructor injection, closure lifting and the tree codegen builds as it goes all CONSTRUCT nodes, and a cursor is a read pointer into a shared token buffer. `nodebridge` is the seam instead — a rewriting pass keeps producing a `PNode`, and anything that only reads is handed a `TokenBuf`, from which a `Cursor` (and so a `BNode`) is a pointer. The bridge is NOT the `.bif` format, and the difference is the point. A `.bif` is read by a different process, so every symbol and type has to be written as a NAME to look up again. A bridged buffer is read by the process that built it, so a symbol reference is `(bsym <idx>)` into a side table holding the very `PSym` the encoder was handed, and the type slot is `(btyp <idx>)` the same way. The node shape is otherwise identical to the file format, so `bnode` reads a bridged buffer with the accessors it already has; `bodynav` grows one branch each in `symAt`/`typeAt`, and `bnode` learns that `bsym` is an `nkSym`. That buys the property this branch has been blocked on: **`sym` IS IDEMPOTENT ON A BRIDGED BUFFER, FIELDS INCLUDED.** On the file path it cannot be — `loadFieldStub` mints a fresh stub per use because two distinct fields can share a name and a position across types — which is what stops `aliases.isPartOf` moving to the seam. A bridge hands back the object it was given. The grinder now asserts exactly that: field syms are excluded from the idempotence check on the file path and INCLUDED on a bridged one. Verification is the existing oracle pointed at a harder target. `grindBNode` compares two decodings of one file and has to excuse two differences; the bridge is compared against its own live input and must excuse NEITHER, so both tolerances are counted and the bridge asserts it took neither. `toPNode` is covered without a hand-written comparator that could share the encoder's bugs: decode, RE-ENCODE, and grade the second buffer against the ORIGINAL tree, so anything the decoder drops shows up as a disagreement. Reach, measured: 1431 bodies and 260_431 nodes, against 782 and 67_857 for the file path — the bridge sees every body, including the one-line `nkAsgn` ones `ast2nif` never defers and the grinder therefore never saw. 0 disagreements. Not vacuous: dropping node flags, perturbing the sym index and dropping the type slot each make it fail immediately (`flags`, `sym identity`, `typ nil-ness`). A bridged buffer must never be written to a file — `(bsym …)` means nothing without the tables beside it. `ast2nif` remains the only serializer. Verified: 215/215 byte-identical `.c` against HEAD on the default path; all four build configurations compile, plus `nodebridge` standalone. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XEF7FJvUkGKvG9LSGuEaNR |
||
|
Araq
|
bca07265a4 |
tests/ic: say how to run the suite, and how to tell slow from hung
The metamorphic tests compile the program twice per step — once under `nim ic`, once as the `nim c` reference — which is 100+ compilations for the category, each fanning out a compiler process per module per stage. On a machine short of RAM that swaps, and the symptoms read exactly like a deadlock: processes at 0% CPU, no output, a different test "stuck" every run, and the same compilation finishing in seconds standalone. It is not a deadlock. A `nim ic` parent at 0% CPU is waiting on its children, and the test name does keep changing if you watch long enough. Writing this down because it has now been misdiagnosed as a testament/`nim ic` interaction more than once, by me, before I measured `vm_stat` and `vm.swapusage`. Also records the fan-out caps (`--parallelBuild:N`, `-d:icJobs:N`, `-d:icNoParallel`), that serial mode is what makes per-process diagnostic output readable rather than interleaved, that `testament r` cannot run a multi-step metamorphic file, and that `*_temp.nim` are gitignored scratch rather than tests. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XEF7FJvUkGKvG9LSGuEaNR |
||
|
Araq
|
c1a815fa07 |
IC: take the last leaf readers, and map where the leaf migration ends
`skipAddr`, `skipAddrDeref` and `isInactiveDestructorCall` move to the seam — all three pure, none of them touching a field symbol. The node-returning ones are graded by a `checkNodeResult` template rather than a hand-written pair, so adding the next one is a line. The more useful half of this commit is the map. Counting rather than guessing: of the 191 `PNode`-taking procs across `cgen` and the `ccg*` files, 160 EMIT — they take a `Builder`/`TLoc` out-param or write into `p` directly. Those do not move one at a time. They are one mutual recursion rooted at `expr`/`genStmts`, so the generator moves as a unit or not at all, and doing that needs write-side capability the seam does not have. What is left over is not a backlog, it is six named blockers, and `bnode`'s module doc now says which proc each one holds up and why: * a type's RECORD TREE is not a body (`PType.n` stays `PNode` by design, so `asgnComplexity` and friends were never candidates); * RETURNS A NODE OR NIL (`getPragmaStmt` — no nil token exists to return); * WRITES TO THE NODE (`easyResultAsgn` sets `nfPreventCg`; the seam is read-only); * NEEDS RENDERING (`preventNrvo` interpolates `$le` into a warning); * NEEDS STABLE FIELD IDENTITY (`lhsDoesAlias`, `potentialAlias`, through `isPartOf` — see the `sym` note, this one is blocked on a property rather than on effort); * MIXED REPRESENTATION (`potentialAlias`, `getPotentialReads` carry a `seq[PNode]` beside the node). Stating it this way because the alternative is someone re-deriving each blocker by trying the migration and watching it fail, which is how three of the six were found. Verified: grind clean (67_857 nodes, 0 disagreements); 215/215 byte-identical `.c` against HEAD on the default path; all four build configurations compile. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XEF7FJvUkGKvG9LSGuEaNR |
||
|
Araq
|
fe39ad5fba |
IC: sym is not a function of its argument for fields — find out, then say so
The goal was `aliases.isPartOf`, the deepest thing on the migration list: mutual recursion over two trees at once, reaching `sym`, `typ`, `intVal` and `isDeepConstExpr`. It does not migrate, and the reason is worth more than the migration would have been. `bnode.sym` IS NOT IDEMPOTENT for object fields. Two calls on the SAME token yield two different `skField` `PSym`s with consecutive item ids — field uses bypass the nav's memo and go to `loadFieldStub`, which mints per use because two distinct fields can share a name AND a position across types, so one shared stub would mistype one of them. `isPartOf` compares `a[1].sym.id != b[1].sym.id` to decide whether two accessor chains touch the same field, so on a cursor it answers `arNo` where the AST answers `arYes`: wrong alias analysis feeding NRVO and observable-store decisions. The grinder caught it on the first run after the migration, on a self-comparison `isPartOf(n, n)` — a shape production never passes, which is exactly why it was worth grading. The property was then confirmed directly rather than inferred: call `sym` twice on one token and print both, and the ids differ. So `aliases.nim` is reverted, and what stays is the knowledge: * `bnode.sym` says which symbols it is stable for and which it is not, what codegen actually consumes for a field instead (the name it re-navigates the reclist with, plus the position for tuples — the same tolerance the grinder applies), and why this module cannot fix it alone: a stable field identity needs the token's own position as a key, and `nifcore.Cursor` keeps that pointer private. * The grinder asserts idempotence for every NON-field sym at every node. It passes, and admitting fields makes it fail immediately — so the check states a precise boundary rather than a vague warning, and the day fields join it will be visible. `getInt` moves into a template so `bnode` can instantiate it (the `canRaiseImpl` pattern), and `astalgo.sameValue` becomes `AnyNode` and is graded — both are literal-only and unaffected by the field problem. `preventNrvo` records the OTHER kind of blocker while it is fresh: its alias analysis is fine, but the `warnObservableStores` message renders the node, and rendering is a capability the seam does not have at all. Verified: grind clean (67_857 nodes, 0 disagreements); 215/215 byte-identical `.c` against HEAD on the default path; all four build configurations compile. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XEF7FJvUkGKvG9LSGuEaNR |
||
|
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
|
||
|
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 |
||
|
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
|
||
|
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 |
||
|
ringabout
|
dcec8e1cd1 |
fixes #26134; del(seq) performs self-assignment and =destroy for del(… (#26138)
…0) of 1-length seq fixes #26134 |
||
|
Ryan McConnell
|
8cb406cd7a |
Fix 26144; exception propagation for non-raising virtual methods (#26145)
ref #26144 The C backend must not use `sfNeverRaises` to remove exception checks from virtual method calls. The flag describes only the selected base method body, while a vtable override may raise a catchable exception. This change makes `canRaiseDisp` conservative for `skMethod` symbols and adds a regression test covering an exception raised by a child method invoked through a base reference. |
||
|
ringabout
|
802bcf5a2d |
fixes #26132; =destroy should accept non-parametrized generic (#26142)
fixes #26132 |
||
|
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 |
||
|
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
|
||
|
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
|
||
|
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
|
||
|
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>
|
||
|
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> |
||
|
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> |
||
|
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>
|
||
|
Constantine Molchanov
|
f897fe8c29 |
Support :code: argument in .. include:: directive. (#26146)
This is part of the reST spec, useful for code snippet inclusion: https://docutils.sourceforge.io/docs/ref/rst/directives.html#include |
||
|
Ryan McConnell
|
33ee586913 |
fixes #11797; fix C type hashes for imported aliases (#26150)
Fixes #11797. Imported scalar and pointer aliases inherit their external C spelling, but receive a different Nim symbol. Signature hashing previously used that symbol identity, so aliases that emit exactly the same C type could produce different backend names for tuples, sequences, and other generic types. For example, `cint` and `type CIntAlias = cint` both emit `int`, but `seq[cint]` and `seq[CIntAlias]` could be emitted as incompatible C structs. The Nim type checker nevertheless permits assignments and calls between them, causing the generated C or C++ compilation to fail. This changes the backend hash to use the external type spelling when available. A symbol-based fallback remains for imported types without a resolved spelling. The change deliberately does not collapse imported types into their underlying Nim builtin. Types such as `pid_t`, imported pointers with qualifiers, and platform typedefs may require distinct backend representations. ## NIF and incremental compilation This does not change NIF serialization, NIF type keys, or the IC cache format. The bug is in backend type-name generation. An IC regression test is included to ensure that the corrected backend identity is preserved when compilation passes through the NIF pipeline. ## Tests The regressions cover: - tuple and sequence assignments between an imported type and its alias - cross-module sequence parameters and mutation - C and C++ backends - NIF-backed incremental compilation Existing C-type tests were also run under C/C++, refc, and ARC. ## Remaining scope This does not solve the broader question of compatibility between imported and builtin types that have different backend identities, such as `seq[cdouble]` and `seq[float]`. That remains tracked by #19374. |
||
|
Ryan McConnell
|
0be9b4f3f6 |
fix 26147; new-style concepts: broken generic (Case B) (#26151)
ref #26147 |
||
|
ringabout
|
c36c527db3 |
fixes #26143; Possible memory error (#26154)
fixes #26143 follows up https://github.com/nim-lang/Nim/pull/20307 |
||
|
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> |
||
|
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
|
||
|
araq
|
0c0cc1e496 | remove owner-checking-tower | ||
|
araq
|
1387093f99 | don't redo nifler steps unnecessarily | ||
|
Andreas Rumpf
|
546a518b22 | Merge branch 'devel' into araq-ic-fixes | ||
|
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. |
||
|
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.
|
||
|
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> |
||
|
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>
|
||
|
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>
|
||
|
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> |
||
|
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> |
||
|
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> |
||
|
Andreas Rumpf
|
8ca7b75b8b | refactoring: better IC + no unique Id (#26137) | ||
|
bptato
|
7f120229e8 |
Limit use of long checked integer ops to arm-none-eabi (#26121)
#23835 tried to do this, but it also switched to `long int` on the GNU ABI, where it's actually just `int`. Fixes #26111 Co-authored-by: Andreas Rumpf <rumpf_a@web.de> |