IC: one definition of a backend-minted symbol's disambiguator

The rule deciding which integer identifies a backend-minted symbol —
content-derived `disamb` for a lifted hook (`setHookDisamb`), `itemId.item`
otherwise — was written out at three sites: `mangleProcNameExt` and
`ccgutils.makeUnique` for the C name, `ast2nif.toNifSymName` for the NIF name,
each carrying its own copy of the ten-line rationale.

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

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

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

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
araq
2026-08-28 20:03:14 +02:00
parent 0c0cc1e496
commit 1f504865ed
4 changed files with 48 additions and 52 deletions

View File

@@ -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.<mod>@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<item>` 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

View File

@@ -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<item>` 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.<mod>@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,

View File

@@ -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<item>` 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

View File

@@ -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<disamb>`.
# the generated C). The `_c` marker keeps the namespace disjoint from
# `_u<disamb>`; `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<item>` (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