diff --git a/compiler/ast2nif.nim b/compiler/ast2nif.nim index 421a412b4b..869bfed12a 100644 --- a/compiler/ast2nif.nim +++ b/compiler/ast2nif.nim @@ -3698,23 +3698,10 @@ proc populateInterfaceTablesFromIndex(c: var DecodeContext; module: FileIndex; # (moduleId can add to c.mods which would invalidate Table iterators) var indexTab = move c.mods[module].index - # Add all symbols to interf (exported interface) and interfHidden - # A BACKEND stage cannot read the hidden-only half, so it does not build it. - # `interfHidden` is reached exclusively through `modulegraphs.interfSelect`, - # which picks it only when `optImportHidden` is in the module's options — and - # that flag is set in exactly one place, `importer.importModuleAs`, i.e. - # during sem. `cg`/`emit` build their module symbols in `moduleFromNifFile` - # and never import anything, so the flag cannot be set and the table cannot be - # selected. The saving is the bulk of this proc: on a 68-module target the - # hidden branch stubs 2.0M symbols against the exported branch's 0.33M. - # - # Exported symbols are still added to BOTH tables. That costs little beside - # the above and leaves `interfHidden` a coherent view (a module with no hidden - # symbols) rather than an empty one, should anything ever consult it. - let conf = c.infos.config - let backendStage = conf.cmd == cmdNifC and - (conf.icBackendStage == "cg" or conf.icBackendStage == "emit") - + # Only the EXPORTED half; `buildHiddenInterface` below does the rest, on + # demand. Exported symbols go into both tables, which costs little and leaves + # `interfHidden` a coherent view of a module with no hidden symbols rather + # than an empty one. prof pIfaceModules for nifName, entry in indexTab: if entry.vis == Exported: @@ -3723,17 +3710,48 @@ proc populateInterfaceTablesFromIndex(c: var DecodeContext; module: FileIndex; if sym != nil: strTableAdd(interf, sym) strTableAdd(interfHidden, sym) - elif not backendStage and not nifName.startsWith("`t"): - prof pIfaceHidden - # do not load types, they are not part of an interface but an implementation detail! - #echo "LOADING SYM ", nifName, " ", entry.offset - let sym = loadSymFromIndexEntry(c, module, nifName, entry, thisModule) - if sym != nil: - strTableAdd(interfHidden, sym) # Move index table back c.mods[module].index = move indexTab +proc buildHiddenInterface*(c: var DecodeContext; suffix: string; + interfHidden: var TStrTable): bool {.discardable.} = + ## The hidden-only half of a loaded module's interface, materialised on + ## demand. Deferred because almost nothing reads it: `interfHidden` is reached + ## exclusively through `modulegraphs.interfSelect`, which picks it only when + ## `optImportHidden` is in the module's options, and that flag is set in + ## exactly one place — an `import x {.all.}`. Building it eagerly was 1.05s of + ## a cold Atlas build: 1.70M hidden stubs against 0.29M exported ones, made by + ## every `nim m` for every module it imports and read by none of them. + ## + ## Takes the module SUFFIX, not a FileIndex, and that is the whole trick. A + ## module has TWO FileIndexes: `registerNifSuffix` keys + ## `filenameToIndexTbl` by the suffix string and mints a `fikNifModule` entry, + ## while the graph indexes `g.ifaces` by the module's `fikSource` file. `c.mods` + ## is keyed by the former. Asking it with the latter misses every single time, + ## silently, and an `import x {.all.}` then reports "undeclared identifier" + ## for a symbol that is right there. + ## + ## Returns false when the artifact is not on disk yet — an import the build + ## has not produced. The caller must leave the request PENDING then: writing + ## it off on that first miss costs the module its hidden symbols for the rest + ## of the process. + let conf = c.infos.config + if not fileExists((getNimcacheDir(conf) / RelativeFile(suffix & ".s.bif")).string): + return false + let module = moduleId(c, suffix, {}) + if not c.mods.hasKey(module): return false + var indexTab = move c.mods[module].index + for nifName, entry in indexTab: + if entry.vis != Exported and not nifName.startsWith("`t"): + prof pIfaceHidden + # do not load types, they are not part of an interface but an implementation detail! + let sym = loadSymFromIndexEntry(c, module, nifName, entry, suffix) + if sym != nil: + strTableAdd(interfHidden, sym) + c.mods[module].index = move indexTab + result = true + proc moduleSymbolStubs*(c: var DecodeContext; module: FileIndex): seq[PSym] = ## Stubs for every non-type symbol serialized in `module`'s NIF index. The ## per-module backend uses this to emit the routines a module OWNS: procs are diff --git a/compiler/bnode.nim b/compiler/bnode.nim index 613ac1e8d0..918abeef3a 100644 --- a/compiler/bnode.nim +++ b/compiler/bnode.nim @@ -149,35 +149,20 @@ ## up front — a macro-generated `{.all.}` import cannot be seen syntactically, ## and guessing wrong loses symbols silently. ## -## That lazy conversion has been ATTEMPTED and does not work as a -## substitution. The shape is easy: `modulegraphs` already imports `ast2nif` -## (the dependency runs that way, so no callback hook is needed), every read of -## `interfHidden` goes through `interfSelect`, and its 4 call sites all have -## the graph and the module to hand. It builds and it keeps privacy — a plain -## `import` still rejects a private symbol under both `--ic:on` and -## `--ic:off` — but `import x {.all.}` then fails under `--ic:on` with -## "undeclared identifier", where it works today and works under `--ic:off`. -## The failure is silent and it is entangled with the driver's discovery loop: -## the module is not in `DecodeContext.mods` at the point the lookup asks, on -## a COLD cache and on a warm one. -## -## Three fixes were tried and none of them is it, recorded so nobody -## re-guesses them: -## * clearing `hiddenPending` only when the build SUCCEEDS, so an import -## whose `.s.bif` does not exist yet is retried rather than written off. -## Necessary — one early miss otherwise costs the module its hidden -## symbols for the whole process — but not sufficient. -## * loading the module inside the lazy builder (`moduleId` is idempotent) -## instead of assuming the caller already did. -## * building into a LOCAL `TStrTable` and assigning it back, in case the -## `var` alias into `g.ifaces` was being invalidated by the seq growing -## during the load. -## After all three, `ensureHiddenIface` still reports the module missing from -## `DecodeContext.mods` on every one of its 19 calls. Whatever populates -## `hiddenPending` and whatever populates `c.mods` are not agreeing about -## which module they mean, and that is where the next attempt should start — -## with `moduleId`'s FileIndex against `moduleFromNifFile`'s, instrumented at -## both ends, before any more of the mechanism is written. +## That conversion is DONE (`modulegraphs.ensureHiddenIface` + +## `ast2nif.buildHiddenInterface`), and the thing that made it hard is worth +## knowing: a module has TWO FileIndexes. `registerNifSuffix` keys +## `filenameToIndexTbl` by the NIF SUFFIX and mints a `fikNifModule` entry, +## while the graph indexes `g.ifaces` by the module's `fikSource` file. +## `DecodeContext.mods` is keyed by the former. Asking it with the latter +## misses every single time, silently, and `import x {.all.}` then reports +## "undeclared identifier" for a symbol that is right there. So the lazy +## builder takes a SUFFIX, not a FileIndex. Two further conditions are also +## load-bearing: clear the pending flag only when the build SUCCEEDS (an +## import whose `.s.bif` does not exist yet must be retried, not written off), +## and build into a LOCAL table before assigning it back (loading symbols can +## grow `g.ifaces`, which would leave a `var` alias into it dangling). +## `tests/ic/timporthidden.nim` is what says all of this still holds. ## * `-d:icBridgeOnly` builds the buffer but generates off the tree, which ## separates the ENCODER's cost from the READER's. Encoding is free — it does ## not show in wall time at all. diff --git a/compiler/deps.nim b/compiler/deps.nim index afc1c4a591..a8a590c635 100644 --- a/compiler/deps.nim +++ b/compiler/deps.nim @@ -522,6 +522,18 @@ proc parseImportPath(s: var Stream; t: var PackedToken): seq[string] = for r in parseImportPath(s, t): result.add op & r if t.kind == ParRi: t = next(s) # skip closing ')' + elif tag == "pragmax": + # `import x {.all.}` serialises as `(pragmax x (pragmas all))`. Without + # this it fell into the unknown-subtree skip below and the import was + # DROPPED from the static graph: the build only learned about it from the + # `.s.deps` sidecar a round later, after a round that failed with + # "requires precompiled NIF for import". Correct, but a wasted round and + # an alarming error line for an ordinary import. + t = next(s) # skip 'pragmax' tag + result = parseImportPath(s, t) # the path is the first child + while t.kind != ParRi and t.kind != EofToken: + discard parseImportPath(s, t) # the pragma list; consumed, not a path + if t.kind == ParRi: t = next(s) # skip closing ')' elif tag == "bracket": t = next(s) # skip 'bracket' tag while t.kind != ParRi and t.kind != EofToken: diff --git a/compiler/modulegraphs.nim b/compiler/modulegraphs.nim index c26a860099..a6f2cf04dd 100644 --- a/compiler/modulegraphs.nim +++ b/compiler/modulegraphs.nim @@ -36,6 +36,10 @@ type pureEnums*: seq[PSym] interf: TStrTable interfHidden: TStrTable + hiddenPending: bool ## `interfHidden` holds only the exported half so far; + ## `ensureHiddenIface` materialises the hidden-only + ## symbols on first use. See + ## `ast2nif.buildHiddenInterface`. uniqueName*: Rope Operators* = object @@ -257,6 +261,25 @@ proc toBase64a(s: cstring, len: int): string = result.add cb64[a shr 2] result.add cb64[(a and 3) shl 4] +proc ensureHiddenIface(g: ModuleGraph; pos: int) = + ## Materialise a loaded module's hidden-only interface the first time anything + ## asks for it. Every READ of `interfHidden` goes through `interfSelect`, so + ## guarding those sites is complete. + if g.ifaces[pos].hiddenPending: + when not defined(nimKochBootstrap): + # By SUFFIX: `c.mods` and `g.ifaces` use different FileIndexes for the + # same module (see `buildHiddenInterface`). Into a LOCAL table, because + # loading symbols can grow `g.ifaces` and a `var` alias into it would then + # point at the freed buffer. Cleared only on success, so an import whose + # `.s.bif` does not exist yet is retried rather than written off. + var tab = g.ifaces[pos].interfHidden + if buildHiddenInterface(ast.program, + cachedModuleSuffix(g.config, FileIndex pos), tab): + g.ifaces[pos].interfHidden = tab + g.ifaces[pos].hiddenPending = false + else: + g.ifaces[pos].hiddenPending = false + template interfSelect(iface: Iface, importHidden: bool): TStrTable = var ret = iface.interf.addr # without intermediate ptr, it creates a copy and compiler becomes 15x slower! if importHidden: ret = iface.interfHidden.addr @@ -292,6 +315,7 @@ proc initModuleIter*(mi: var ModuleIter; g: ModuleGraph; m: PSym; name: PIdent): assert m.kind == skModule mi.modIndex = m.position mi.importHidden = optImportHidden in m.options + if mi.importHidden: ensureHiddenIface(g, mi.modIndex) result = initIdentIter(mi.ti, g.ifaces[mi.modIndex].interfSelect(mi.importHidden), name) proc nextModuleIter*(mi: var ModuleIter; g: ModuleGraph): PSym = @@ -299,6 +323,7 @@ proc nextModuleIter*(mi: var ModuleIter; g: ModuleGraph): PSym = iterator allSyms*(g: ModuleGraph; m: PSym): PSym = let importHidden = optImportHidden in m.options + if importHidden: ensureHiddenIface(g, m.position) for s in g.ifaces[m.position].interfSelect(importHidden).data: if s != nil: yield s @@ -334,10 +359,12 @@ proc reexportedLocalSyms*(g: ModuleGraph; m: PSym): seq[ItemId] = proc someSym*(g: ModuleGraph; m: PSym; name: PIdent): PSym = let importHidden = optImportHidden in m.options + if importHidden: ensureHiddenIface(g, m.position) result = strTableGet(g.ifaces[m.position].interfSelect(importHidden), name) proc someSymAmb*(g: ModuleGraph; m: PSym; name: PIdent; amb: var bool): PSym = let importHidden = optImportHidden in m.options + if importHidden: ensureHiddenIface(g, m.position) var ti: TIdentIter = default(TIdentIter) result = initIdentIter(ti, g.ifaces[m.position].interfSelect(importHidden), name) if result != nil and nextIdentIter(ti, g.ifaces[m.position].interfSelect(importHidden)) != nil: @@ -1122,6 +1149,7 @@ when not defined(nimKochBootstrap): strTableAdd(interf, inner) g.ifaces[fIdx.int].interf = interf g.ifaces[fIdx.int].interfHidden = interfHidden + g.ifaces[fIdx.int].hiddenPending = true proc moduleFromNifFile*(g: ModuleGraph; fileIdx: FileIndex; flags: set[LoadFlag] = {}): PrecompiledModule = @@ -1167,6 +1195,8 @@ when not defined(nimKochBootstrap): result = loadNifModule(ast.program, fileIdx, g.ifaces[fileIdx.int].interf, g.ifaces[fileIdx.int].interfHidden, flags) + # The hidden-only half was not built; `ensureHiddenIface` will, if asked. + g.ifaces[fileIdx.int].hiddenPending = true result.module = m # Restore the module symbol's persisted flags (see ast2nif `(modflags)`); # `cgen.genTopLevelStmt` gates the destructor pass on `sfInjectDestructors`. diff --git a/tests/ic/mimporthidden.nim b/tests/ic/mimporthidden.nim new file mode 100644 index 0000000000..53f2539c03 --- /dev/null +++ b/tests/ic/mimporthidden.nim @@ -0,0 +1,4 @@ +proc pub*(x: int): int = x + 1 + +proc secret(): int = 7 # no `*` +proc hiddenToo(x: int): int = x diff --git a/tests/ic/timporthidden.nim b/tests/ic/timporthidden.nim new file mode 100644 index 0000000000..596cf35cbd --- /dev/null +++ b/tests/ic/timporthidden.nim @@ -0,0 +1,17 @@ +discard """ +output: '''42''' +""" + +# `import x {.all.}` makes x's PRIVATE symbols visible. Under IC that means the +# hidden half of a loaded module's interface has to be there — and it is now +# built on demand rather than at load time, because almost nothing ever reads it +# (1.70M hidden stubs against 0.29M exported ones on a cold Atlas build). +# +# The trap the first attempt fell into: a module has TWO FileIndexes. `c.mods` +# in the decode context is keyed by the one `registerNifSuffix` mints for the +# NIF suffix; `g.ifaces` is indexed by the module's source file. Asking one with +# the other misses silently, and this test is what says so. + +import mimporthidden {.all.} + +echo secret() + hiddenToo(35)