From dc242e90277ab106cdc4d63b5ea927d4c1590521 Mon Sep 17 00:00:00 2001 From: ringabout <43030857+ringabout@users.noreply.github.com> Date: Wed, 26 Aug 2026 14:40:57 +0800 Subject: [PATCH 1/3] 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 --- compiler/semgnrc.nim | 8 ++++++-- tests/generics/t26124.nim | 7 +++++++ 2 files changed, 13 insertions(+), 2 deletions(-) create mode 100644 tests/generics/t26124.nim diff --git a/compiler/semgnrc.nim b/compiler/semgnrc.nim index da9b1c187c..e8869717a3 100644 --- a/compiler/semgnrc.nim +++ b/compiler/semgnrc.nim @@ -129,7 +129,12 @@ proc semGenericStmtSymbol(c: PContext, n: PNode, s: PSym, result.typ = nil onUse(n.info, s) of skParam: - if s.owner == c.p.owner: + if s.typ != nil and s.typ.kind == tyStatic and s.typ.n != nil: + # The enclosing routine gives this static parameter a concrete value. + # Keep that value so the nested generic can fold it as a compile-time + # expression instead of generating a runtime parameter reference. + result = s.typ.n + elif s.owner == c.p.owner: # Parameters of the routine currently being semchecked stay as local # identifiers result = n @@ -681,4 +686,3 @@ proc semConceptBody(c: PContext, n: PNode): PNode = ) result = semGenericStmt(c, n, {withinConcept}, ctx) semIdeForTemplateOrGeneric(c, result, ctx.cursorInBody) - diff --git a/tests/generics/t26124.nim b/tests/generics/t26124.nim new file mode 100644 index 0000000000..5c18f6686b --- /dev/null +++ b/tests/generics/t26124.nim @@ -0,0 +1,7 @@ +proc u(k: static int) = + proc r(_: static int) = + while k > 0: + discard + r(0) + +u(0) From bd95f88f7412b80b15bc0d667f39169e6f8fc05f Mon Sep 17 00:00:00 2001 From: Zoom Date: Wed, 26 Aug 2026 20:01:06 +0400 Subject: [PATCH 2/3] 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. --- changelog.md | 7 +++ compiler/jsgen.nim | 104 +++++++++++++++++++++++++++++++-- tests/openarray/topenarray.nim | 21 ++++++- 3 files changed, 126 insertions(+), 6 deletions(-) diff --git a/changelog.md b/changelog.md index d6561b42d2..c38267aa3e 100644 --- a/changelog.md +++ b/changelog.md @@ -150,6 +150,13 @@ parameter and result types, not just their source-level shape. Use The issue was that `hasValuelessStatics` in `semtypinst.nim` didn't recognize `tyTypeDesc(tyGenericParam)` as an unresolved generic parameter. +- The JS backend now implements write-through for `var openArray` parameters that + receive a `toOpenArray` view (bug #15952): mutations reach the caller's storage + instead of silently writing to a copy. Fixed homogeneous numeric arrays + (`array[N, T]`, JS typed arrays) slice via `subarray`; `seq` and non-numeric + arrays slice via a `{base, off, len}` view. This also covers seq/non-numeric-array + write-through, pass-through, re-slicing and `@` (openArray-to-seq) of such views. + ## Tool changes - Added `--raw` flag when generating JSON docs to not render markup. diff --git a/compiler/jsgen.nim b/compiler/jsgen.nim index 963c147fac..5decd63ef6 100644 --- a/compiler/jsgen.nim +++ b/compiler/jsgen.nim @@ -1450,6 +1450,20 @@ proc genCheckedFieldOp(p: PProc, n: PNode, addrTyp: PType, r: var TCompRes) = r.res = "$1.$2" % [tmp, field.loc.snippet] r.kind = resExpr +proc isVarOpenArrayParam(n: PNode): bool = + ## True if `n` resolves to a `var openArray` parameter. The JS backend + ## represents such parameters as a `{base, off, len}` slice view so that + ## writes through a `toOpenArray` view reach the caller's storage (bug #15952). + var it = n + while true: + case it.kind + of nkHiddenDeref, nkDerefExpr, nkHiddenAddr, nkAddr: it = it[0] + of nkHiddenStdConv, nkConv, nkObjDownConv, nkObjUpConv: it = it[1] + else: break + result = it.kind == nkSym and it.sym.kind == skParam and + it.sym.typ != nil and it.sym.typ.kind == tyVar and + it.sym.typ.len > 0 and it.sym.typ[0].kind == tyOpenArray + proc genArrayAddr(p: PProc, n: PNode, r: var TCompRes) = var a, b: TCompRes = default(TCompRes) @@ -1458,6 +1472,19 @@ proc genArrayAddr(p: PProc, n: PNode, r: var TCompRes) = let m = if n.kind == nkHiddenAddr: n[0] else: n gen(p, m[0], a) gen(p, m[1], b) + if isVarOpenArrayParam(m[0]): + # `var openArray` param is a `{base, off, len}` view; index the base with + # the offset applied. `m[0]` is a plain param name, safe to reference + # repeatedly (no side effects, so no temp needed). + let pn = a.rdLoc + r.address = "($1).base" % [pn] + if optBoundsCheck in p.options: + useMagic(p, "chckIndx") + r.res = "($1).off + chckIndx($2, 0, ($1).len - 1)" % [pn, b.rdLoc] + else: + r.res = "($1).off + ($2)" % [pn, b.rdLoc] + r.kind = resExpr + return #internalAssert p.config, a.typ != etyBaseIndex and b.typ != etyBaseIndex let (x, tmp) = maybeMakeTemp(p, m[0], a) r.address = x @@ -1726,8 +1753,47 @@ proc genArgNoParam(p: PProc, n: PNode, r: var TCompRes) = else: r.res.add(a.res) +proc genVarOpenArrayArg(p: PProc, n: PNode, r: var TCompRes) = + ## Emit a `{base, off, len}` slice view for an argument to a `var openArray` + ## parameter (bug #15952). The view always aliases the base storage, so writes + ## through the callee's `openArray` reach the caller's array/seq/typed array. + var b, lo, hi, v: TCompRes = default(TCompRes) + # the argument reaches codegen as `addr(toOpenArray(x, lo, hi))` (possibly + # under conversions); unwrap to the actual `toOpenArray` call. + var sl = n + while true: + case sl.kind + of nkHiddenAddr, nkAddr, nkHiddenDeref, nkDerefExpr: sl = sl[0] + of nkHiddenStdConv, nkConv, nkObjDownConv, nkObjUpConv: sl = sl[1] + else: break + if sl.kind in nkCallKinds and getMagic(sl) == mSlice: + gen(p, sl[1], b) + gen(p, sl[2], lo) + gen(p, sl[3], hi) + if isVarOpenArrayParam(sl[1]): + # slicing a `var openArray` view: rebase onto the same underlying storage + r.res = "{base: ($1).base, off: ($1).off + $2, len: $3 - $2 + 1}" % [ + b.rdLoc, lo.rdLoc, hi.rdLoc] + else: + r.res = "{base: $1, off: $2, len: $3 - $2 + 1}" % [ + b.rdLoc, lo.rdLoc, hi.rdLoc] + elif isVarOpenArrayParam(sl): + # already a view from another `var openArray` param: forward it unchanged + gen(p, sl, b) + r.res = b.rdLoc + else: + # a whole array/seq/typed-array value: wrap with a zero offset + gen(p, n, v) + r.res = "{base: $1, off: 0, len: ($1).length}" % [v.rdLoc] + r.kind = resExpr + proc genArg(p: PProc, n: PNode, param: PSym, r: var TCompRes; emitted: ptr int = nil) = var a: TCompRes = default(TCompRes) + if param.typ != nil and param.typ.kind == tyVar and param.typ[0].kind == tyOpenArray: + # `var openArray` params are passed as a `{base, off, len}` slice view. + genVarOpenArrayArg(p, n, a) + r.res.add(a.rdLoc) + return gen(p, n, a) if skipTypes(param.typ, abstractVar).kind in {tyOpenArray, tyVarargs} and a.typ == etyBaseIndex: @@ -1737,6 +1803,13 @@ proc genArg(p: PProc, n: PNode, param: PSym, r: var TCompRes; emitted: ptr int = r.res.add(", ") r.res.add(a.res) if emitted != nil: inc emitted[] + elif skipTypes(param.typ, abstractVar).kind == tyOpenArray and + isVarOpenArrayParam(n): + # a `var openArray` view passed to a read-only `openArray` param: materialize + # a snapshot so the callee sees a plain array. + var w: TCompRes = default(TCompRes) + gen(p, n, w) + r.res.add("(($1).base).slice(($1).off, ($1).off + ($1).len)" % [w.rdLoc]) elif n.typ.kind in {tyVar, tyPtr, tyRef, tyLent, tyOwned} and n.kind in nkCallKinds and mapType(param.typ) == etyBaseIndex: # this fixes bug #5608: @@ -2371,13 +2444,21 @@ proc genMagic(p: PProc, n: PNode, r: var TCompRes) = useMagic(p, "nimCopy") r.res = "nimCopy(null, $1, $2)" % [x.rdLoc, genTypeInfo(p, n.typ)] of mOpenArrayToSeq: - genCall(p, n, r) + if isVarOpenArrayParam(n[1]): + var x: TCompRes = default(TCompRes) + gen(p, n[1], x) + r.res = "(($1).base).slice(($1).off, ($1).off + ($1).len)" % [x.rdLoc] + r.kind = resExpr + else: + genCall(p, n, r) of mDestroy, mTrace: discard "ignore calls to the default destructor" of mOrd: genOrd(p, n, r) of mLengthStr, mLengthSeq, mLengthOpenArray, mLengthArray: var x: TCompRes = default(TCompRes) gen(p, n[1], x) - if skipTypes(n[1].typ, abstractInst).kind == tyCstring: + if isVarOpenArrayParam(n[1]): + r.res = "($1).len" % [x.rdLoc] + elif skipTypes(n[1].typ, abstractInst).kind == tyCstring: let (a, tmp) = maybeMakeTemp(p, n[1], x) r.res = "(($1) == null ? 0 : ($2).length)" % [a, tmp] else: @@ -2386,7 +2467,9 @@ proc genMagic(p: PProc, n: PNode, r: var TCompRes) = of mHigh: var x: TCompRes = default(TCompRes) gen(p, n[1], x) - if skipTypes(n[1].typ, abstractInst).kind == tyCstring: + if isVarOpenArrayParam(n[1]): + r.res = "($1).len - 1" % [x.rdLoc] + elif skipTypes(n[1].typ, abstractInst).kind == tyCstring: let (a, tmp) = maybeMakeTemp(p, n[1], x) r.res = "(($1) == null ? -1 : ($2).length - 1)" % [a, tmp] else: @@ -2469,11 +2552,24 @@ proc genMagic(p: PProc, n: PNode, r: var TCompRes) = genCall(p, n, r) of mSlice: # arr.slice([begin[, end]]): 'end' is exclusive + # Fixed homogeneous numeric arrays lower to JS typed arrays; `slice` + # copies, which silently breaks `var openArray` write-through (bug #15952). + # `subarray` returns a live shared-buffer view with the same + # exclusive-end signature, so use it there; keep `slice` for seqs/strings. var x, y, z: TCompRes = default(TCompRes) gen(p, n[1], x) gen(p, n[2], y) gen(p, n[3], z) - r.res = "($1.slice($2, $3 + 1))" % [x.rdLoc, y.rdLoc, z.rdLoc] + if isVarOpenArrayParam(n[1]): + # re-slicing a `var openArray` view: materialize from the view's base/offset + r.res = "(($1).base).slice(($1).off + $2, ($1).off + $3 + 1)" % [ + x.rdLoc, y.rdLoc, z.rdLoc] + else: + let baseTy = skipTypes(n[1].typ, abstractVarRange + {tyLent}) + if baseTy.kind == tyArray and arrayTypeForElemType(p.config, elemType(baseTy)).len > 0: + r.res = "($1.subarray($2, $3 + 1))" % [x.rdLoc, y.rdLoc, z.rdLoc] + else: + r.res = "($1.slice($2, $3 + 1))" % [x.rdLoc, y.rdLoc, z.rdLoc] r.kind = resExpr of mMove: genMove(p, n, r) diff --git a/tests/openarray/topenarray.nim b/tests/openarray/topenarray.nim index 25b983651a..01c063d3fb 100644 --- a/tests/openarray/topenarray.nim +++ b/tests/openarray/topenarray.nim @@ -11,6 +11,9 @@ proc fn2[T](a: var openArray[T]): seq[T] = proc fn3[T](a: var openArray[T]) = for i, ai in mpairs(a): ai = i * 10 +proc wr[T](a: var openArray[T]; v: T) = + a[0] = v + proc main = var a = [1,2,3,4,5] @@ -20,8 +23,22 @@ proc main = doAssert fn2(a.toOpenArray(1,3)) == @[2,3,4] fn3(a.toOpenArray(1,3)) - when defined(js): discard # xxx bug #15952: `a` left unchanged - else: doAssert a == [1, 0, 10, 20, 5] + doAssert a == [1, 0, 10, 20, 5] + + block: # bug #15952: `toOpenArray` slices are live views on JS + # Fixed homogeneous numeric arrays lower to JS typed arrays; seqs and + # non-numeric fixed arrays lower to plain JS arrays. In all cases a slice + # passed to a `var openArray` must alias the source so writes propagate + # (JS: subarray view for typed arrays, {base,off,len} view otherwise). + var si = @[1, 2, 3, 4, 5] + fn3(si.toOpenArray(1, 3)) + doAssert si == @[1, 0, 10, 20, 5] + var ss = ["a", "b", "c", "d", "e"] + wr(ss.toOpenArray(1, 3), "Z") + doAssert ss == ["a", "Z", "c", "d", "e"] + # read-only slicing must still work and never throw, on every backend. + doAssert fn1(@[1, 2, 3, 4, 5].toOpenArray(1, 3)) == @[2, 3, 4] + doAssert fn1(["a", "b", "c", "d", "e"].toOpenArray(1, 3)) == @["b", "c", "d"] block: # bug #12521 block: From c87926dadf4b2b3a9f274b607a61d2532b9e05b9 Mon Sep 17 00:00:00 2001 From: Andreas Rumpf Date: Thu, 27 Aug 2026 19:35:11 +0200 Subject: [PATCH 3/3] IC: more bugfixes (#26141) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- compiler/ast2nif.nim | 129 +++++++++- compiler/astdef.nim | 2 +- compiler/ccgtypes.nim | 35 ++- compiler/cgen.nim | 83 ++++++- compiler/cgendata.nim | 4 + compiler/cnif.nim | 30 ++- compiler/commands.nim | 15 +- compiler/deps.nim | 377 +++++++++++++++++++++++++++--- compiler/icconfig.nim | 17 +- compiler/injectdestructors.nim | 5 + compiler/main.nim | 24 +- compiler/modulegraphs.nim | 92 +++++++- compiler/nifbackend.nim | 34 ++- compiler/nim.nim | 2 +- compiler/options.nim | 20 +- compiler/pipelines.nim | 19 +- compiler/semmagic.nim | 9 + compiler/semstmts.nim | 2 + doc/ic.md | 127 +++++++++- koch.nim | 6 +- testament/categories.nim | 106 ++++++++- testament/testament.nim | 156 ++++++++++++- tests/ic/tconfig_invalidation.nim | 34 +++ tests/ic/tdead_when_import.nim | 37 +++ tests/ic/tdeleted_module.nim | 26 +++ tests/ic/tdestructor_fidelity.nim | 68 ++++++ tests/ic/tdiscovered_import.nim | 38 +++ tests/ic/terror_recovery.nim | 32 +++ tests/ic/tglobal_dtors.nim | 49 ++++ 29 files changed, 1484 insertions(+), 94 deletions(-) create mode 100644 tests/ic/tconfig_invalidation.nim create mode 100644 tests/ic/tdead_when_import.nim create mode 100644 tests/ic/tdeleted_module.nim create mode 100644 tests/ic/tdestructor_fidelity.nim create mode 100644 tests/ic/tdiscovered_import.nim create mode 100644 tests/ic/terror_recovery.nim create mode 100644 tests/ic/tglobal_dtors.nim diff --git a/compiler/ast2nif.nim b/compiler/ast2nif.nim index 135a14a20b..f6d2094090 100644 --- a/compiler/ast2nif.nim +++ b/compiler/ast2nif.nim @@ -269,6 +269,16 @@ const ## in a Nim identifier), so a field use can never be misrouted to a same-named ## local var/param. Mirrors the `` `t `` (`typeToNifSym`) and `PkgMarker` ## namespaces. + CursorFieldMarker = "`fc" + ## `FieldMarker` for a field declared `{.cursor.}`. A field USE serializes as + ## a bare `SymUse` — there is nowhere to put symbol flags — and the use-site + ## stub `loadFieldStub` mints carries none, so `trees.isCursor` (which reads + ## `sfCursor` off the field sym of an `nkDotExpr`) said "not a cursor" for + ## every loaded field. `lists.DoublyLinkedNode.prev` then became a COUNTED + ## reference: every node held its predecessor alive, no refcount ever hit + ## zero, and a doubly linked list leaked its whole contents. Both the reclist + ## def and every use derive their name from the same `PSym`, so marking the + ## name keeps them in lockstep. PkgMarker = "`pkg" ## Appended to the ident of `skPackage` symbols in NIF names. A package sym ## has no module of its own: it is written once into every module NIF that @@ -290,7 +300,7 @@ proc toNifSymName(w: var Writer; sym: PSym): string = # agree by construction; the loader recovers `name.s` and `mangleField` produces # the matching struct member name regardless of which module references it. result = sym.name.s - result.add FieldMarker + result.add (if sfCursor in sym.flagsImpl: CursorFieldMarker else: FieldMarker) result.add '.' # Use the field's POSITION as the local name's numeric component: it is unique # within the owning type (so the local name is unambiguous there) AND it is what @@ -382,11 +392,20 @@ proc parseSymName*(s: string): ParsedSymName = dec i return ParsedSymName(name: s, module: "") +proc isFieldMarked(rawName: string): bool {.inline.} = + rawName.endsWith(FieldMarker) or rawName.endsWith(CursorFieldMarker) + +proc stripFieldMarker(rawName: string): string {.inline.} = + if rawName.endsWith(CursorFieldMarker): + rawName[0 ..< rawName.len - CursorFieldMarker.len] + else: + rawName[0 ..< rawName.len - FieldMarker.len] + proc isFieldNifName(name: string): bool {.inline.} = ## True for an object field's local NIF name ``f.` (see ## `FieldMarker`): no module suffix, marker on the ident. let sn = parseSymName(name) - sn.module.len == 0 and sn.name.endsWith(FieldMarker) + sn.module.len == 0 and isFieldMarked(sn.name) proc stubKindAndName(cache: IdentCache; rawName: string): (TSymKind, PIdent) = ## The user-visible name of a symbol stub must NOT keep NIF-only name @@ -397,11 +416,11 @@ proc stubKindAndName(cache: IdentCache; rawName: string): (TSymKind, PIdent) = ## the marked NIF name for the index lookup. if rawName.endsWith(PkgMarker): (skPackage, cache.getIdent(rawName[0 ..< rawName.len - PkgMarker.len])) - elif rawName.endsWith(FieldMarker): + elif isFieldMarked(rawName): # Object field (local NIF symbol, see `FieldMarker`): strip the marker so the # backend mangles the clean field name, and record the kind so a use-site stub # is a real `skField` (cgen branches on it for `obj.field` access). - (skField, cache.getIdent(rawName[0 ..< rawName.len - FieldMarker.len])) + (skField, cache.getIdent(stripFieldMarker(rawName))) else: (skStub, cache.getIdent(rawName)) @@ -1305,6 +1324,7 @@ var repDeepCopyTag = registerTag("repdeepcopy") var repEnumToStrTag = registerTag("repenumtostr") var repMethodTag = registerTag("repmethod") var repPureEnumTag = registerTag("reppureenum") +var repCppMemberTag = registerTag("repcppmember") #var repClassTag = registerTag("repclass") var includeTag = registerTag("include") var importTag = registerTag("import") @@ -1330,6 +1350,29 @@ var sigTag = registerTag("sig") # by construction (replaces relying on the `@bk` module-marker bit, which the # loader dropped on type USES). Mirrors NIF's `.unusedname` directive. var unusedIdTag = registerTag("unusedid") +# `(modflags )` — the MODULE symbol's backend-relevant flags. Only +# `sfInjectDestructors` (bit 0) so far: sempass2 sets it on the module sym when +# the module's TOP-LEVEL statements need the destructor pass, and `cgen. +# genTopLevelStmt` gates `injectDestructorCalls` on it. `moduleFromNifFile` +# builds the module PSym from scratch, so without this record the flag was lost +# and a NIF-loaded module's top-level locals were never destroyed (`block: let +# h = openHandle()` leaked, silently and only under `nim ic`). +const ModFlagInjectDestructors* = 1'i32 +var modFlagsTag = registerTag("modflags") + +# `(nflags )` — an `nkSym` NODE's own flags. A sym node is +# normally emitted as a bare NIF `SymUse` token, which has nowhere to put them, +# so every node flag on a sym use was silently dropped. Two of those flags are +# the frontend's move/first-write analysis results (`nfFirstWrite`, `nfLastRead`, +# both listed in `PersistentNodeFlags`) that `injectdestructors` reads in the +# backend: without them EVERY first assignment to a destructor-bearing local +# compiled as `=sink` (i.e. `=destroy` on still-zeroed memory, then a copy) +# instead of a plain construction, and no read was ever recognised as a move. +# Only wrap when there is something to say, so the common sym use stays a bare +# token. +const symNodeFlagsTagName = "nflags" +var symNodeFlagsTag = registerTag(symNodeFlagsTagName) +const PersistedSymNodeFlags = PersistentNodeFlags - {nfLazyType, nfHasComment} proc registerNifAstTags*() = ## (Re)registers ast2nif's NIF tags explicitly. The top-level `registerTag` @@ -1345,6 +1388,8 @@ proc registerNifAstTags*() = tdefTag = registerTag(typeDefTagName) hiddenTypeTag = registerTag(hiddenTypeTagName) bindingIdTag = registerTag(bindingIdTagName) + modFlagsTag = registerTag("modflags") + symNodeFlagsTag = registerTag(symNodeFlagsTagName) replayTag = registerTag("replay") repConverterTag = registerTag("repconverter") repDestroyTag = registerTag("repdestroy") @@ -1357,6 +1402,7 @@ proc registerNifAstTags*() = repEnumToStrTag = registerTag("repenumtostr") repMethodTag = registerTag("repmethod") repPureEnumTag = registerTag("reppureenum") + repCppMemberTag = registerTag("repcppmember") includeTag = registerTag("include") importTag = registerTag("import") implTag = registerTag("implementation") @@ -1437,7 +1483,14 @@ proc writeNode(w: var Writer; dest: var IcBuilder; n: PNode; forAst = false) = w.withNode dest, n: dest.addIdent n.ident.s of nkSym: - writeSymNode(w, dest, n, n.sym) + let persisted = n.flags * PersistedSymNodeFlags + if persisted == {}: + writeSymNode(w, dest, n, n.sym) + else: + dest.addParLe symNodeFlagsTag, trLineInfo(w, n.info) + writeFlags(dest, persisted) + writeSymNode(w, dest, n, n.sym) + dest.addParRi of nkCharLit: w.withNode dest, n: dest.add charToken(n.intVal.char, NoLineInfo) @@ -1668,6 +1721,11 @@ proc writeOp(w: var Writer; content: var IcBuilder; op: LogEntry) = content.add strToken(pool.strings.getOrIncl(op.key), NoLineInfo) content.add symToken(pool.syms.getOrIncl(w.toNifSymName(op.sym)), NoLineInfo) content.addParRi() + of CppMemberEntry: + content.addParLe repCppMemberTag, NoLineInfo + content.add strToken(pool.strings.getOrIncl(op.key), NoLineInfo) + content.add symToken(pool.syms.getOrIncl(w.toNifSymName(op.sym)), NoLineInfo) + content.addParRi() of GenericInstEntry: discard "will only be written later to ensure it is materialized" @@ -2085,7 +2143,8 @@ proc writeNifModule*(config: ConfigRef; thisModule: int32; n: PNode; typeOffers: seq[tuple[generic: PSym; inst: PType]] = @[]; resolvedImportDeps: seq[FileIndex] = @[]; firstUnusedId: int32 = 0; - expansions: seq[(PSym, TLineInfo)] = @[]) = + expansions: seq[(PSym, TLineInfo)] = @[]; + moduleFlags: int32 = 0) = var w = Writer(infos: newLineInfoWriter(config), currentModule: thisModule) w.deps = newIcBuilder(64) var content = newIcBuilder(300) @@ -2239,6 +2298,10 @@ proc writeNifModule*(config: ConfigRef; thisModule: int32; n: PNode; dest.addParLe unusedIdTag, NoLineInfo dest.addIntLit firstUnusedId.int64 dest.addParRi() + # The module symbol's backend-relevant flags (see `(modflags)`). + dest.addParLe modFlagsTag, NoLineInfo + dest.addIntLit moduleFlags.int64 + dest.addParRi() addAll(dest, w.deps) # do not write the (stmts .. ) wrapper: addStmtsBody(dest, content) @@ -2816,6 +2879,9 @@ proc loadFieldStub(c: var DecodeContext; symAsStr: string; thisModule: string; result = PSym(itemId: c.nextSymId(module, isBk = false), kindImpl: stubKind, name: stubName, disamb: sn.count.int32, state: Complete) result.positionImpl = sn.count.int32 + # `{.cursor.}` rides in the marker (see `CursorFieldMarker`) because the move + # optimizer reads it straight off the use site (`trees.isCursor`). + if sn.name.endsWith(CursorFieldMarker): result.flagsImpl.incl sfCursor if typ != nil: result.typImpl = typ proc loadSymStub(c: var DecodeContext; symAsStr: string; thisModule: string; @@ -2828,7 +2894,7 @@ proc loadSymStub(c: var DecodeContext; symAsStr: string; thisModule: string; result = localSyms.getOrDefault(symAsStr) if result != nil: return result - elif sn.name.endsWith(FieldMarker): + elif isFieldMarked(sn.name): # A cross-context object-field reference reaching a non-dotExpr slot (e.g. a # `{.guard.}` field, an owner): stub it like any other field use. return c.loadFieldStub(symAsStr, thisModule, localSyms) @@ -3259,6 +3325,13 @@ proc loadNode(c: var DecodeContext; n: var Cursor; thisModule: string; loadSymFromCursor(c, sym, n, thisModule, localSyms) result = newSymNode(sym, info) result.flags.incl nfLazyType + elif tagIs(n, symNodeFlagsTagName): + # `(nflags )`: node flags for the wrapped sym use. + n.into: + let flags = loadAtom(TNodeFlags, n) + result = loadNode(c, n, thisModule, localSyms) + if result != nil: result.flags = result.flags + flags + while n.hasMore: skip n elif tagIs(n, typeDefTagName): raiseAssert "`td` tag in invalid context" elif tagIs(n, "none"): @@ -3458,13 +3531,23 @@ proc moduleSymbolStubs*(c: var DecodeContext; module: FileIndex): seq[PSym] = ## symbol can register new modules and invalidate the iterator), so the caller ## forces full load (`.kind`, `.ast`) and filters AFTER this returns, with the ## index back in place. + ## + ## Ordered by the entry's OFFSET, i.e. the order the writer emitted them, which + ## is source order. A `Table` iteration is hash order — arbitrary, and not even + ## stable between two compilers — so the `lower` stage transformed a module's + ## routines in a random order. That is visible (`--expandArc` diagnostics came + ## out shuffled) and it makes the backend's minted ids depend on the hash seed. result = @[] if not c.mods.hasKey(module): return var indexTab = move c.mods[module].index let thisModule = c.mods[module].suffix + var entries: seq[(int, string)] = @[] for nifName, entry in indexTab: if nifName.startsWith("`t"): continue # types are not routines - let sym = loadSymFromIndexEntry(c, module, nifName, entry, thisModule) + entries.add (entry.offset, nifName) + sort entries + for (_, nifName) in entries: + let sym = loadSymFromIndexEntry(c, module, nifName, indexTab[nifName], thisModule) if sym != nil: result.add sym c.mods[module].index = move indexTab @@ -3592,6 +3675,8 @@ type ## `typeInstCache` from them so a consumer reuses the baked instance ## (e.g. a `mixin`/`compiles()`-dependent array bound) instead of ## re-instantiating it with a different bound in its own scope. + moduleFlags*: int32 ## the module SYMBOL's backend-relevant flags; see + ## `(modflags)` / `ModFlagInjectDestructors`. includes*: seq[string] # resolved full paths of files this module `include`s; # replayed into `inclToMod` by modulegraphs.nim so that # nimsuggest can map a query in an include file back to @@ -3737,6 +3822,12 @@ proc processTopLevel(c: var DecodeContext; cur: var Cursor; flags: set[LoadFlag] # backend id seed — consumed eagerly by `moduleId`/`readUnusedId`; just # skip past it here so the rest of the header still loads. skip cur + elif tagIs(cur, "modflags"): + cur.into: + if cur.hasMore and cur.kind == IntLit: + result.moduleFlags = int32 intVal(cur) + skip cur + while cur.hasMore: skip cur elif tagIs(cur, "repconverter"): loadLogOp(c, result.logOps, cur, ConverterEntry, attachedTrace, module) elif tagIs(cur, "repdestroy"): loadLogOp(c, result.logOps, cur, HookEntry, attachedDestructor, module) elif tagIs(cur, "repwasmoved"): loadLogOp(c, result.logOps, cur, HookEntry, attachedWasMoved, module) @@ -3748,6 +3839,7 @@ proc processTopLevel(c: var DecodeContext; cur: var Cursor; flags: set[LoadFlag] elif tagIs(cur, "repenumtostr"): loadLogOp(c, result.logOps, cur, EnumToStrEntry, attachedTrace, module) elif tagIs(cur, "repmethod"): loadLogOp(c, result.logOps, cur, MethodEntry, attachedTrace, module) elif tagIs(cur, "reppureenum"): loadLogOp(c, result.logOps, cur, PureEnumEntry, attachedTrace, module) + elif tagIs(cur, "repcppmember"): loadLogOp(c, result.logOps, cur, CppMemberEntry, attachedTrace, module) elif tagIs(cur, "export"): cur.into: while cur.hasMore and cur.kind == DotToken: skip cur # flags / type @@ -3851,6 +3943,23 @@ proc processTopLevel(c: var DecodeContext; cur: var Cursor; flags: set[LoadFlag] else: cont = false +proc registerModuleSelfSym*(c: var DecodeContext; suffix: string; m: PSym) = + ## Bind the module's NIF name to the ONE module symbol the graph registered. + ## + ## A module's own symbol is the owner of every top-level symbol, so the writer + ## emits it as a real `(sd)` with an index entry (`mymod.0.`). Without + ## this binding the loader mints a SECOND `skModule` PSym for it the first time + ## some symbol's owner slot is resolved — and `sym.owner == owner` is an + ## IDENTITY test in `aliasanalysis.isAnalysableFieldAccess`, so every + ## module-level location looked un-analysable to the move optimizer: a + ## top-level `let (a, b) = f()` copied instead of moved, which is a hard error + ## for a type with a disabled `=copy`. + ## + ## Only the backend (`nim nifc`) does this — see the call site. + let key = m.name.s & ".0." & suffix + if not c.syms.hasKey(key): + c.syms[key] = (m, NifIndexEntry()) + proc loadNifModule*(c: var DecodeContext; suffix: ModuleSuffix; interf, interfHidden: var TStrTable; flags: set[LoadFlag] = {}): PrecompiledModule = # Ensure module index is loaded - moduleId returns the FileIndex for this suffix @@ -4010,6 +4119,10 @@ proc writeLoweredModule*(c: var DecodeContext; config: ConfigRef; dest.addParLe unusedIdTag, NoLineInfo dest.addIntLit loweredSeed.int64 dest.addParRi() + # Carry the module flags forward: `cg` loads THIS `.t.bif`, not the `.s.bif`. + dest.addParLe modFlagsTag, NoLineInfo + dest.addIntLit precomp.moduleFlags.int64 + dest.addParRi() addAll(dest, w.deps) addStmtsBody(dest, content) dest.addParRi() diff --git a/compiler/astdef.nim b/compiler/astdef.nim index 11f9a4474d..42ccf4c7b0 100644 --- a/compiler/astdef.nim +++ b/compiler/astdef.nim @@ -1049,7 +1049,7 @@ proc newStrNode*(strVal: string; info: TLineInfo): PNode = type LogEntryKind* = enum HookEntry, ConverterEntry, MethodEntry, EnumToStrEntry, GenericInstEntry, - PureEnumEntry + PureEnumEntry, CppMemberEntry LogEntry* = object kind*: LogEntryKind op*: TTypeAttachedOp diff --git a/compiler/ccgtypes.nim b/compiler/ccgtypes.nim index a7416e0815..08cab8c13b 100644 --- a/compiler/ccgtypes.nim +++ b/compiler/ccgtypes.nim @@ -1289,6 +1289,14 @@ proc genMemberProcHeader(m: BModule; prc: PSym; result: var Builder; asPtr: bool name = typDesc if isFnConst: fnConst = " const" + if not isCtor: + # The call-site form (`x->salute(@)`), not the mangled Nim name. Set it on + # BOTH paths: whole-program cgen always emitted the out-of-class definition + # (the `else` branch) before any caller, but the per-module backend emits a + # foreign member proc's body in ITS OWN module, so the caller's TU only ever + # reaches the in-class declaration below — and called the member by the + # mangled name (`loo->salute_u0__vireouyks1()`, "struct Loo has no member"). + prc.locImpl.snippet = "$1$2(@)" % [memberOp, name] if isFwdDecl: if isStatic: result.add "static " @@ -1298,9 +1306,7 @@ proc genMemberProcHeader(m: BModule; prc: PSym; result: var Builder; asPtr: bool override = " override" superCall = "" else: - if not isCtor: - prc.locImpl.snippet = "$1$2(@)" % [memberOp, name] - elif superCall != "": + if isCtor and superCall != "": superCall = " : " & superCall name = "$1::$2" % [typDesc, name] @@ -1891,11 +1897,30 @@ proc genVTable(result: var Builder, seqs: seq[PSym]) = result.add(cCast(CPointer, seqs[i].loc.snippet)) proc genTypeInfoV2OldImpl(m: BModule; t, origType: PType, name: Rope; info: TLineInfo) = + ## The C++/HCR flavour: C++ has no designated initializers, so the RTTI record + ## is a bare variable that the module's `DatInit` fills field by field. cgsym(m, "TNimTypeV2") - m.s[cfsStrData].addDeclWithVisibility(Private): - m.s[cfsStrData].addVar(kind = Local, name = name, typ = "TNimTypeV2") if m.config.cmd == cmdNifC: + # Same emit-everywhere split as `genTypeInfoV2Impl`: every `cg` process that + # demands this type declares it `extern`, and the DEFINITION is a droppable + # `'d'` unit the merge stage gives a single owner. Without the split the bare + # `TNimTypeV2 x;` in each TU is a tentative definition — which C's linker + # merges but C++'s does not, so `nim cpp --ic:on` died at link with + # "multiple definition of NTIv2__…". The field ASSIGNMENTS stay in every + # TU's `DatInit`: they are top-level code, not a definition, and every module + # computes the same values. + m.s[cfsStrData].addDeclWithVisibility(Extern): + m.s[cfsStrData].addVar(kind = Local, name = name, typ = "TNimTypeV2") + m.s[cfsVars].add(cnifDefDirective(name, "d", icNifName(m, origType))) + var def = newBuilder("") + def.addDeclWithVisibility(Private): + def.addVar(kind = Local, name = name, typ = "TNimTypeV2") + m.s[cfsVars].add extract(def) + m.s[cfsVars].add(cnifEndDefs()) m.icDataDefs.add (name, icNifName(m, origType)) + else: + m.s[cfsStrData].addDeclWithVisibility(Private): + m.s[cfsStrData].addVar(kind = Local, name = name, typ = "TNimTypeV2") var flags = 0 if not canFormAcycle(m.g.graph, t): flags = flags or 1 diff --git a/compiler/cgen.nim b/compiler/cgen.nim index 8c4fbe87c4..4250575c09 100644 --- a/compiler/cgen.nim +++ b/compiler/cgen.nim @@ -1635,8 +1635,17 @@ proc genProcLvl3*(m: BModule, prc: PSym) = # `extern`/`rtl` pragma at sem time), so its uses are invisible to the # artifact's liveness walk — conservatively keep the definition. defFlags.add 'x' - m.s[cfsProcs].add(cnifDefDirective(stripCnifMarks(prc.loc.snippet), defFlags, - icNifName(m, prc))) + # A C++ member's `loc.snippet` is a CALL PATTERN (`#->salute(@)`), not a + # linker name — and every member of that name, in every class, mints the + # same one. Ownership is assigned per name, so `Loo::salute` and `Foo::salute` + # collided: the merge stage handed both to one artifact and the other TU's + # definition was dropped (undefined vtable at link). Key member definitions by + # their NIF name instead, which is unique by construction. Dots cannot occur + # in a mangled C name, so the two namespaces stay disjoint. + let defName = + if sfCppMember * prc.flags != {}: icNifName(m, prc) + else: stripCnifMarks(prc.loc.snippet) + m.s[cfsProcs].add(cnifDefDirective(defName, defFlags, icNifName(m, prc))) m.s[cfsProcs].add(extract(generatedProc)) m.s[cfsProcs].add(cnifEndDefs()) else: @@ -1661,7 +1670,20 @@ proc requiresExternC(m: BModule; sym: PSym): bool {.inline.} = proc genProcPrototype(m: BModule, sym: PSym) = useHeader(m, sym) - if lfNoDecl in sym.loc.flags or sfCppMember * sym.flags != {}: return + if lfNoDecl in sym.loc.flags: return + if sfCppMember * sym.flags != {}: + # A C++ member is declared INSIDE its class, never as a free prototype — but + # this TU still needs its CALL-SITE name (`x->salute(@)`), and only + # `genMemberProcHeader` derives that (from the pragma's declaration pattern). + # Whole-program cgen got it for free: the module defining the member was code + # generated in the same process, ahead of any caller. The per-module backend + # emits that body in ANOTHER process, so the caller was left with the mangled + # Nim name `fillBackendName` minted and C++ rejected + # `loo->salute_u0__vireouyks1()` ("struct Loo has no member named ..."). + if m.compileToCpp: + var scratch = newBuilder("") + genMemberProcHeader(m, sym, scratch, false, true) + return if lfDynamicLib in sym.loc.flags: if m.config.cmd == cmdNifC and m.config.icBackendStage == "cg": # Under IC per-module cg every demander emits the dynlib proc's DEFINITION @@ -2611,6 +2633,7 @@ proc genModule(m: BModule, cfile: Cfile): Rope = m.icDataDefs, semmedNif = toNifFilename(m.config, FileIndex m.module.position), moduleBase = getSomeNameForModule(m), + globalDtor = m.icGlobalDtorName, implDeps = implDeps) m.g.graph.icCnifFiles.add artifact # NB: under cmdNifC the returned text still carries the cnif marks; the @@ -2710,7 +2733,7 @@ proc getCFile*(m: BModule): AbsoluteFile = let ext = if m.compileToCpp: ".nim.cpp" elif m.config.backend == backendObjc or sfCompileToObjc in m.module.flags: ".nim.m" - else: ".nim.c" + else: icCFileExt(m.config) result = changeFileExt(completeCfilePath(m.config, mangleModuleName(m.config, m.cfilename).AbsoluteFile), ext) when false: @@ -2855,6 +2878,42 @@ proc generateLibraryDestroyGlobals(graph: ModuleGraph; m: BModule; body: PNode; theProc[bodyPos] = body result.ast = theProc +proc genIcModuleDestroyGlobals*(graph: ModuleGraph; m: BModule): string = + ## Per-module backend (`cg` stage), non-main module: wrap this module's + ## accumulated top-level global destructors in a nullary exported proc and + ## return its C name ("" when there are none). + ## + ## `graph.globalDestructors` is filled while a module's own `cg` process + ## injects destructors into its top level, but the teardown code is emitted + ## by the MAIN module's `cg` — a different process, whose `graph` only ever + ## sees its own entries. So each module emits its own teardown here and + ## records the name in its `.c.nif` meta head; the main module's `cg` reads + ## the heads (like it already does for init/datInit) and calls them. + result = "" + if graph.globalDestructors.len == 0: return + var body = newNodeI(nkStmtList, m.module.info) + for i in countdown(high(graph.globalDestructors), 0): + body.add graph.globalDestructors[i] + body.flags.incl nfTransf # should not be further transformed + graph.globalDestructors.setLen 0 + + result = m.config.nimMainPrefix & "NimDestroyGlobals__" & $getSomeNameForModule(m) + let procname = getIdent(graph.cache, result) + var dtor = newSym(skProc, procname, m.idgen, m.module.owner, m.module.info) + dtor.typ = newProcType(m.module.info, m.idgen, dtor) + dtor.typ.callConv = ccNimCall + backendEnsureMutable dtor + incl dtor.flagsImpl, sfExportc # a root for the merge stage's DCE: nothing + # inside this TU calls it, only main does + dtor.locImpl.snippet = result + + let theProc = newNodeI(nkProcDef, m.module.info, bodyPos+1) + for i in 0.. 0 and sfMainModule in m.module.flags and + {optGenStaticLib, optGenDynLib, optNoMain} * m.config.globalOptions == {}: + # Per-module backend: the other modules' top-level global destructors were + # emitted into their own TUs (`genIcModuleDestroyGlobals`); call them from + # the end of the main module's init proc — which IS the program body — right + # after main's own destructors, in the order `generateCgStage` computed + # (reverse dependency order, mirroring whole-program cgen's single reversed + # `globalDestructors` list). The lib/noMain flavour — where the whole-program + # backend collects the destructors into an exported `NimDestroyGlobals` + # instead — is not reachable: `nim ic` only builds executables. + for dn in graph.icModuleDtors: + m.g.mainModProcs.addDeclWithVisibility(Private): + m.g.mainModProcs.addProcHeader(ccNimCall, dn, CVoid, cProcParams()) + m.g.mainModProcs.finishProcHeaderAsProto() + m.initProc.s(cpsStmts).addCallStmt(markCName(dn)) + if m.hcrOn: # make sure this is pulled in (meaning hcrGetGlobal() is called for it during init) let sym = magicsys.getCompilerProc(m.g.graph, "programResult") diff --git a/compiler/cgendata.nim b/compiler/cgendata.nim index 8e096d4f00..ae4cc58391 100644 --- a/compiler/cgendata.nim +++ b/compiler/cgendata.nim @@ -186,6 +186,10 @@ type # embeds (redirected defs, shared instances, # hooks); recorded as the artifact's cdeps so # the reuse gate can check their impl cookies + icGlobalDtorName*: string # per-module backend: the C name of this + # module's global-destructor proc, recorded in + # the artifact's meta head so the main module's + # `cg` — a different process — can call it icDataDefs*: seq[tuple[cname, nifname: string]] # C names of data definitions (consts, globals, # RTTI) this TU embeds plus their NIF symbol diff --git a/compiler/cnif.nim b/compiler/cnif.nim index 0caebd8682..2e12331efe 100644 --- a/compiler/cnif.nim +++ b/compiler/cnif.nim @@ -73,14 +73,15 @@ proc stripCnifMarks*(s: string): string = inc i const - CnifVersion* = "4" + CnifVersion* = "5" ## Artifact format version, stored in the meta head. Artifacts written ## by an older compiler lack the NIF names and the cref group the ## def-retention check needs (v2), the cdeps group the fine-grained - ## reuse gate needs (v3), or the type NIF names and cnif-marked extern + ## reuse gate needs (v3), the type NIF names and cnif-marked extern ## RTTI references the typeinfo flavor of the def-retention check - ## needs (v4); `readCnifHeads` reports them as invalid so their TUs - ## simply regenerate once. + ## needs (v4), or the global-destructor name the main module's `cg` + ## calls at teardown (v5); `readCnifHeads` reports them as invalid so + ## their TUs simply regenerate once. proc cnifDefDirective*(name, flags, nifName: string): string = CnifDefStart & name & CnifDefSep & flags & CnifDefSep & nifName & CnifDefEnd @@ -91,15 +92,17 @@ proc cnifEndDefs*(): string = proc writeCnifArtifact*(code: string; outfile: string; initRequired = false; datInitRequired = false; dataDefs: openArray[tuple[cname, nifname: string]] = []; - semmedNif = ""; moduleBase = ""; + semmedNif = ""; moduleBase = ""; globalDtor = ""; implDeps: openArray[string] = []) = ## Splits the marked module text into the `.c.nif` artifact. ## The artifact starts with a `(meta "semmedNif" "moduleBase" - ## "version")` head — whether the module has an init/datInit proc - ## ('i'/'d'), which semmed NIF it was generated from and the module's + ## "version" "globalDtor")` head — whether the module has an init/datInit + ## proc ('i'/'d'), which semmed NIF it was generated from, the module's ## mangled base name (what `registerModuleToMain` and the reuse decision ## need when the TU is reused in a later run, possibly without the module - ## ever being loaded again) — a `(cdata (SymbolDef StrLit)*)` group naming + ## ever being loaded again) and the C name of the module's global-destructor + ## proc, if any (what the main module's `cg` calls at program teardown; see + ## `cgen.genIcModuleDestroyGlobals`) — a `(cdata (SymbolDef StrLit)*)` group naming ## the data definitions (consts, globals, RTTI) the TU embeds together ## with their NIF names, a `(cref Ident*)` group naming every C name ## the TU references but does not define itself (what the def-retention @@ -153,6 +156,7 @@ proc writeCnifArtifact*(code: string; outfile: string; b.addStrLit semmedNif b.addStrLit moduleBase b.addStrLit CnifVersion + b.addStrLit globalDtor b.withTree "cdata": for d in dataDefs: b.addSymbolDef d.cname @@ -261,6 +265,8 @@ type datInitRequired*: bool semmedNif*: string ## the semmed NIF this TU was generated from moduleBase*: string ## the module's mangled base name + globalDtor*: string ## C name of the module's global-destructor proc + ## ("" when the module has no global destructors) cdefs*: seq[tuple[cname, nifname: string]] ## the proc definitions cdata*: seq[tuple[cname, nifname: string]] ## the data definitions crefs*: seq[string] ## C names referenced but not defined here @@ -303,6 +309,7 @@ proc readCnifHeads*(f: string): CnifHeads = if strIdx == 0: result.semmedNif = strVal(c) elif strIdx == 1: result.moduleBase = strVal(c) elif strIdx == 2: version = strVal(c) + elif strIdx == 3: result.globalDtor = strVal(c) inc strIdx inc c else: @@ -585,6 +592,13 @@ proc computeMergeDecision*(files: openArray[string]): MergeDecision = if d in result.live: inc result.liveDefs const MergeDecisionFile* = "ic.backend.merge.nif" +const LiveModulesFile* = "ic.backend.live.txt" + ## One `.c.nif` path per line: exactly the artifacts of the modules the CURRENT + ## build graph considers live. The `merge` stage reads this instead of globbing + ## `*.c.nif` off the nimcache, so a leftover artifact from an unrelated build + ## that happens to share the cache directory cannot be merged in (which is what + ## made a shared prebuilt cache unusable: merge picked owners in modules the + ## program does not import, and the link then wanted their objects). ## Fixed name of the merge stage's output in the nimcache, read by `emit`. proc writeMergeDecision*(outfile: string; d: MergeDecision) = diff --git a/compiler/commands.nim b/compiler/commands.nim index 597063a680..680ab8427e 100644 --- a/compiler/commands.nim +++ b/compiler/commands.nim @@ -627,7 +627,11 @@ proc processMemoryManagementOption(switch, arg: string, pass: TCmdLinePass, conf.selectedGC = gcHooks defineSymbol(conf.symbols, "gchooks") incl conf.globalOptions, optSeqDestructors - processOnOffSwitchG(conf, {optSeqDestructors}, arg, pass, info) + # (The `arg` here is the mm MODE — "hooks" — so feeding it to an on/off + # switch made `--mm:hooks` fail outright with "'on' or 'off' expected, but + # 'hooks' found". The `incl` above is what that call was meant to do. + # Reachable only via the explicit switch: `--newruntime` sets + # `selectedGC` directly, which is why this stayed hidden.) if pass in {passCmd2, passPP}: defineSymbol(conf.symbols, "nimSeqsV2") of "go": @@ -1088,9 +1092,14 @@ proc processSwitch*(switch, arg: string, pass: TCmdLinePass, info: TLineInfo; expectNoArg(conf, switch, arg, pass, info) helpOnError(conf, pass) of "symbolfiles", "incremental", "ic": - if switch.normalize == "symbolfiles": deprecatedAlias(switch, "incremental") + if pass in {passCmd2, passPP} and switch.normalize == "symbolfiles": + deprecatedAlias(switch, "incremental") # xxx maybe also ic, since not in help? - if pass in {passCmd2, passPP}: + # `--ic:on` is read in passCmd1 too: `nim.nim` decides BEFORE config loading + # whether this run is an IC driver (`ensureIcConfig` must produce the + # precompiled config the driver itself then replays), and passCmd1 is the + # only pass that has run by then. + if pass in {passCmd1, passCmd2, passPP}: case arg.normalize of "on": conf.ic = true of "legacy": conf.symbolFiles = v2Sf diff --git a/compiler/deps.nim b/compiler/deps.nim index 63ef12a2b1..9450670edf 100644 --- a/compiler/deps.nim +++ b/compiler/deps.nim @@ -11,6 +11,7 @@ ## This enables incremental and parallel compilation using the `m` switch. import std / [os, tables, sets, times, osproc, algorithm, strtabs, strutils, syncio] +from std/sha1 import secureHash, `$` import options, msgs, lineinfos, pathutils, condsyms, modulepaths, extccomp, cnif, platform @@ -26,6 +27,14 @@ type Node = ref object files: seq[FilePair] # main file + includes deps: seq[int] # indices into DepContext.nodes + specDeps: seq[int] # the subset of `deps` reached ONLY through a `when` + # condition the scanner could not evaluate + missingImport: string # an `import` path this module's source names, under a + # `when` the scanner could not decide, that does not + # exist on disk (empty when all resolved) + missingHardImport: string ## ditto but NOT under any undecidable `when`: the + ## real compile would reach this `import`, so it is + ## a genuine "cannot open file" error id: int DepContext = object @@ -41,6 +50,9 @@ type scanningMain: bool # currently scanning the project main module's deps; # makes `when isMainModule` conditions evaluate true # only there (every other module is imported) + speculating: int # nesting depth of `when` guards the scanner could not + # decide; every import edge added while this is > 0 is + # recorded as speculative (see pruneDeadSpeculative) proc toPair(c: DepContext; f: string): FilePair = FilePair(nimFile: f, modname: moduleSuffix(f, cast[seq[string]](c.config.searchPaths))) @@ -206,6 +218,18 @@ proc getsImplicitImports(c: DepContext; nimFile: string): bool = ## system.nim and never reaches them). Stdlib == under conf.libpath. not isRelativeTo(nimFile, c.config.libpath.string) +proc addDepEdge(c: DepContext; current: Node; depId: int) = + ## Record `current -> depId`. While the scanner is inside a `when` guard it + ## could not evaluate (`c.speculating > 0`) the edge is *speculative*: it may + ## not exist in the real compile at all. An edge seen at least once outside + ## such a guard is hard and stays hard. + if depId notin current.deps: current.deps.add depId + if c.speculating > 0: + if depId notin current.specDeps: current.specDeps.add depId + else: + let i = current.specDeps.find(depId) + if i >= 0: current.specDeps.delete i + proc processImport(c: var DepContext; importPath: string; current: Node; origin: string) = # `origin` = the file the `import` literally appears in. Crucial for imports # inside `include`d files: e.g. `system.nim` includes `system/excpt.nim`, which @@ -217,6 +241,14 @@ proc processImport(c: var DepContext; importPath: string; current: Node; origin: # only after the post-sem `.s.deps` revealed the edge. let resolved = resolveImport(c, origin, importPath) if resolved.len == 0 or not fileExists(resolved): + # The module does not exist on disk. Silently ignoring this is right for the + # scanner (the `import` may sit in a dead `when` branch and the real compile + # never looks at it), but remember it: `pruneDeadSpeculative` uses it to tell + # a module that is merely unused apart from one that cannot compile at all. + if c.speculating > 0: + if current.missingImport.len == 0: current.missingImport = importPath + elif current.missingHardImport.len == 0: + current.missingHardImport = importPath return let pair = c.toPair(resolved) @@ -225,7 +257,7 @@ proc processImport(c: var DepContext; importPath: string; current: Node; origin: if existingIdx == -1: # New module - create node and process it let newNode = Node(files: @[pair], id: c.nodes.len) - current.deps.add newNode.id + addDepEdge(c, current, newNode.id) # Every module depends on system.nim if c.systemNodeId >= 0: newNode.deps.add c.systemNodeId @@ -243,8 +275,7 @@ proc processImport(c: var DepContext; importPath: string; current: Node; origin: traverseDeps(c, pair, newNode) else: # Already processed - just add dependency - if existingIdx notin current.deps: - current.deps.add existingIdx + addDepEdge(c, current, existingIdx) proc skipSubtree(s: var Stream; first: PackedToken) = ## Consume tokens until the ParLe at `first` is balanced. Caller has @@ -533,14 +564,19 @@ proc readDepsFile(c: var DepContext; pair: FilePair; current: Node) = # entirely. Otherwise advance past the marker and parse the path. t = next(s) var live = true + var speculative = false if t.kind == ParLe and pool.tags[t.tagId] == "when": # whenMarkerHolds consumes everything up to and including the # closing `)` of the `(when ...)` subtree. Drop the import only when # the condition is PROVABLY false; a `cvUnknown` condition (e.g. an # `else:` branch guarded by `not `, as in # `when tryImport x: ... else: import x`) keeps the dependency so the - # static graph never misses a real import. - live = whenMarkerHolds(c, s) != cvFalse + # static graph never misses a real import — but marks every edge it + # creates speculative, so `pruneDeadSpeculative` can still drop a + # subtree that provably cannot compile in this configuration. + let cond = whenMarkerHolds(c, s) + live = cond != cvFalse + speculative = cond == cvUnknown t = next(s) if not live: # Drain the rest of this import/include node. @@ -558,6 +594,7 @@ proc readDepsFile(c: var DepContext; pair: FilePair; current: Node) = # that expand to several imports. A plain `import a, b, c` lists several # modules as siblings; a `fromimport` has a single path followed by the # imported symbol list, which must not be treated as modules. + if speculative: inc c.speculating if tag == "fromimport" or tag == "importexcept": # `from m import syms` / `import m except syms`: the first child is the # module path; the rest is the (in/ex)cluded symbol list, which must not @@ -573,6 +610,7 @@ proc readDepsFile(c: var DepContext; pair: FilePair; current: Node) = processInclude(c, importPath, current, pair.nimFile) else: processImport(c, importPath, current, pair.nimFile) + if speculative: dec c.speculating # Drain any remaining tokens of this node (e.g. the symbol list of a # `fromimport`), up to and including the node's closing ')'. var depth = 1 @@ -689,6 +727,131 @@ proc traverseDeps(c: var DepContext; pair: FilePair; current: Node) = return readDepsFile(c, pair, current) +proc pruneDeadSpeculative(c: var DepContext) = + ## Drop modules that are reachable only through a `when` guard the scanner + ## cannot evaluate AND that cannot possibly compile because they import a + ## module which does not exist on disk. + ## + ## The motivating shape is the ordinary `{.strdefine.}` backend switch: + ## + ## const figdrawTextBackend* {.strdefine.} = "pixie" + ## when figdrawTextBackend == "harfbuzzy": + ## import ./textrasters/glyphid_raster # imports `pkg/harfbuzzy` + ## + ## The value of that const needs sem, so `evalCondCmp` answers `cvUnknown` and + ## the conservative rule keeps the import — the right call for an edge, but it + ## also gives `glyphid_raster` its own `nim m` rule. The classic compiler never + ## looks at that file; IC compiles it, cannot find `pkg/harfbuzzy`, and the + ## whole build dies on a package the user never installed because they never + ## selected that backend. + ## + ## Dropping is safe: if the guard *was* live, the importer's own `nim m` fails + ## on the missing NIF, records the import in its `.s.deps` sidecar, and the + ## discovery fixpoint re-adds the node — this time reporting the honest + ## `cannot open file: pkg/harfbuzzy/raw` instead of a cascade of + ## `undeclared identifier` noise. + let n = c.nodes.len + if n == 0: return + + var roots = @[0] + if c.systemNodeId >= 0: roots.add c.systemNodeId + for i in c.implicitNodeIds: roots.add i + + # Reachability through NON-speculative edges only: these modules are compiled + # for certain, so a missing import in them is a genuine user error to report. + var hard = newSeq[bool](n) + var stack = roots + while stack.len > 0: + let v = stack.pop() + if hard[v]: continue + hard[v] = true + for d in c.nodes[v].deps: + if d notin c.nodes[v].specDeps and not hard[d]: stack.add d + + # A module the real compile DOES reach, naming an import that is not on disk, + # is a plain user error — and one nifmake cannot notice on its own: deleting + # `effects.nim` moves no mtime, so the importer's `nim m` never re-fires and + # `nim ic` happily relinked a stale binary while `nim c` said "cannot open + # file". Report it here, where the graph scan is the only thing that looks at + # import paths at all. + var reported = false + for i in 0 ..< n: + if hard[i] and c.nodes[i].missingHardImport.len > 0: + rawMessage(c.config, errGenerated, + c.nodes[i].files[0].nimFile & ": cannot open file: " & + c.nodes[i].missingHardImport) + reported = true + if reported: return + + var dead = newSeq[bool](n) + var anyDead = false + for i in 0 ..< n: + if not hard[i] and c.nodes[i].missingImport.len > 0: + dead[i] = true + anyDead = true + if not anyDead: return + + # Anything left reachable only through a dead node is dead too. + var alive = newSeq[bool](n) + stack = @[] + for r in roots: + if not dead[r]: stack.add r + while stack.len > 0: + let v = stack.pop() + if alive[v]: continue + alive[v] = true + for d in c.nodes[v].deps: + if not dead[d] and not alive[d]: stack.add d + + var cascaded = 0 + for i in 0 ..< n: + if not alive[i]: + # Drop the scan artifacts of a module that just left the graph. `nifler` + # ran on it during `traverseDeps` (that is how we learned it cannot + # build), and leaving its `.p.nif`/`.deps.nif` behind makes an + # edit-accumulated cache differ from a clean one for no reason. Re-running + # nifler if it ever comes back costs a single parse. + for f in c.nodes[i].files: + removeFile(c.parsedFile(f)) + removeFile(c.depsFile(f)) + removeFile(c.parsedFile(f).changeFileExt("") & ".deps.nif") + if c.nodes[i].missingImport.len > 0: + rawMessage(c.config, hintSuccess, + "ic: skipping " & c.nodes[i].files[0].nimFile & + " (reached only under an undecidable `when`, and imports " & + c.nodes[i].missingImport & ", which is not installed)") + else: + inc cascaded + if cascaded > 0: + rawMessage(c.config, hintSuccess, + "ic: " & $cascaded & " further module(s) skipped, reachable only through those") + + # Compact `c.nodes`; node ids ARE indices everywhere, so remap them all. + var remap = newSeq[int](n) + var newNodes: seq[Node] = @[] + for i in 0 ..< n: + if alive[i]: + remap[i] = newNodes.len + newNodes.add c.nodes[i] + else: + remap[i] = -1 + proc remapped(remap: seq[int]; src: seq[int]): seq[int] = + result = @[] + for x in src: + if remap[x] >= 0 and remap[x] notin result: result.add remap[x] + for node in newNodes: + node.id = remap[node.id] + node.deps = remapped(remap, node.deps) + node.specDeps = remapped(remap, node.specDeps) + c.nodes = newNodes + + var pm = initTable[string, int]() + for name, idx in c.processedModules: + if idx >= 0 and idx < n and remap[idx] >= 0: pm[name] = remap[idx] + c.processedModules = pm + if c.systemNodeId >= 0: c.systemNodeId = remap[c.systemNodeId] + c.implicitNodeIds = remapped(remap, c.implicitNodeIds) + proc computeSCCs(c: DepContext): seq[seq[int]] = ## Tarjan's strongly-connected-components over the module dependency graph ## (`node.deps`). Each returned component is a list of node indices; a module @@ -771,6 +934,19 @@ proc computeForwardedArgs(c: DepContext): seq[string] = # them — phantom outputs that re-fire the build on every rerun). if c.config.selectedGC != gcUnselected: result.add "--mm:" & $c.config.selectedGC + # The children are invoked as `nim m` / `nim nifc`, so the driver's own command + # token (`c`, `cpp`, `ic`) is gone and with it the backend it selected. Name it + # explicitly — `nim cpp --ic:on` must not have its stdlib sem'd and its TUs + # emitted as C. The exception model rides along for the same reason: `nim cpp` + # defaults to `--exceptions:cpp`, which changes both codegen and sem. + if c.config.backend != backendInvalid: + result.add "--backend:" & $c.config.backend + if c.config.exc != excNone: + result.add "--exceptions:" & (case c.config.exc + of excGoto: "goto" + of excCpp: "cpp" + of excQuirky: "quirky" + else: "setjmp") # method dispatch semantics must match across the child processes: # a child compiled without --multimethods:on builds different dispatch # buckets (and rejects calls as ambiguous that multi-dispatch accepts) @@ -798,6 +974,71 @@ proc computeForwardedArgs(c: DepContext): seq[string] = # replayed (`conf.icPreparsedConfig`); `commandIc` has already guaranteed it # exists, else it bailed. result.add "--icPreparsedConfig:" & c.config.icPreparsedConfig + # Everything else the user typed on the `nim ic` command line. The children + # replay the project's CONFIG FILES (ic_config.cfg.nif), never the driver's + # argv, so a switch that exists only there — `--opt:speed`, `--panics:on`, + # `--experimental:…`, `--passC:…` — silently did not reach them: `nim ic + # --opt:speed` produced a byte-identical debug binary. Forward the switches + # verbatim, minus the ones that MUST differ per child (the output/cache paths, + # the command itself, and IC's own per-rule switches, which each rule sets). + const notForwarded = [ + "nimcache", "out", "o", "outdir", "usenimcache", "run", "r", + "incremental", "ic", "symbolfiles", "genbif", + "icproject", "icpreparsedconfig", "icconfigout", "icgroup", + "icbackendstage", "icbackendmodule", "ismainmodule", + "help", "h", "fullhelp", "version", "v", "advanced"] + for a in commandLineParams(): + if a.len < 2 or a[0] != '-': continue + var i = 1 + if i < a.len and a[i] == '-': inc i + var name = "" + while i < a.len and a[i] notin {':', '='}: + name.add a[i] + inc i + if normalize(name) notin notForwarded and a notin result: + result.add a + +proc configSignatureFile(c: DepContext; forwardedArgs: seq[string]): string = + ## nifmake decides staleness from file mtimes alone — it never looks at a + ## rule's command line. So changing `-d:someDefine`, `--mm:` or `--threads:` + ## between two `nim ic` runs re-generated the build file with the new switches + ## but re-fired nothing: the user got a silently stale binary built with the + ## OLD configuration. Reify the configuration as a FILE and make every rule + ## that consumes it an input, so a config change moves an mtime like any edit. + ## Written `OnlyIfChanged` so a genuine no-op run stays a no-op. + ## + ## Deliberately EXCLUDES the two per-build path switches (`--icproject:`, + ## `--icPreparsedConfig:`): they name where this build lives, not what it + ## produces, so including them made the signature differ between two caches + ## holding byte-identical artifacts — which defeats prefilling a test's cache + ## from a shared warm one (every rule would re-fire on the rewritten + ## signature). The precompiled config still counts, by CONTENT: a `nim.cfg` + ## edit changes the artifact, hence the hash, hence every rule. + result = getNimcacheDir(c.config).string / "ic_build_args.txt" + var content = "" + for p in c.config.searchPaths: + content.add "--path:" & p.string & "\n" + for a in forwardedArgs: + if a.startsWith("--icproject:") or a.startsWith("--icPreparsedConfig:"): + continue + content.add a & "\n" + if c.config.icPreparsedConfig.len > 0 and fileExists(c.config.icPreparsedConfig): + # Hash the precompiled config MINUS its `(nimcache "...")` entry — the one + # line in the artifact that records where this build's cache lives rather + # than what the config says. Everything else is genuinely config-derived, so + # two builds with the same `nim.cfg`/`config.nims` hash the same no matter + # which directory they run in. + var normalized = "" + try: + for line in lines(c.config.icPreparsedConfig): + if "(nimcache " in line: continue + normalized.add line + normalized.add '\n' + except IOError, OSError: + normalized = c.config.icPreparsedConfig + content.add "config:" & $secureHash(normalized) & "\n" + if not fileExists(result) or readFile(result) != content: + writeFile(result, content) proc generateFrontendBuildFile(c: DepContext; forwardedArgs: seq[string]): string = ## Frontend build file: the nifler (parse) and `nim m` (sem) rules only. The @@ -878,6 +1119,7 @@ proc generateFrontendBuildFile(c: DepContext; forwardedArgs: seq[string]): strin # a NIF for each. Only dependencies *outside* the component become build-graph # inputs — intra-component edges are produced by this very rule and listing # them would reintroduce the cycle nifmake just rejected. + let argsFile = configSignatureFile(c, forwardedArgs) let sccs = computeSCCs(c) var sccOf = newSeq[int](c.nodes.len) for sccId, comp in sccs: @@ -906,6 +1148,10 @@ proc generateFrontendBuildFile(c: DepContext; forwardedArgs: seq[string]): strin # Input 0 (the project file passed to `nim m`): the representative's .nim. b.withTree "input": b.addStrLit repPair.nimFile + # The configuration this child is invoked with (see configSignatureFile). + b.addTree "input" + b.addStrLit argsFile + b.endTree() # All parsed files of every member (nifler outputs this group consumes). for m in members: for f in c.nodes[m].files: @@ -999,7 +1245,7 @@ proc backendCFile(c: DepContext; node: Node): string = if node.id == 0: AbsoluteFile node.files[0].nimFile else: AbsoluteFile node.files[0].modname result = changeFileExt(completeCfilePath(c.config, - mangleModuleName(c.config, cfilename).AbsoluteFile), ".nim.c").string + mangleModuleName(c.config, cfilename).AbsoluteFile), icCFileExt(c.config)).string proc computeLiveBackendNodes(c: DepContext): seq[bool] = ## Which nodes the backend must code-generate: the closure reachable from the @@ -1096,6 +1342,8 @@ proc generateBackendBuildFile(c: DepContext; forwardedArgs: seq[string]): string if prunedStale: removeFile(mergeFile) + let argsFile = configSignatureFile(c, forwardedArgs) + var b = nifbuilder.open(result) defer: b.close() @@ -1153,6 +1401,7 @@ proc generateBackendBuildFile(c: DepContext; forwardedArgs: seq[string]): string b.addStrLit "--icBackendStage:lower" b.addStrLit "--icBackendModule:" & node.files[0].modname inputStr c.semmedFile(node.files[0]) + inputStr argsFile outputStr tFiles[i] b.endTree() @@ -1174,6 +1423,7 @@ proc generateBackendBuildFile(c: DepContext; forwardedArgs: seq[string]): string b.addStrLit "--icBackendStage:cg" b.addStrLit "--icBackendModule:" & node.files[0].modname inputStr tFiles[i] + inputStr argsFile if node.id == 0: for j in 0 ..< c.nodes.len: if c.nodes[j].id != 0 and live[j]: @@ -1181,13 +1431,29 @@ proc generateBackendBuildFile(c: DepContext; forwardedArgs: seq[string]): string outputStr cnifFiles[i] b.endTree() - # merge: read every `.c.nif`, write the ownership/liveness decision. + # merge: read the live modules' `.c.nif`, write the ownership/liveness + # decision. The list is handed over as a FILE (`LiveModulesFile`) because the + # merge child is a separate process that never sees the build file: without it + # merge globbed `*.c.nif` off the nimcache and so silently absorbed artifacts + # belonging to some other program that shares the directory. + let liveFile = nimcache / LiveModulesFile + block: + var manifest = "" + for i in 0 ..< c.nodes.len: + if live[i]: + manifest.add cnifFiles[i] + manifest.add "\n" + # OnlyIfChanged: its mtime is a merge input, so rewriting it every run would + # re-fire merge (and, through the decision, every `emit`) on a no-op build. + if not fileExists(liveFile) or readFile(liveFile) != manifest: + writeFile(liveFile, manifest) b.addTree "do" b.addIdent "nim_nifc" b.withTree "args": b.addStrLit "--icBackendStage:merge" for i in 0 ..< c.nodes.len: if live[i]: inputStr cnifFiles[i] + inputStr liveFile outputStr mergeFile b.endTree() @@ -1221,11 +1487,59 @@ proc generateBackendBuildFile(c: DepContext; forwardedArgs: seq[string]): string b.addStrLit "--out:" & exeFile for i in 0 ..< c.nodes.len: if live[i]: inputStr cFiles[i] + inputStr argsFile outputStr exeFile b.endTree() b.endTree() # stmts +proc deriveFromSemDeps(c: var DepContext): bool = + ## Fold every already-compiled module's `.s.deps` sidecar (its REAL post-sem + ## imports, macro-generated ones included) back into the graph. Returns true + ## if anything new was added. + ## + ## Run BEFORE the first nifmake pass as well as after a failure. The static + ## scanner cannot see `parseStmt("import dyn")`, so on the run that first hits + ## it the frontend fails, this recovers the node, and the retry succeeds. But + ## the graph is rebuilt from scratch on every `nim ic`, so on the NEXT run the + ## frontend succeeds on round one — with `dyn` absent from the graph again, + ## hence with no nifler/`nim m` rule of its own and no edge into its importer. + ## Editing `dyn.nim` then changed nothing at all: the build silently reused the + ## `.s.bif` from the run that discovered it. Seeding from the sidecars makes + ## the discovery stick across runs. + ## + ## The edges are recorded SPECULATIVELY: a sidecar says what the module + ## imported the last time it was semmed, which is a statement about the past. + ## Flip a `when`, or delete an `import`, and a module that is no longer reached + ## would otherwise linger in the graph forever (and fail to build, if what it + ## imports is gone). Marking the edge speculative lets `pruneDeadSpeculative` + ## drop such a leftover, while a genuinely-needed macro import — which compiles + ## fine — stays. + result = false + inc c.speculating + defer: dec c.speculating + let n0 = c.nodes.len # snapshot: new nodes are traversed as they're added + for ni in 0 ..< n0: + for p in readSemDeps(c, c.nodes[ni].files[0]): + let pair = c.toPair(p) + var idx = c.processedModules.getOrDefault(pair.modname, -1) + if idx == -1: + if not fileExists(pair.nimFile): continue + let newNode = Node(files: @[pair], id: c.nodes.len) + if c.systemNodeId >= 0: + newNode.deps.add c.systemNodeId + if getsImplicitImports(c, pair.nimFile): + for impId in c.implicitNodeIds: + if impId != newNode.id: newNode.deps.add impId + c.processedModules[pair.modname] = newNode.id + c.nodes.add newNode + idx = newNode.id + traverseDeps(c, pair, newNode) + result = true + if idx != ni and idx notin c.nodes[ni].deps: + addDepEdge(c, c.nodes[ni], idx) + result = true + proc commandIc*(conf: ConfigRef; frontendOnly = false) = ## Main entry point for `nim ic`. With `frontendOnly` (used by `nim track` for ## IDE queries) it runs only Phase 1 — the incremental nifler + `nim m` @@ -1323,6 +1637,17 @@ proc commandIc*(conf: ConfigRef; frontendOnly = false) = # Process dependencies traverseDeps(c, rootPair, rootNode) + # Re-apply what earlier runs discovered post-sem (macro-generated imports), + # so those modules keep their rules on a warm build instead of vanishing from + # the graph until the next failure. No-op on a cold cache. Runs BEFORE the + # prune so a sidecar entry that has since gone stale is prunable too. + discard deriveFromSemDeps(c) + + # Modules that only a `when` the scanner cannot decide pulls in, and that + # import something not installed, are dead in this configuration; scheduling + # them would fail the build over code the classic compiler never reads. + pruneDeadSpeculative(c) + # Discovery via `.s.deps`: imports GENERATED by macros (chronicles builds # `import chronicles/textlines` via parseStmt from the chronicles_sinks # define) are invisible to the static scanner. Each `nim m` records the @@ -1393,28 +1718,20 @@ proc commandIc*(conf: ConfigRef; frontendOnly = false) = var discovered = false inc rounds if rounds <= 20: - let n0 = c.nodes.len # snapshot: new nodes are traversed as they're added - for ni in 0 ..< n0: - for p in readSemDeps(c, c.nodes[ni].files[0]): - let pair = c.toPair(p) - var idx = c.processedModules.getOrDefault(pair.modname, -1) - if idx == -1: - let newNode = Node(files: @[pair], id: c.nodes.len) - if c.systemNodeId >= 0: - newNode.deps.add c.systemNodeId - if getsImplicitImports(c, pair.nimFile): - for impId in c.implicitNodeIds: - if impId != newNode.id: newNode.deps.add impId - c.processedModules[pair.modname] = newNode.id - c.nodes.add newNode - idx = newNode.id - traverseDeps(c, pair, newNode) - discovered = true - if idx != ni and idx notin c.nodes[ni].deps: - c.nodes[ni].deps.add idx - discovered = true + discovered = deriveFromSemDeps(c) if not discovered: - rawMessage(conf, errGenerated, "nifmake failed with exit code: " & $exitCode) + # The children have already printed the real diagnostics. Adding an + # `Error:` line of our own here made a build-system status the LAST error + # in the stream, hiding the compiler's own message from anything that + # reads the final error (testament's `errormsg:`, editors, CI log + # scrapers) — every `reject`-style test under `nim ic` reported + # "nifmake failed with exit code: 1" instead of what the compiler said. + # The non-zero exit is what signals failure; this line is context. + rawMessage(conf, hintExecuting, + "nifmake reported failures (exit code " & $exitCode & ")") + # Fail the run without printing an `Error:` of our own (see above): the + # exit code is derived from `errorCounter`. + inc conf.errorCounter break # Phase 2 — backend (whole-program `nim nifc`), run once over the now-final @@ -1429,6 +1746,8 @@ proc commandIc*(conf: ConfigRef; frontendOnly = false) = rawMessage(conf, hintExecuting, cmd) let exitCode = execShellCmd(cmd) if exitCode != 0: - rawMessage(conf, errGenerated, "nifmake (backend) failed with exit code: " & $exitCode) + rawMessage(conf, hintExecuting, + "nifmake reported backend failures (exit code " & $exitCode & ")") + inc conf.errorCounter else: rawMessage(conf, errGenerated, "nim ic not available in bootstrap build") diff --git a/compiler/icconfig.nim b/compiler/icconfig.nim index a0255c23dc..55c8ad8815 100644 --- a/compiler/icconfig.nim +++ b/compiler/icconfig.nim @@ -32,7 +32,7 @@ ## would misresolve. import options, commands, lineinfos, pathutils, msgs -import std/[algorithm, os, sets, osproc, times, streams, syncio] +import std/[algorithm, os, sets, osproc, times, streams, syncio, strutils] import "../dist/nimony/src/lib" / [nifbuilder, nifcoreparse] const @@ -269,11 +269,26 @@ proc ensureIcConfig*(conf: ConfigRef) = # verbatim: all `-`-prefixed switches first (in encounter order), then the # non-switch project token(s). The producer re-reads `nim.cfg` itself. var pargs = @["icconfig", "--icConfigOut:" & outPath] + # The command token is dropped below, so `nim cpp --ic:on` would hand the + # producer a C-backend config: name the backend explicitly. (`nim ic + # --backend:cpp` already carries the switch; the duplicate is harmless.) + if conf.backend != backendInvalid: + pargs.add "--backend:" & $conf.backend var rest: seq[string] = @[] var droppedCmd = false for a in commandLineParams(): if a.len == 0: continue if a[0] == '-': + # `--run`/`-r` must not reach the producer: it only serialises the + # resolved config, has no output binary, and `nim.nim`'s run step asserts + # on the empty `outFile` (`nim cpp --ic:on -r foo.nim`). + var name = "" + var i = 1 + if i < a.len and a[i] == '-': inc i + while i < a.len and a[i] notin {':', '='}: + name.add a[i] + inc i + if normalize(name) in ["r", "run"]: continue pargs.add a elif not droppedCmd: droppedCmd = true # drop the original command token (`ic`/`track`) diff --git a/compiler/injectdestructors.nim b/compiler/injectdestructors.nim index bc99008a0d..ba05ffb6a6 100644 --- a/compiler/injectdestructors.nim +++ b/compiler/injectdestructors.nim @@ -1119,6 +1119,11 @@ proc p(n: PNode; c: var Con; s: var Scope; mode: ProcessMode; tmpFlags = {sfSing result[i] = n[i] of nkGotoState, nkState, nkAsmStmt: result = n + of nkReplayAction: + # A `.rod`/NIF replay record. It only ever appears in a NIF-loaded + # module's TOP-LEVEL statements (the loader prepends the `(replay ...)` + # entries there); cgen discards it, so pass it through untouched. + result = n else: result = nil internalError(c.graph.config, n.info, "cannot inject destructors to node kind: " & $n.kind) diff --git a/compiler/main.nim b/compiler/main.nim index 0365eba486..cddf2fc96b 100644 --- a/compiler/main.nim +++ b/compiler/main.nim @@ -29,7 +29,7 @@ when defined(nimPreviewSlimSystem): import ../dist/checksums/src/checksums/sha1 import pipelines -from icconfig import produceIcConfig +from icconfig import produceIcConfig, ensureIcConfig when not defined(nimKochBootstrap): import nifbackend @@ -269,6 +269,28 @@ proc mainCommand*(graph: ModuleGraph) = proc compileToBackend() = customizeForBackend(conf.backend) + if isIcDriver(conf): + # `nim c --ic:on` / `nim cpp --ic:on`: same driver as `nim ic`, entered + # through the ordinary compile command so every backend switch the user + # already knows keeps working (`nim cpp`, `--exceptions:`, `-d:`, ...). + # `customizeForBackend` above has already defined the backend symbol and + # picked the exception model, which is exactly what the per-module + # children must inherit — `computeForwardedArgs` forwards both. + setUseIc(true) + wantMainModule(conf) + setOutFile(conf) + when not defined(nimKochBootstrap): + if conf.icPreparsedConfig.len == 0: + # `--ic:on` came from a `nim.cfg`/`config.nims` rather than the command + # line, so `nim.nim` could not see it before config loading and the + # precompiled config the children replay does not exist yet. Produce it + # now. (The driver then keeps the config IT parsed instead of replaying + # the artifact; both come from the same files.) + ensureIcConfig(conf) + commandIc(conf) + else: + rawMessage(conf, errGenerated, "--ic:on not available in bootstrap build") + return setOutFile(conf) case conf.backend of backendC: commandCompileToC(graph) diff --git a/compiler/modulegraphs.nim b/compiler/modulegraphs.nim index 4cdeac0348..31ae39828b 100644 --- a/compiler/modulegraphs.nim +++ b/compiler/modulegraphs.nim @@ -136,6 +136,10 @@ type systemModule*: PSym sysTypes*: array[TTypeKind, PType] compilerprocs*: TStrTable + missingCompilerProcs*: HashSet[string] + # `nim nifc` only: compilerproc names no + # loaded module defines, so the whole-program + # index scan in `loadCompilerProc` runs once exposed*: TStrTable packageTypes*: TStrTable emptyNode*: PNode @@ -165,6 +169,11 @@ type onDefinitionResolveForward*: proc (graph: ModuleGraph; s: PSym; info: TLineInfo) {.nimcall.} onUsage*: proc (graph: ModuleGraph; s: PSym; info: TLineInfo) {.nimcall.} globalDestructors*: seq[PNode] + icModuleDtors*: seq[string] # per-module backend: the C names of the + # other modules' global-destructor procs + # (`genIcModuleDestroyGlobals`), already in + # call order; only the main module's `cg` + # fills this, from the `.c.nif` meta heads strongSemCheck*: proc (graph: ModuleGraph; owner: PSym; body: PNode) {.nimcall.} compatibleProps*: proc (graph: ModuleGraph; formal, actual: PType): bool {.nimcall.} idgen*: IdGenerator @@ -481,6 +490,49 @@ proc logMethodDef*(g: ModuleGraph; s: PSym) = g.opsLog.add LogEntry(kind: MethodEntry, module: s.itemId.module.int, key: "", sym: s) +proc logCppMember*(g: ModuleGraph; s: PSym) = + ## Log a C++ `{.member.}`/`{.virtual.}`/`{.constructor.}` registration (and the + ## `importcpp` default-initializer flavour) so the NIF backend can rebuild + ## `memberProcsPerType`/`initializersPerType`, which live only in the sem + ## process. Without them the per-module backend emitted the struct WITHOUT its + ## in-class member declarations and the out-of-class definitions did not match + ## ("no declaration matches 'void Doo::memberProc()'"). + ## + ## No type key: `replayCppMember` re-derives the type from the routine's + ## signature exactly as `semCppMember` does, so nothing has to survive the + ## round trip except the routine itself. + if g.config.cmd in {cmdNifC, cmdM}: + g.opsLog.add LogEntry(kind: CppMemberEntry, module: s.itemId.module.int, + key: "", sym: s) + +proc replayCppMember*(g: ModuleGraph; s: PSym) = + ## Inverse of `logCppMember`, mirroring `semstmts.semCppMember`'s derivation. + if s == nil or s.typ == nil: return + if sfImportc notin s.flags: + var typ = if sfConstructor in s.flags: s.typ.returnType else: s.typ.firstParamType + if typ != nil and typ.kind == tyPtr and sfConstructor notin s.flags: + typ = typ.elementType + if typ != nil and typ.kind == tyObject: + let procs = addr g.memberProcsPerType.mgetOrPut(typ.bindingId, @[]) + for prc in procs[]: + if prc == s: return + procs[].add s + else: + let typ = s.typ.returnType + if typ != nil and typ.kind == tyObject and + typ.bindingId notin g.initializersPerType and s.typ.n != nil: + # The default values sem read off the `nkIdentDefs` live on the param syms. + var call = newTree(nkCall, newSymNode(s)) + var isInitializer = s.typ.n.len > 1 + for i in 1 ..< s.typ.n.len: + let p = s.typ.n[i] + if p.kind != nkSym or p.sym.ast == nil or p.sym.ast.kind == nkEmpty: + isInitializer = false + break + call.add p.sym.ast + if isInitializer: + g.initializersPerType[typ.bindingId] = call + proc registerLoadedMethod*(g: ModuleGraph; m: PSym) = ## Rebuild the dispatch buckets from a serialized method registration. ## Buckets group the methods sharing a dispatcher; the dispatcher's BODY @@ -638,6 +690,29 @@ proc loadCompilerProc*(g: ModuleGraph; name: string): PSym = strTableAdd(g.compilerprocs, result) return result + # `nim nifc`: a module loaded from a NIF is named by its mangled suffix + # (`thrkxstl4`), not by its source name, and its file index resolves to + # that suffix too — so the `"threadpool"` match below can never fire and + # `spawn`, expanded at codegen time, died on `system module needs: + # nimArgsPassingDone`. The backend loads the WHOLE program before + # codegen starts, so just consult every loaded module's index; a miss is + # final for the rest of the process (nothing more gets loaded) and is + # remembered, because `getCompilerProc` is also used as a mere presence + # probe and would otherwise rescan every index on every call. + if g.config.cmd == cmdNifC: + if name in g.missingCompilerProcs: return nil + for moduleIdx in 0.. 0: g.icModuleDtors.add heads.globalDtor + # `ordered` is dependency (post-order) init order; teardown runs in reverse, + # so an importer's globals are destroyed before the ones it may still point + # at. This mirrors whole-program cgen, which walks its single accumulated + # `globalDestructors` list backwards. Main's own destructors come first and + # are added by `finalCodegenActions` itself. + reverse g.icModuleDtors let tb = bl.mods[target.module.position] if tb != nil: finishModule(g, tb) @@ -724,8 +739,19 @@ proc generateMergeStage(g: ModuleGraph) = ## in-process first-claimant/DCE coordination. let nimcache = getNimcacheDir(g.config).string var files: seq[string] = @[] - for artifact in walkFiles(nimcache / "*.c.nif"): - files.add artifact + # The driver lists the live modules' artifacts explicitly (deps.nim's + # `writeLiveModules`); only fall back to globbing when that manifest is + # absent (a cache written by an older compiler). Globbing merges whatever + # `.c.nif` happens to sit in the directory, which is wrong the moment the + # cache is shared with another program — see `LiveModulesFile`. + let manifest = nimcache / LiveModulesFile + if fileExists(manifest): + for line in lines(manifest): + let p = line.strip() + if p.len > 0: files.add p + else: + for artifact in walkFiles(nimcache / ("*" & icCFileExt(g.config) & ".nif")): + files.add artifact sort files let decision = computeMergeDecision(files) if decision.broken: @@ -764,7 +790,7 @@ proc generateEmitStage(g: ModuleGraph; mainFileIdx: FileIndex) = if targetIsMain: AbsoluteFile toFullPath(g.config, mainFileIdx) else: AbsoluteFile g.config.icBackendModule let cfile = changeFileExt(completeCfilePath(g.config, - mangleModuleName(g.config, cfilename).AbsoluteFile), ".nim.c").string + mangleModuleName(g.config, cfilename).AbsoluteFile), icCFileExt(g.config)).string let artifact = cfile & ".nif" if not fileExists(artifact): rawMessage(g.config, errGenerated, @@ -847,7 +873,7 @@ proc generateLinkStage(g: ModuleGraph; mainFileIdx: FileIndex) = if not decision.broken: var liveOwners = initHashSet[string]() for cname, owner in decision.owners: - if owner.endsWith(".c.nif") and cname in decision.live: + if owner.endsWith(icCFileExt(g.config) & ".nif") and cname in decision.live: liveOwners.incl owner for owner in liveOwners: let cbase = owner[0 ..< owner.len - ".nif".len] # "@m….nim.c.nif" -> ".c" diff --git a/compiler/nim.nim b/compiler/nim.nim index 789657e216..68765365a2 100644 --- a/compiler/nim.nim +++ b/compiler/nim.nim @@ -120,7 +120,7 @@ proc handleCmdLine(cache: IdentCache; conf: ConfigRef) = # so `loadConfigs` replays it instead of re-parsing the `nim.cfg` chain — the # driver runs on the exact same config its children will. See icconfig.nim. when not defined(nimKochBootstrap): - if conf.cmd in {cmdIc, cmdTrack}: + if conf.cmd in {cmdIc, cmdTrack} or isIcDriver(conf): ensureIcConfig(conf) var graph = newModuleGraph(cache, conf) diff --git a/compiler/options.nim b/compiler/options.nim index 69b28ab6f3..a8a874a7f5 100644 --- a/compiler/options.nim +++ b/compiler/options.nim @@ -29,7 +29,7 @@ const nimEnableCovariance* = defined(nimEnableCovariance) - icFormatVersion* = "34" + icFormatVersion* = "37" ## Version of the IC cache format (the sem-NIF module layout written by ## ast2nif.nim plus the iface/impl/edges side files). Bump it whenever ## that layout changes: `commandIc` wipes a nimcache whose `ic.version` @@ -940,6 +940,24 @@ proc getOsCacheDir(): string = else: result = getHomeDir() / genSubDir.string +proc isIcDriver*(conf: ConfigRef): bool = + ## True for `nim c --ic:on` / `nim cpp --ic:on`: this process is the `nim ic` + ## DRIVER (it builds the nifmake graph and spawns the per-module children), + ## not a compilation. `nim ic` itself keeps its own `cmdIc` branch. + conf.ic and conf.cmd in {cmdCompileToC, cmdCompileToCpp, cmdCompileToOC} + +proc icCFileExt*(conf: ConfigRef): string = + ## The extension the per-module backend gives a module's translation unit. + ## Mirrors `cgen.getCFile` at BACKEND granularity, which is all the `nim ic` + ## driver can know: it DECLARES every module's `.c`/`.cpp` output to nifmake + ## without loading a single module, so a per-module `{.compile: cpp.}` + ## (`sfCompileToCpp`) is out of reach — and `nim cpp` selects the backend for + ## the whole program anyway. + case conf.backend + of backendCpp: ".nim.cpp" + of backendObjc: ".nim.m" + else: ".nim.c" + proc getNimcacheDir*(conf: ConfigRef): AbsoluteDir = proc nimcacheSuffix(conf: ConfigRef): string = if conf.ideActive: "_nimsuggest" # dedicated cache, never shared with `nim c` diff --git a/compiler/pipelines.nim b/compiler/pipelines.nim index fbfdb948f7..b28b60644a 100644 --- a/compiler/pipelines.nim +++ b/compiler/pipelines.nim @@ -248,7 +248,15 @@ proc processPipelineModule*(graph: ModuleGraph; module: PSym; idgen: IdGenerator # current strongly-connected import group (`--icGroup`) are the exception: # they are compiled from source here, so each must write its own NIF. let shouldWriteNif = - if graph.config.ideActive: + if graph.config.errorCounter > 0: + # Never persist an artifact built from erroneous AST. `nim m` does exit + # non-zero, but its outputs would still land on disk NEWER than their + # inputs, so nifmake sees the rule as satisfied on the next run: the + # build then "succeeds" from a poisoned NIF — a silently wrong binary, + # or an internal error once codegen meets an `nkError` body. Leaving the + # outputs missing keeps the rule dirty so it re-fires and re-reports. + false + elif graph.config.ideActive: # nimsuggest (cmdM): persist NIF for cleanly-compiled, SAVED modules so # later queries load them instead of recompiling. Never persist the # actively edited buffer (it may hold unsaved/incomplete code) nor a @@ -320,10 +328,17 @@ proc processPipelineModule*(graph: ModuleGraph; module: PSym; idgen: IdGenerator let firstUnusedId = max(idgen.symId, idgen.typeId) var expansions: seq[(PSym, TLineInfo)] = @[] discard graph.nifExpansions.take(module.position.int32, expansions) + # The module symbol's own backend-relevant flags. `sfInjectDestructors` is + # set by sempass2 when the module's TOP-LEVEL statements need the + # destructor pass; `moduleFromNifFile` builds a fresh module PSym, so + # without persisting it `cgen.genTopLevelStmt` skipped + # `injectDestructorCalls` and top-level locals were never destroyed. + let moduleFlags = + if sfInjectDestructors in module.flags: ModFlagInjectDestructors else: 0'i32 writeNifModule(graph.config, module.position.int32, topLevelStmts, graph.opsLog, replayActions, implDeps, reexportedModuleSyms(graph, module), genericOffers, typeOffers, resolvedImportDeps, firstUnusedId, - expansions) + expansions, moduleFlags) # The module's REAL direct imports (incl. macro-generated) for `nim ic`'s # graph re-derivation; see ast2nif.writeSemDeps / semdata.addImportFileDep. var semDepPaths: seq[string] = @[] diff --git a/compiler/semmagic.nim b/compiler/semmagic.nim index 0e1eab91ea..27b738ef3d 100644 --- a/compiler/semmagic.nim +++ b/compiler/semmagic.nim @@ -478,6 +478,15 @@ proc turnFinalizerIntoDestructor(c: PContext; orig: PSym; info: TLineInfo): PSym # proc signature: result.typ = newProcType(result.info, c.idgen, result) result.typ.addParam newParam + # `transform` only rewrites the PARAMETER, so the copied AST still names `orig` + # at `namePos`. Make the definition name itself, the invariant every other + # routine AST keeps: the NIF writer re-derives a routine's serialized AST from + # `ast[namePos].sym.ast` (ast2nif's `nkProcDef` branch), so a stale name node + # made this proc serialize `orig`'s body — whose parameter belongs to `orig`. + # Lambda lifting then saw the body's parameter as a variable captured from + # another proc and aborted with "internal error: environment misses: x". + if result.ast != nil and result.ast.safeLen > namePos: + result.ast[namePos] = newSymNode(result, result.info) proc semQuantifier(c: PContext; n: PNode): PNode = checkSonsLen(n, 2, c.config) diff --git a/compiler/semstmts.nim b/compiler/semstmts.nim index 9b9e8a75b9..6f9696cea6 100644 --- a/compiler/semstmts.nim +++ b/compiler/semstmts.nim @@ -2410,6 +2410,7 @@ proc semCppMember(c: PContext; s: PSym; n: PNode) = localError(c.config, n.info, pragmaName & " must be either ptr to object or object type.") if sameOwners(typ.owner, s.owner) and sameOwners(c.module, s.owner): c.graph.memberProcsPerType.mgetOrPut(typ.bindingId, @[]).add s + logCppMember(c.graph, s) else: localError(c.config, n.info, pragmaName & " procs must be defined in the same scope as the type they are virtual for and it must be a top level scope") @@ -2432,6 +2433,7 @@ proc semCppMember(c: PContext; s: PSym; n: PNode) = inc j if isInitializer: c.graph.initializersPerType[typ.bindingId] = initializerCall + logCppMember(c.graph, s) proc semMethodPrototype(c: PContext; s: PSym; n: PNode) = if s.isGenericRoutine: diff --git a/doc/ic.md b/doc/ic.md index 1b619aa263..a6e6860597 100644 --- a/doc/ic.md +++ b/doc/ic.md @@ -2,12 +2,23 @@ Incremental Compilation (IC) ====================================== -The ``nim ic`` command provides incremental compilation for Nim projects. It -decomposes compilation into per-module steps whose results are cached as NIF -files, and uses the external ``nifmake`` build tool to re-run only the steps -whose inputs changed. +``--ic:on`` turns an ordinary compile into an incremental one. It decomposes +compilation into per-module steps whose results are cached as NIF files, and +uses the external ``nifmake`` build tool to re-run only the steps whose inputs +changed. -This document describes **how `nim ic` works today**, including the edge cases +.. code-block:: cmd + + nim c --ic:on myproject.nim + nim cpp --ic:on myproject.nim + +It is a switch on the normal compile commands, not a command of its own, so +everything else keeps working unchanged: ``cpp`` and ``objc`` backends, ``-r``, +``-d:release``, ``--exceptions:``, and a project-wide opt-in from ``nim.cfg`` / +``config.nims``. The older spelling ``nim ic`` still works and drives the same +code, but it is the C backend only and cannot run the binary it built. + +This document describes **how IC works today**, including the edge cases that shaped the current design. The per-module backend rewrite that earlier editions of this document listed as a *Plan* has **landed**: the whole-program, reuse/redirect/def-retention backend is gone and codegen is now a set of @@ -16,7 +27,7 @@ reuse/redirect/def-retention backend is gone and codegen is now a set of Overview ======== -The pipeline has two halves driven by one process (`nim ic`, `commandIc` in +The pipeline has two halves driven by one process (the *driver*, `commandIc` in ``compiler/deps.nim``) that constructs a dependency graph, writes a build file, and hands it to ``nifmake``: @@ -220,7 +231,7 @@ Edge cases (and why the machinery exists) - **Config cost.** Each child re-parsing `nim.cfg` + re-running `config.nims` in the VM was ~80 ms; replaced by a precompiled `ic_config.cfg.nif` replayed in `loadConfigs` (`compiler/icconfig.nim`). -- **`koch bootic`** bootstraps the compiler through `nim ic` (a 3-iteration +- **`koch bootic`** bootstraps the compiler through `--ic:on` (a 3-iteration fixed-point check). It writes its binary to ``bin/nim_ic`` and never clobbers ``bin/nim``. @@ -247,7 +258,7 @@ Known residual hack Status and performance ====================== -`nim ic` self-builds the compiler (`koch bootic`'s byte-identical fixed-point +IC self-builds the compiler (`koch bootic`'s byte-identical fixed-point check) under both `orc` and `--mm:refc`, and passes the external-package CI set. Cold full bootstrap on a 32-core box (`-d:release`, **no edits** — IC's worst @@ -256,7 +267,7 @@ case, since incremental reuse is not exercised): | | wall | notes | | - | ---- | ----- | | `koch boot` (classic) | ~1m00s | reference | -| `koch bootic` (`nim ic`) | ~1m39s | **~1.66×** | +| `koch bootic` (`--ic:on`) | ~1m39s | **~1.66×** | This is down from ~7.5× in the whole-program-backend era. IC does modestly more aggregate work (more processes, NIF re-parsing of imports per process), but on a @@ -405,3 +416,101 @@ See also - NIF format spec: [nifspec/doc/nif-spec.md](../nifspec/doc/nif-spec.md) - NIFC (C-like target) spec: dist/nimony/doc/nifc-spec.md + +Testing IC +========== + +Two mechanisms, at very different scales. + +**`tests/ic` — metamorphic tests.** A `t*.nim` whose body contains `#? metamorphic` +drives a sequence of cross-module edits through the IC driver in one fixed build +directory (see `testament/categories.nim`, `runMetamorphicIcTest`). Directives: + +| directive | effect | +| --------- | ------ | +| ``#!FILE `` | (re)write a module in the virtual file system | +| ``#!DELETE `` | remove a module, from the vfs and from disk | +| ``#!FLAGS `` | change the compiler switches from here on | +| ``#!STEP `` | materialise the files, build, run, check | + +Step attributes: ``expect: ``, ``fails: `` (BOTH compilers must +reject it, with that text), ``noop``, ``body-edit``, ``iface-edit``, +``modules: ``, ``clean``, ``no-oracle``. + +Every successful step is **also compiled with `nim c` and run, and the two +outputs must agree**. That oracle is the only check in the suite that is not +IC-against-IC: `clean == incremental`, `noop changes nothing` and the cookie +invariants are all satisfied by an IC that is *consistently* wrong, which is how +two silent miscompilations survived (a NIF-loaded module's `sfInjectDestructors` +was lost, so top-level destructors were never injected; `nfFirstWrite`/`nfLastRead` +had nowhere to live on a serialized sym node, so every first assignment to a +destructor-bearing local became `=sink` over zeroed memory). `koch bootic` has the +same blind spot — it proves the compiler reproduces *itself*. + +**`testament --ic` — the whole corpus.** Appends `--ic:on` to every C and C++ +test compile, so IC inherits the existing ~10k programs and their expected +output instead of the handful written for it by hand. Because it is a switch and +not a command, a test that overrides the command wholesale (`cmd: "nim cpp -r +$file"`) simply gains the switch — no verb rewriting, and the C++ corpus comes +along for free. Each also gets a private nimcache; without one they would share +a cache and thrash it. + +To keep that affordable, testament borrows nimony's hastur model +(`warmupSharedCache` + `prefillFromWarmup`): a generated warmup program pulling in +`system` and the most-imported stdlib modules is compiled once per distinct +compile configuration into `nimcache/ic_warmup_`, and each test's empty +cache is seeded from it with **mtimes preserved** (nifmake compares +output-mtime > input-mtime, so stamping the copies "now" would re-fire the whole +graph). Only program-independent artifacts are copied — the frontend NIFs and +cookies plus the per-module `lower`/`cg` outputs. The `.c`/`.o` are deliberately +left behind: the merge decision (which module owns each emit-everywhere +definition) is whole-program, so those are re-rendered for every program anyway. + +Measured on `tests/destructor` (97 test runs, 32-core box): + +| | cold | warm | +| - | ---- | ---- | +| `nim c` | 35s | 32s | +| `--ic:on` | ~3m30 | **9.8s** | + +The warm number is the developer loop and it is 3.2x faster than the classic +backend; the cold number is paid once per configuration and then cached on disk. +The disk cost is real and worth knowing: ~3.4 GB of nimcache for that one +category. + +One property of an incremental compiler is worth spelling out because it looks +like a test bug: **a cached stage emits no diagnostics**. `--expandArc` output, a +hint, a warning — all of it is produced by the process that actually runs, so a +build that reuses every artifact prints nothing. Tests that check `nimout` (and +anything you are debugging by eye) therefore need a cold cache; running the same +test twice in a row makes the second run's `nimout` empty. + +The C++ backend +=============== + +``nim cpp --ic:on`` works, and `tests/cpp` passes under it. Three things had to +change for that, and they are worth knowing because they are the shape of every +"C++ needs the whole program" problem the per-module backend has: + +* **The driver must name the right file.** ``deps.nim`` DECLARES each module's + translation unit to ``nifmake`` without loading a single module, so it cannot + ask ``cgen.getCFile``; ``options.icCFileExt`` mirrors that formula at backend + granularity (``.nim.cpp`` / ``.nim.m`` / ``.nim.c``). + +* **C++ has no designated initializers**, so the RTTI record is a bare variable + that ``DatInit`` fills field by field. That bare ``TNimTypeV2 x;`` is a + tentative definition, which C's linker merges and C++'s does not — every TU + that demanded the type defined it. It now gets the same extern-declaration + + owned-``'d'``-definition split the C flavour has. + +* **A C++ member is declared inside its class.** ``memberProcsPerType`` and + ``initializersPerType`` live only in the sem process, so the backend emitted + the struct WITHOUT its member declarations; they are replayed from a + ``(repcppmember …)`` log entry now (``modulegraphs.replayCppMember`` re-derives + the type from the routine's signature, exactly as ``semCppMember`` does). + Two follow-on details: a member's ``loc.snippet`` is a CALL PATTERN + (``#->salute(@)``), so it must be computed even in the TU that only *calls* the + member (whole-program cgen got that for free by generating the defining module + first), and it is not a linker name — every ``salute`` member in every class + mints the same one, so definitions are keyed by their NIF name in the merge + stage instead. diff --git a/koch.nim b/koch.nim index 50d76e03d1..225956c639 100644 --- a/koch.nim +++ b/koch.nim @@ -76,7 +76,7 @@ Options: --skipIntegrityCheck skips integrity check when booting the compiler Possible Commands: boot [options] bootstraps with given command line options - bootic [options] bootstraps via the incremental compiler (`nim ic`) + bootic [options] bootstraps via the incremental compiler (`--ic:on`) distrohelper [bindir] helper for distro packagers tools builds Nim related tools toolsNoExternal builds Nim related tools (except external tools, @@ -450,7 +450,7 @@ proc bootic(args: string, skipIntegrityCheck: bool) = # everything. if i > 0: removeDir smartNimcache let nimi = if i == 0: nimStart else: i.thVersion - exec "$# ic --nimcache:$# $# compiler" / "nim.nim" % + exec "$# c --ic:on --nimcache:$# $# compiler" / "nim.nim" % [nimi, smartNimcache, args] if sameFileContent(output, i.thVersion): copyExe(output, finalDest) @@ -615,7 +615,7 @@ proc runIcTestFile(inp: string) = for fragment in content.split("#!EDIT!#"): let file = inp.replace(".nim", "_temp.nim") writeFile(file, fragment) - var cmd = nimExe & " ic --hint:Conf:off --warnings:off " + var cmd = nimExe & " c --ic:on --hint:Conf:off --warnings:off " cmd.add quoteShell(file) exec(cmd) diff --git a/testament/categories.nim b/testament/categories.nim index 7ed98b8aa9..377955feb5 100644 --- a/testament/categories.nim +++ b/testament/categories.nim @@ -516,7 +516,15 @@ proc testNimblePackages(r: var TResults; cat: Category; packageFilter: string) = # accumulated file set is materialised before each `#!STEP`. A `#!STEP`'s # attributes are `;`-separated, each either `key: value` or a bare flag: # expect: noop body-edit iface-edit modules: clean +# fails: no-oracle # The last step always also runs the clean==incremental check. +# +# Every successful step is ALSO compiled with `nim c` and run, and the two +# outputs must agree (`no-oracle` opts out). This is the only check in the suite +# that is not IC-against-IC; without it a consistently wrong IC passes +# everything. `#!DELETE ` removes a module, `#!FLAGS ` changes +# the compiler switches from that point on, and `fails: ` asserts that +# BOTH compilers reject the program with that text. type MetamorphicError = object of CatchableError resultKind: TResultEnum @@ -590,16 +598,34 @@ proc runMetamorphicIcTest(r: var TResults; file: string; cat: Category; options: let buildDir = (file.changeFileExt("") & "_mm").absolutePath let nc = buildDir / "nc" let bin = buildDir / "prog".addFileExt(ExeExt) + # The ORACLE: the same sources compiled by the classic backend. Every + # invariant this runner checked before was IC-against-IC (clean == incremental, + # no-op changes nothing, ...), which a *consistently* wrong IC satisfies + # perfectly — that is how a whole class of silent miscompilations (top-level + # destructors never injected; `nfFirstWrite`/`nfLastRead` dropped by the + # serializer, so every first assignment to a destructor-bearing local became + # `=sink` over zeroed memory) stayed invisible. `nim c` is the reference the + # suite was missing. + let ncRef = buildDir / "ncref" + let binRef = buildDir / "progref".addFileExt(ExeExt) removeDir(buildDir) createDir(buildDir) + # Extra switches for both compilers, settable per step via `#!FLAGS`. + var extraFlags: seq[string] = @[] + template compileIc(): untyped = - execCmdEx2(compilerPrefix, ["ic", "--hint:Conf:off", "--warnings:off", - "--nimcache:" & nc, "--out:" & bin, "main.nim"], + execCmdEx2(compilerPrefix, @["ic", "--hint:Conf:off", "--warnings:off", + "--nimcache:" & nc, "--out:" & bin] & extraFlags & @["main.nim"], + workingDir = buildDir) + + template compileRef(): untyped = + execCmdEx2(compilerPrefix, @["c", "--hint:Conf:off", "--warnings:off", + "--nimcache:" & ncRef, "--out:" & binRef] & extraFlags & @["main.nim"], workingDir = buildDir) # Parse the source into a flat op list: ("file", name, content) | ("step", attrs, ""). - type OpKind = enum opFile, opStep + type OpKind = enum opFile, opStep, opDelete, opFlags type Op = object kind: OpKind a, b: string @@ -615,6 +641,18 @@ proc runMetamorphicIcTest(r: var TResults; file: string; cat: Category; options: if s.startsWith("#!FILE"): flushFile() curName = s["#!FILE".len .. ^1].strip + elif s.startsWith("#!DELETE"): + # Remove a module from the virtual file system AND from disk. Deleting a + # still-imported file moves no mtime, so nothing in an mtime-keyed build + # re-fires: `nim ic` used to relink a stale binary where `nim c` reports + # `cannot open file`. Untestable until the format could express it. + flushFile() + ops.add Op(kind: opDelete, a: s["#!DELETE".len .. ^1].strip) + elif s.startsWith("#!FLAGS"): + # Change the compiler switches for the following steps. Config changes + # are not files, so an mtime-keyed build cannot see them either. + flushFile() + ops.add Op(kind: opFlags, a: s["#!FLAGS".len .. ^1].strip) elif s.startsWith("#!STEP"): flushFile() ops.add Op(kind: opStep, a: s["#!STEP".len .. ^1].strip) @@ -630,11 +668,21 @@ proc runMetamorphicIcTest(r: var TResults; file: string; cat: Category; options: var prevSnap = initTable[string, string]() var prevBin = "" var stepIdx = 0 + var deleted: seq[string] = @[] try: for o in ops: - if o.kind == opFile: + case o.kind + of opFile: vfs[o.a] = o.b continue + of opDelete: + vfs.del o.a + deleted.add o.a + continue + of opFlags: + extraFlags = o.a.splitWhitespace() + continue + of opStep: discard inc stepIdx let where = "step " & $stepIdx # Parse step attributes. @@ -646,8 +694,36 @@ proc runMetamorphicIcTest(r: var TResults; file: string; cat: Category; options: if c >= 0: attrs[p[0 ..< c].strip] = p[c+1 .. ^1].strip else: attrs[p] = "" + for fn in deleted: + removeFile(buildDir / fn) + deleted.setLen 0 for fn, content in vfs: writeFile(buildDir / fn, content) let (_, cout, ccode) = compileIc() + + # `fails: ` — the build MUST fail, with that text in its output. + # Without this every step had to succeed, so the whole error path was + # untested: a `nim m` that errored still wrote its `.s.bif`, nifmake then + # saw the rule satisfied, and the NEXT run reported success for a program + # that does not compile. + if "fails" in attrs: + if ccode == 0: + mmRaise(reBuildFailed, "a failed build", where & ": `nim ic` unexpectedly succeeded") + let want = attrs["fails"] + if want.len > 0 and want notin cout: + mmRaise(reOutputsDiffer, want, where & ": error text did not contain it:\n" & cout) + # The oracle must reject it too, else the test is asserting an IC-only + # error rather than a real one. + let (_, refOut, refCode) = compileRef() + if refCode == 0: + mmRaise(reBuildFailed, "`nim c` to fail too", + where & ": `nim ic` failed but `nim c` accepted the program:\n" & cout) + if want.len > 0 and want notin refOut: + mmRaise(reOutputsDiffer, want, + where & ": `nim c` failed differently:\n" & refOut) + prevSnap = snapshotDir(nc) + prevBin = "" + continue + if ccode != 0: mmRaise(reBuildFailed, "", where & ": `nim ic` failed:\n" & cout) let (_, rout, rcode) = execCmdEx2(bin.absolutePath, [], workingDir = buildDir) @@ -658,6 +734,22 @@ proc runMetamorphicIcTest(r: var TResults; file: string; cat: Category; options: if rout.strip == want.strip: discard else: mmRaise(reOutputsDiffer, want, where & " output:\n" & rout.strip) + # ORACLE: same sources through the classic backend, same observable + # behaviour. Unlike `expect:` this needs no foresight from the test author — + # it compares everything the program does, not only what someone thought to + # print, which is exactly what a silently-skipped destructor evades. + block oracle: + if "no-oracle" in attrs: break oracle + let (_, refCout, refCcode) = compileRef() + if refCcode != 0: + mmRaise(reBuildFailed, "", where & ": `nim c` (oracle) failed:\n" & refCout) + let (_, refRout, refRcode) = execCmdEx2(binRef.absolutePath, [], + workingDir = buildDir) + if refRout.strip != rout.strip or refRcode != rcode: + mmRaise(reOutputsDiffer, "`nim c` output:\n" & refRout.strip, + where & ": `nim ic` disagrees with `nim c`\n ic (exit " & $rcode & + "):\n" & rout.strip & "\n c (exit " & $refRcode & "):\n" & refRout.strip) + let snap = snapshotDir(nc) let binBytes = stableBinary(bin) if stepIdx > 1: @@ -755,6 +847,12 @@ proc processSingleTest(r: var TResults, cat: Category, options, test: string, ta let target = if cat.string.normalize == "js": targetJS else: targetC targets = {target} doAssert fileExists(test), test & " test does not exist" + # `testament r ` must dispatch metamorphic IC tests the same way + # `testament cat ic` does, otherwise a single-test run tries to parse the + # header as an ordinary spec and rejects it. + if isMetamorphicIcTest(readFile(test)): + runMetamorphicIcTest(r, test, cat, options) + return testSpec r, makeTest(test, options, cat), targets proc isJoinableSpec(spec: TSpec): bool = diff --git a/testament/testament.nim b/testament/testament.nim index 73af243c57..bf2bb87c49 100644 --- a/testament/testament.nim +++ b/testament/testament.nim @@ -12,7 +12,7 @@ import std/[strutils, pegs, os, osproc, streams, json, parseopt, browsers, terminal, exitprocs, - algorithm, times, intsets, macros] + algorithm, times, intsets, macros, tables] import backend, specs, azure, htmlgen @@ -35,6 +35,12 @@ var simulate = false var optVerbose = false var useMegatest = true var valgrindEnabled = true +var useIc = false + ## `--ic`: compile every C-target test with `nim ic` instead of `nim c`, so the + ## incremental compiler inherits the whole existing corpus (~10k programs with + ## expected output) instead of the handful of tests written for it by hand. + ## Every invariant the `tests/ic` suite checks is IC-against-IC; this is the + ## part that compares IC against the reference backend at scale. proc verboseCmd(cmd: string) = if optVerbose: @@ -58,6 +64,7 @@ Arguments: Options: --print print results to the console --verbose print commands (compiling and running tests) + --ic compile C-target tests with `nim ic` (incremental) --simulate see what tests would be run but don't run them (for debugging) --failing only show failing/ignored tests --targets:"c cpp js objc" run tests for specified targets (default: c) @@ -155,11 +162,40 @@ proc execCmdEx2(command: string, args: openArray[string]; workingDir: string = " if result.exitCode != -1: break close(p) -proc nimcacheDir(filename, options: string, target: TTarget): string = +proc nimcacheDir(filename, options: string, target: TTarget, + extraOptions = ""): string = ## Give each test a private nimcache dir so they don't clobber each other's. - let hashInput = options & $target + ## `extraOptions` (a `matrix:` entry) is part of the key: two matrix variants + ## of one file are two different compilations, and sharing a cache between them + ## means each run invalidates what the previous left. Harmless for the classic + ## backend, which caches only object files, but it makes an incremental cache + ## useless — every variant re-sems the world every time. + let hashInput = options & extraOptions & $target result = "nimcache" / (filename & '_' & hashInput.getMD5) +const icWarmupSource = """ +# Generated by testament for `--ic`. Compiling this once fills a shared IC cache +# with `system` and the stdlib modules the test corpus imports most, so each +# test's own cold build starts from precompiled NIFs instead of re-semming the +# world. Mirrors nimony's hastur `tools/warmup.nim` + `prefillFromWarmup`. +import std/[assertions, macros, strutils, tables, os, typetraits, sequtils, + sugar, math, options, times, json, sets, algorithm, hashes, + strformat, parseutils, streams, unicode] + +proc icWarmupAnchor*(): int = + # Reference a few generic instantiations the corpus leans on so their + # `.c.nif` artifacts are precompiled too, not just the modules' interfaces. + var t = initTable[string, int]() + t["a"] = 1 + var s = @[1, 2, 3] + s.sort() + result = s.len + t.len + "x".repeat(2).len +""" + +var icWarmupCaches: Table[string, string] + ## Compile-config key -> shared warm IC cache (or "" when unavailable). +var buildingIcWarmup = false + proc prepareTestCmd(cmdTemplate, filename, options, nimcache: string, target: TTarget, extraOptions = ""): string = var options = target.defaultOptions & ' ' & options @@ -169,9 +205,103 @@ proc prepareTestCmd(cmdTemplate, filename, options, nimcache: string, result = cmdTemplate % ["target", targetToCmd[target], "options", options, "file", filename.quoteShell, "filedir", filename.getFileDir(), "nim", compilerPrefix] + if useIc and target in {targetC, targetCpp}: + # `--ic:on` turns the ordinary compile command into the IC driver, so the + # verb is left alone: roughly half the corpus overrides the command wholesale + # (`cmd: "nim c --gc:arc $file"`), which neither goes through `$target` nor + # picks up `$options`, and such a test now simply gains the switch. Each also + # gets a private nimcache, which is what makes it incremental at all. + # + # Switches must land BEFORE the project file: anything after it is swallowed + # into `config.arguments`, and a non-empty `arguments` without `--run` is a + # hard error ("arguments can only be given if the '--run' option is + # selected"). + var switches = "--ic:on " + if nimcache.len > 0 and "--nimCache:" notin result and "--nimcache:" notin result: + switches.add "--nimCache:" & nimcache.quoteShell & " " + # `rfind`, not `find`: the private nimcache path embeds the test's file name + # (`nimcache/tests/destructor/tmove.nim_`), so the FIRST occurrence is + # inside a switch's value. The project file is the last one. + let fileArg = filename.quoteShell + let at = result.rfind(fileArg) + if at >= 0: result = result[0 ..< at] & switches & result[at .. ^1] + else: result.add " " & switches + +proc icWarmupCache(cmdTemplate, filename, options: string, target: TTarget, + extraOptions: string): string = + ## The shared warm cache for this test's exact compile configuration, built on + ## first use and kept in `nimcache/` across runs. Keyed by the switches AND the + ## test's directory, because both decide what the artifacts contain: the + ## switches through `-d:`/`--mm:` etc., the directory through the `nim.cfg` / + ## `config.nims` it inherits. A cache built under a different configuration + ## would just be invalidated wholesale on first use, which is worse than none. + if buildingIcWarmup: return "" + let dir = filename.getFileDir() + let key = options & extraOptions & $target & dir + if icWarmupCaches.hasKey(key): return icWarmupCaches[key] + result = "nimcache" / ("ic_warmup_" & key.getMD5) + icWarmupCaches[key] = result + if dirExists(result / "ic.version"): return # already built by an earlier run + if fileExists(result / "ic.version"): return + # The warmup must live in the test's own directory so it inherits the same + # config files; a stray `.nim` there is not picked up as a test (testament + # only collects `t*.nim`). The name must be a valid Nim identifier. + let src = dir / "icwarmup_generated.nim" + try: + writeFile(src, icWarmupSource) + except IOError, OSError: + icWarmupCaches[key] = "" + return "" + buildingIcWarmup = true + let cmd = prepareTestCmd(cmdTemplate, src, options, result, target, extraOptions) + let (outp, code) = execCmdEx(cmd) + buildingIcWarmup = false + try: removeFile(src) + except OSError: discard + if code != 0: + # Non-fatal: without a warm cache every test just pays its own cold build. + if optVerbose: echo "ic warmup failed: ", cmd, "\n", outp + icWarmupCaches[key] = "" + return "" + +proc prefillIcCache(warmup, nimcache: string) = + ## Seed a test's empty cache from the shared warm one. Only the artifacts that + ## do NOT depend on which program is being built are copied: the frontend NIFs + ## and cookies, plus the per-module `lower`/`cg` outputs. The `.c`/`.o` are + ## deliberately left out — the merge decision (who owns each emit-everywhere + ## definition) is whole-program, so those get re-rendered for every program + ## anyway and copying them is pure I/O. + ## + ## Mtimes are preserved, and that is load-bearing: nifmake decides staleness by + ## output-mtime > input-mtime, so stamping every prefilled file with "now" + ## would scramble the DAG ordering the warmup established and re-fire the + ## whole graph — exactly what the copy is meant to avoid. + if warmup.len == 0 or not dirExists(warmup): return + if dirExists(nimcache): return # the test already has its own cache + const wanted = [".p.nif", ".p.deps.nif", ".deps.nif", ".s.bif", ".iface.bif", + ".impl.bif", ".edges.bif", ".s.deps.bif", ".t.bif", + ".c.nif", ".cpp.nif"] + try: + createDir(nimcache) + for path in walkFiles(warmup / "*"): + let name = path.extractFilename + var take = name == "ic.version" or name == "ic_build_args.txt" + if not take: + for ext in wanted: + if name.endsWith(ext): take = true; break + if not take: continue + let dst = nimcache / name + copyFile(path, dst) + try: setLastModificationTime(dst, getLastModificationTime(path)) + except OSError, IOError: discard + except OSError, IOError: + discard # best effort; a cold build still works proc callNimCompiler(cmdTemplate, filename, options, nimcache: string, target: TTarget, extraOptions = ""): TSpec = + if useIc and target in {targetC, targetCpp} and nimcache.len > 0 and not buildingIcWarmup: + prefillIcCache(icWarmupCache(cmdTemplate, filename, options, target, extraOptions), + nimcache) result = TSpec(cmd: prepareTestCmd(cmdTemplate, filename, options, nimcache, target, extraOptions)) verboseCmd(result.cmd) @@ -415,21 +545,28 @@ proc cmpMsgs(r: var TResults, expected, given: TSpec, test: TTest, result = r.finishTestRetryable(test, target, extraOptions, expected.msg, given.msg, reSuccess) inc(r.passed) -proc generatedFile(test: TTest, target: TTarget): string = +proc generatedFile(test: TTest, target: TTarget, extraOptions: string): string = if target == targetJS: result = test.name.changeFileExt("js") else: let (_, name, _) = test.name.splitFile let ext = targetToExt[target] - result = nimcacheDir(test.name, test.options, target) / "@m" & name.changeFileExt(ext) + # `extraOptions` must match what `testSpecWithNimcache` passed to the + # compiler — the matrix entry is part of the nimcache key, so leaving it out + # here looks for the `.c` of a DIFFERENT variant's cache (which does not + # exist) and every `ccodeCheck` test with a `matrix:` failed as + # `reCodeNotFound`. + result = nimcacheDir(test.name, test.options, target, extraOptions) / + "@m" & name.changeFileExt(ext) proc needsCodegenCheck(spec: TSpec): bool = result = spec.maxCodeSize > 0 or spec.ccodeCheck.len > 0 -proc codegenCheck(test: TTest, target: TTarget, spec: TSpec, expectedMsg: var string, +proc codegenCheck(test: TTest, target: TTarget, extraOptions: string, + spec: TSpec, expectedMsg: var string, given: var TSpec) = try: - let genFile = generatedFile(test, target) + let genFile = generatedFile(test, target, extraOptions) let contents = readFile(genFile) for check in spec.ccodeCheck: if check.len > 0 and check[0] == '\\': @@ -457,7 +594,7 @@ proc compilerOutputTests(test: TTest, target: TTarget, extraOptions: string, var givenmsg: string = "" if given.err == reSuccess: if expected.needsCodegenCheck: - codegenCheck(test, target, expected, expectedmsg, given) + codegenCheck(test, target, extraOptions, expected, expectedmsg, given) givenmsg = given.msg if not nimoutCheck(expected, given) or not checkForInlineErrors(expected, given): @@ -590,7 +727,7 @@ proc targetHelper(r: var TResults, test: TTest, expected: TSpec, extraOptions: s inc count echo "testSpec count: ", count, " expected: ", expected else: - let nimcache = nimcacheDir(test.name, test.options, target) + let nimcache = nimcacheDir(test.name, test.options, target, extraOptions) var testClone = test let target = changeTarget(extraOptions, target) testSpecHelper(r, testClone, expected, target, extraOptions, nimcache) @@ -691,6 +828,7 @@ proc main() = case p.key.normalize of "print": optPrintResults = true of "verbose": optVerbose = true + of "ic": useIc = true of "failing": optFailing = true of "pedantic": discard # deadcode refs https://github.com/nim-lang/Nim/issues/16731 of "targets": diff --git a/tests/ic/tconfig_invalidation.nim b/tests/ic/tconfig_invalidation.nim new file mode 100644 index 0000000000..8f2f81f197 --- /dev/null +++ b/tests/ic/tconfig_invalidation.nim @@ -0,0 +1,34 @@ +discard """ + description: '''IC: changing the compiler switches must invalidate the cache''' +""" + +#? metamorphic + +# nifmake decides staleness from file mtimes and never looks at a rule's command +# line, so `-d:` / `--mm:` / `--opt:` changes re-generated the build file with +# the new switches and re-fired nothing: a silently stale binary built with the +# PREVIOUS configuration. And switches given only on the driver's command line +# never reached the per-module children at all, because they replay the +# project's config files rather than the driver's argv. + +#!FILE cfg.nim +const Mode* {.strdefine.} = "plain" + +proc describe*(): string = + when Mode == "loud": "LOUD" + elif Mode == "quiet": "quiet" + else: "plain" + +#!FILE main.nim +import cfg +echo describe() +#!STEP expect: plain + +#!FLAGS -d:Mode=loud +#!STEP expect: LOUD + +#!FLAGS -d:Mode=quiet +#!STEP expect: quiet + +#!FLAGS +#!STEP expect: plain diff --git a/tests/ic/tdead_when_import.nim b/tests/ic/tdead_when_import.nim new file mode 100644 index 0000000000..477a11197f --- /dev/null +++ b/tests/ic/tdead_when_import.nim @@ -0,0 +1,37 @@ +discard """ + description: '''IC: an import under an undecidable `when` must not be compiled''' +""" + +#? metamorphic + +# `when SomeStrdefineConst == "x": import y` is `cvUnknown` to the dependency +# scanner, which conservatively keeps the edge — right for an edge, but it also +# gave `y` its own `nim m` rule. `nim c` never looks at that file, so a build +# died on a package the user never installed because they never selected that +# backend. Selecting it must still produce the honest error. + +#!FILE needsmissing.nim +import pkg/definitely_not_an_installed_package +proc unreachable*(): string = "never" + +#!FILE guarded.nim +const Backend* {.strdefine.} = "plain" + +when Backend == "fancy": + import ./needsmissing + +proc pick*(): string = + when Backend == "fancy": unreachable() + else: "plain" + +#!FILE main.nim +import guarded +echo pick() +#!STEP expect: plain + +# selecting the branch that really does need the missing package must report it +#!FLAGS -d:Backend=fancy +#!STEP fails: cannot open file + +#!FLAGS +#!STEP expect: plain diff --git a/tests/ic/tdeleted_module.nim b/tests/ic/tdeleted_module.nim new file mode 100644 index 0000000000..c9ad4dcbca --- /dev/null +++ b/tests/ic/tdeleted_module.nim @@ -0,0 +1,26 @@ +discard """ + description: '''IC: deleting a still-imported module must be an error''' +""" + +#? metamorphic + +# Deleting a file moves no mtime, so nothing in an mtime-keyed build re-fires: +# `nim ic` relinked a stale binary while `nim c` reported `cannot open file`. +# The dependency scan is the only part of the pipeline that looks at import +# paths at all, so that is where the vanished module has to be noticed. + +#!FILE helper.nim +proc help*(): string = "helped" + +#!FILE main.nim +import helper +echo help() +#!STEP expect: helped + +#!DELETE helper.nim +#!STEP fails: cannot open file + +# putting it back recovers +#!FILE helper.nim +proc help*(): string = "back" +#!STEP expect: back diff --git a/tests/ic/tdestructor_fidelity.nim b/tests/ic/tdestructor_fidelity.nim new file mode 100644 index 0000000000..12340f0e54 --- /dev/null +++ b/tests/ic/tdestructor_fidelity.nim @@ -0,0 +1,68 @@ +discard """ + description: '''IC vs `nim c`: destructor injection and move analysis must agree''' +""" + +#? metamorphic + +# Two whole classes of IC miscompilation are invisible to any IC-vs-IC check, +# because IC was *consistently* wrong: warm == cold == not what `nim c` does. +# The oracle is what catches them. +# +# * `sfInjectDestructors` lives on the MODULE symbol, which the NIF loader +# rebuilds from scratch — so `genTopLevelStmt` skipped the destructor pass +# entirely and a module-level `block: let h = ...` never ran `=destroy`. +# * `nfFirstWrite`/`nfLastRead` sit on `nkSym` nodes, which serialize as bare +# NIF `SymUse` tokens with nowhere to put node flags — so the frontend's move +# analysis never reached the backend and EVERY first assignment to a +# destructor-bearing local became `=sink` over still-zeroed memory. + +#!FILE res.nim +var log*: seq[string] + +type R* = object + tag*: string + +proc `=destroy`*(r: R) = log.add "d(" & r.tag & ")" +proc `=copy`*(d: var R, s: R) = (log.add "c(" & s.tag & ")"; d.tag = s.tag) + +proc mk*(t: string): R = R(tag: t) +proc mkVia*(t: string): R = (result = R(tag: t)) +proc consume*(r: sink R): string = "u:" & r.tag + +#!FILE main.nim +import res + +# in a proc: worked before +proc inProc() = + let a = mk("proc") + discard a +inProc() + +# module top level: the pass was skipped wholesale +block: + let t = mk("toplevel") + discard t + +for i in 0 .. 1: + let l = mk("loop" & $i) + discard l + +# every `result` shape: each must construct in place, not `=sink` over zeroes +block: + let x = mk("direct") + let y = mkVia("via") + discard x + discard y + +# last read is a move, a re-read is a copy +proc moves(): string = + var m = mk("moved") + result = consume(m) +proc copies(): string = + var k = mk("kept") + result = consume(k) & "/" & k.tag +discard moves() +discard copies() + +echo log +#!STEP expect: @["d(proc)", "d(toplevel)", "d(loop0)", "d(loop1)", "d(via)", "d(direct)", "d(moved)", "c(kept)", "d(kept)", "d(kept)"] diff --git a/tests/ic/tdiscovered_import.nim b/tests/ic/tdiscovered_import.nim new file mode 100644 index 0000000000..e7dcd4d496 --- /dev/null +++ b/tests/ic/tdiscovered_import.nim @@ -0,0 +1,38 @@ +discard """ + description: '''IC: a macro-generated import stays in the graph across runs''' +""" + +#? metamorphic + +# The static scanner cannot see `parseStmt("import dyn")`. The discovery +# fixpoint recovers it — but only ran AFTER a failure, and the graph is +# re-derived statically on every run, so on a warm build the discovered module +# had no nifler/`nim m` rule at all: editing it changed nothing, forever. + +#!FILE dyn.nim +proc hidden*(): string = "first" + +#!FILE gen.nim +import std/macros + +macro generatedImport(): untyped = + parseStmt("import dyn") + +generatedImport() + +proc reveal*(): string = hidden() + +#!FILE main.nim +import gen +echo reveal() +#!STEP expect: first + +# the warm build must see this edit +#!FILE dyn.nim +proc hidden*(): string = "second" +#!STEP expect: second + +# and again, to prove it is not a one-shot recovery +#!FILE dyn.nim +proc hidden*(): string = "third" +#!STEP expect: third diff --git a/tests/ic/terror_recovery.nim b/tests/ic/terror_recovery.nim new file mode 100644 index 0000000000..c38b1f5034 --- /dev/null +++ b/tests/ic/terror_recovery.nim @@ -0,0 +1,32 @@ +discard """ + description: '''IC: a failed `nim m` must not poison the cache''' +""" + +#? metamorphic + +# A `nim m` that errored still wrote its `.s.bif` and cookie sidecars. nifmake +# then saw the rule satisfied (outputs newer than inputs) and the NEXT run +# reported success for a program that does not compile — linking a binary +# generated from error-bearing AST, or crashing codegen outright. Expressing +# this needs a step that is allowed to FAIL and a following step that recovers. + +#!FILE dep.nim +proc value*(): int = 41 + +#!FILE main.nim +import dep +echo value() + 1 +#!STEP expect: 42 + +# introduce a real error +#!FILE dep.nim +proc value*(): int = undefinedThing() + 1 +#!STEP fails: undeclared identifier: 'undefinedThing' + +# ... and again: the second run must NOT decide the rule is up to date. +#!STEP fails: undeclared identifier: 'undefinedThing' + +# fixing it must rebuild rather than serve the poisoned artifact +#!FILE dep.nim +proc value*(): int = 100 +#!STEP expect: 101 diff --git a/tests/ic/tglobal_dtors.nim b/tests/ic/tglobal_dtors.nim new file mode 100644 index 0000000000..ed9a14b5be --- /dev/null +++ b/tests/ic/tglobal_dtors.nim @@ -0,0 +1,49 @@ +discard """ + description: '''IC vs `nim c`: module-level globals must be destroyed at exit''' +""" + +#? metamorphic + +# `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 program +# exit. Under `nim ic` every module's `cg` is a separate process, so the main +# module's `cg` only ever saw its OWN entries and a module-level `var` with a +# `=destroy` in any imported module was simply never destroyed. +# +# The teardown ORDER is the other half: it must be the reverse of the init order +# (importers before their dependencies), which is what the oracle pins down here +# — three modules in a chain plus main, each with a global of its own. + +#!FILE gdlog.nim +type G* = object + tag*: string + +proc `=destroy`*(g: G) = echo "destroy ", g.tag +proc mk*(t: string): G = G(tag: t) + +#!FILE gda.nim +import gdlog +var ga* = mk("a") + +#!FILE gdb.nim +import gdlog, gda +var gb* = mk("b:" & ga.tag) + +#!FILE gdc.nim +import gdlog, gdb +var gcv* = mk("c:" & gb.tag) + +#!FILE main.nim +import gdlog, gda, gdb, gdc + +var gmain = mk("main") + +echo "body ", ga.tag, " ", gb.tag, " ", gcv.tag, " ", gmain.tag +#!STEP + +# touching a leaf module must not lose anyone's teardown +#!FILE gda.nim +import gdlog +var ga* = mk("a2") +#!STEP