From 1f504865ed376ef878face98a672dfd3db79d90b Mon Sep 17 00:00:00 2001 From: araq Date: Fri, 28 Aug 2026 20:03:14 +0200 Subject: [PATCH] IC: one definition of a backend-minted symbol's disambiguator MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The rule deciding which integer identifies a backend-minted symbol — content-derived `disamb` for a lifted hook (`setHookDisamb`), `itemId.item` otherwise — was written out at three sites: `mangleProcNameExt` and `ccgutils.makeUnique` for the C name, `ast2nif.toNifSymName` for the NIF name, each carrying its own copy of the ten-line rationale. They drifted, which is exactly cce17461d: `toNifSymName` lacked the hook exception, so the loader overwrote a content-derived value and two unrelated `=destroy` hooks collided on one C name (C accepted the mistyped call, C++ rejected it). `astdef.backendMintedDisamb` is now the single definition and all three call it. `globalName` still reads `disamb` directly — correct for a loaded symbol, and the round-trip invariant that makes it agree is now stated in the shared function instead of left implicit. Pure de-duplication, verified as such: all 219 generated `.c` files of a 219-module corpus and the linked binary are byte-identical to the previous commit. Plus 16/16 metamorphic IC tests, the 17-file `koch ic` suite, 13/13 differential edit checks against `nim c`, and the debug and release `bootic` fixed points. Note for the record: this started as an attempt to replace the per-process `_c` counter with a content hash. Instrumenting both mangling sites showed that branch is never taken — 0 of 166 backend-minted manglings on the corpus, 0 of 257 on the compiler, all going through the content-derived path — so rewriting the scheme would have shifted every backend-minted C name and forced an `icFormatVersion` bump for no demonstrable benefit. Co-Authored-By: Claude Opus 5 --- compiler/ast2nif.nim | 32 +++++++------------------------- compiler/astdef.nim | 32 ++++++++++++++++++++++++++++++++ compiler/ccgutils.nim | 16 ++++------------ compiler/mangleutils.nim | 20 +++++--------------- 4 files changed, 48 insertions(+), 52 deletions(-) diff --git a/compiler/ast2nif.nim b/compiler/ast2nif.nim index a3b9ac7fb6..5e1e482390 100644 --- a/compiler/ast2nif.nim +++ b/compiler/ast2nif.nim @@ -314,33 +314,15 @@ proc toNifSymName(w: var Writer; sym: PSym): string = # during a VM transform): re-home to the current module with the `@bk` # marker so each referencing module self-contains it. See transformBody. # - # Use `itemId.item` (the writer's dedup identity, see `emittedBackendSyms`) - # as the numeric name component, NOT `disamb`: closure `:env` syms in one - # module are minted from TWO id spaces — the backend lower stage's - # `tb.idgen` and sem's `vmTransfIdgen` (transf.transformBody) — whose - # `disambTable`s each start `:env` at the same low count, so a macro-lowered - # `:env` (e.g. `implementSendProcBody`) and a backend-lowered one - # (`peerTrimmerHeartbeat`) collide on `:env.2.@bk`. Two distinct syms - # then share a NIF name; the loader's name-keyed index/`c.syms` return the - # first for both, so one proc's `:env` gets the OTHER proc's env type - # (mismatched-pointer C, "has no member colonup_" at link). `itemId.item` is - # unique per `@bk` sym (both are emitted as defs, see writeSym), mirroring - # how `@bk` TYPES already key off `itemId.item` (nifTypeName). The loader - # copies this back into `disamb` (sn.count), so `globalName` round-trips. + # The numeric name component comes from `astdef.backendMintedDisamb` — the + # ONE definition of which integer identifies a backend-minted symbol, shared + # with the two C-name manglers (`mangleProcNameExt`, `ccgutils.makeUnique`) + # so the NIF name and the C name cannot disagree. `@bk` TYPES key off + # `itemId.item` the same way (see `nifTypeName`). The loader copies this back + # into `disamb` (sn.count), so `globalName` round-trips. result = sym.name.s result.add '.' - if (sym.disamb and HookDisambBit) != 0'i32: - # EXCEPTION, mirroring `mangleProcNameExt`: a lifted hook's `disamb` is - # CONTENT-derived (`setHookDisamb`), so it is the same in every process, - # whereas `itemId.item` is a per-process backend counter. Such a hook DOES - # cross process boundaries — the `lower` stage mints the env hooks of - # nested routines while `cg` mints those of the module's top level, and - # both end up in the same translation unit — so two unrelated hooks - # collided on `_c` and the merge stage kept one body for both - # (C accepted the mistyped call, C++ rejected it). - result.addInt sym.disamb - else: - result.addInt sym.itemId.item + result.addInt backendMintedDisamb(sym) result.add '.' result.add modname(w.currentModule, w.infos.config) result.add BackendLocalMarker diff --git a/compiler/astdef.nim b/compiler/astdef.nim index 6802d391bc..0bebdb44a3 100644 --- a/compiler/astdef.nim +++ b/compiler/astdef.nim @@ -1060,6 +1060,38 @@ const ## Both live here rather than in `modulegraphs` because `ast2nif` — which ## cannot import that module — names symbols by them. +proc backendMintedDisamb*(s: PSym): int32 {.inline.} = + ## The integer that identifies a BACKEND-MINTED symbol (`isBackendMinted`) in + ## every name derived from it: its NIF name (`ast2nif.toNifSymName`) and its C + ## name (`mangleutils.mangleProcNameExt`, `ccgutils.makeUnique`). + ## + ## Two cases, and the whole point of having ONE function is that all three + ## sites take the same one: + ## + ## * A lifted HOOK's `disamb` is CONTENT-derived (`modulegraphs.setHookDisamb`), + ## so it is identical in every process. Such a hook really does cross process + ## boundaries — `lower` mints the env hooks of nested routines while `cg` + ## mints those of the module's top level, and both land in the same + ## translation unit — and its C name is also baked into emit-everywhere RTTI + ## tables. `itemId.item` would differ per process, so two unrelated hooks + ## collided on one `_c` and the merge stage kept a single body for both + ## (C accepted the mistyped call, C++ rejected it). + ## * Otherwise `itemId.item` — the writer's dedup identity, unique per `@bk` + ## sym. `disamb` cannot serve here: a module's `:env` syms are minted from TWO + ## id spaces (the backend `lower` stage's idgen and sem's `vmTransfIdgen`) + ## whose `disambTable`s each start `:env` at the same low count, so a + ## macro-lowered and a backend-lowered `:env` collide on `:env.2.@bk`. + ## + ## The loader copies the name's numeric component back into `disamb`, so after a + ## round trip `disamb` equals this value and `ast2nif.globalName` — which always + ## reads `disamb` — agrees with the name the writer produced. + ## + ## This rule used to be written out at each of the three sites. They drifted: + ## `toNifSymName` lacked the hook exception, so a content-derived value was + ## overwritten by the loader and two backend hooks merged into one C function. + if (s.disamb and HookDisambBit) != 0'i32: s.disamb + else: s.itemId.item + type LogEntryKind* = enum HookEntry, ConverterEntry, MethodEntry, EnumToStrEntry, GenericInstEntry, diff --git a/compiler/ccgutils.nim b/compiler/ccgutils.nim index 430c16f172..aa5d171a45 100644 --- a/compiler/ccgutils.nim +++ b/compiler/ccgutils.nim @@ -113,20 +113,12 @@ proc encodeName*(name: string): string = proc makeUnique(m: BModule; s: PSym, name: string = ""): string = result = if name == "": s.name.s else: name # keep backend-minted ids out of the `_u` namespace; their item counter - # restarts at 0 and would collide with loaded symbols' ids + # restarts at 0 and would collide with loaded symbols' ids. Which integer + # identifies such a symbol is decided ONCE, in `astdef.backendMintedDisamb`, + # shared with `mangleProcNameExt` and `ast2nif.toNifSymName`. if s.itemId.isBackendMinted: result.add "_c" - if (s.disamb and HookDisambBit) != 0'i32: - # A backend-minted sym whose `disamb` is content-derived (setHookDisamb gave - # it HookDisambBit) — e.g. the `rttiDestroy` wrapper. Its `itemId.item` is a - # PER-PROCESS backend counter, so using it makes the C name diverge across - # the emit-everywhere processes: the type's RTTI table (emit-everywhere, - # merge-deduped) ends up referencing one process's `_c` while the - # wrapper is defined with another's -> undefined at link (`rttiDestroy_c23`). - # The content-derived disamb is stable across processes, so use it. - result.add $s.disamb - else: - result.add $s.itemId.item + result.add $backendMintedDisamb(s) else: result.add "_u" # Mirror `mangleProcNameExt`: use the per-(module,name) `disamb`, NOT diff --git a/compiler/mangleutils.nim b/compiler/mangleutils.nim index d9215135a8..62ef647adb 100644 --- a/compiler/mangleutils.nim +++ b/compiler/mangleutils.nim @@ -61,22 +61,12 @@ proc mangleProcNameExt*(graph: ModuleGraph, s: PSym): string = # starts with an EMPTY per-name disamb table, so its `disamb` restarts at 0 # and collides with same-named sem-time symbols loaded from NIFs (two # `=destroy` hooks both mangling to `_u2` → "conflicting types for ..." in - # the generated C). Most such symbols never cross a process boundary (nifc - # lifts, emits and compiles them in one run), so the per-module-unique - # item id is a safe and deterministic discriminator; the `_c` marker keeps - # the namespace disjoint from `_u`. + # the generated C). The `_c` marker keeps the namespace disjoint from + # `_u`; `backendMintedDisamb` (astdef) is the ONE definition of which + # integer identifies such a symbol, shared with `ccgutils.makeUnique` and + # `ast2nif.toNifSymName` so the C name and the NIF name cannot drift apart. result = "_c" - if (s.disamb and HookDisambBit) != 0'i32: - # EXCEPTION: a backend-minted sym whose `disamb` is content-derived - # (setHookDisamb gave it HookDisambBit) — e.g. the `rttiDestroy` wrapper — - # DOES cross process boundaries: its C name is baked into the type's RTTI - # table, which is emit-everywhere and merge-deduped, so one process's - # `_c` (a per-process backend counter) ends up referenced while the - # wrapper is defined with another's → undefined at link (`rttiDestroy_c23`). - # The content-derived disamb is stable across processes; use it. - result.addInt s.disamb - else: - result.addInt s.itemId.item + result.addInt backendMintedDisamb(s) else: result = "_u" # Use `disamb` rather than `itemId.item`: under incremental compilation a