IC: bugfixes (#25914)

compiler/ast2nif.nim

@@ -25,6 +25,15 @@ import "../dist/nimony/src/models" / nifindex_tags
 import typekeys
 import ic / [enum2nif]
 
+const SysModuleSuffix* = "@sys"
+  ## Reserved module-suffix sentinel for module-less magic singleton types — the
+  ## `nil` type is created via `newSysType` with the graph idgen, whose `module`
+  ## can be `-1` (e.g. during VM const-eval before a real module is current), so
+  ## its `uniqueId.module` is unresolvable. Such a type has no fields and an
+  ## identity that is fully captured by its kind, so we serialize it with this
+  ## sentinel and reconstruct it on load (see `createTypeStub`) without ever
+  ## touching a `.nif` file. A real `moduleSuffix` never starts with '@'.
+
 proc typeToNifSym(typ: PType; config: ConfigRef): string =
   # NOTE: uniqueId is the serialization identity and is unique per instance —
   # `exactReplica` keeps only itemId shared with its original (see ast.nim)
@@ -34,7 +43,10 @@ proc typeToNifSym(typ: PType; config: ConfigRef): string =
   result.add '.'
   result.addInt typ.uniqueId.item
   result.add '.'
-  result.add modname(typ.uniqueId.module, config)
+  if typ.uniqueId.module < 0:
+    result.add SysModuleSuffix
+  else:
+    result.add modname(typ.uniqueId.module, config)
 
 proc icNifTypeName*(typ: PType; config: ConfigRef): string =
   ## The serialized NIF name of a type, recorded next to RTTI data
@@ -1527,6 +1539,19 @@ proc loadNode(c: var DecodeContext; n: var Cursor; thisModule: string;
 proc loadSymFromCursor(c: var DecodeContext; s: PSym; n: var Cursor; thisModule: string;
                        localSyms: var Table[string, PSym])
 
+proc reconstructSysType(c: var DecodeContext; name: string; k: int; itemVal: int32): PType =
+  ## Rebuild a module-less magic singleton (see `SysModuleSuffix`) from its kind
+  ## alone — it has no fields and no `.nif` to load. Cached in `c.types` so all
+  ## references in this decode context share one instance.
+  result = c.types.getOrDefault(name)[0]
+  if result == nil:
+    let id = itemId(-1'i32, itemVal)
+    result = PType(itemId: id, uniqueId: id, kind: TTypeKind(k), state: Complete)
+    if TTypeKind(k) == tyNil:
+      result.sizeImpl = c.infos.config.target.ptrSize
+      result.alignImpl = int16 c.infos.config.target.ptrSize
+    c.types[name] = (result, NifIndexEntry())
+
 proc tryCreateTypeStub(c: var DecodeContext; t: SymId): PType =
   ## Like `createTypeStub` but returns nil instead of raising when the type has
   ## no offset in its module index (used by the best-effort `(offer …)` loader).
@@ -1546,6 +1571,8 @@ proc tryCreateTypeStub(c: var DecodeContext; t: SymId): PType =
       inc i
     if i < name.len and name[i] == '.': inc i
     let suffix = name.substr(i)
+    if suffix == SysModuleSuffix:
+      return reconstructSysType(c, name, k, itemVal)
     let id = itemId(moduleId(c, suffix).int32, itemVal)
     let ii = addr c.mods[id.module.FileIndex].index
     let offs = ii[].getOrDefault(name)
@@ -1571,6 +1598,8 @@ proc createTypeStub(c: var DecodeContext; t: SymId): PType =
       inc i
     if i < name.len and name[i] == '.': inc i
     let suffix = name.substr(i)
+    if suffix == SysModuleSuffix:
+      return reconstructSysType(c, name, k, itemVal)
     let id = itemId(moduleId(c, suffix).int32, itemVal)
     let ii = addr c.mods[id.module.FileIndex].index
     let offs = ii[].getOrDefault(name)

compiler/msgs.nim

@@ -125,12 +125,25 @@ proc fileInfoIdx*(conf: ConfigRef; filename: AbsoluteFile): FileIndex =
   var dummy: bool = false
   result = fileInfoIdx(conf, filename, dummy)
 
+proc expandOrPseudo(filename: string): AbsoluteFile =
+  # `expandFilename` raises OSError when the path does not exist on disk. That is
+  # fine for a real source path, but a macro can legitimately set a node's
+  # line-info file to a name that has no file behind it — e.g. the `???` sentinel
+  # produced by `toFilename` for a NIF-loaded node whose `fileIndex` is unknown
+  # (FileIndex(-1)). Falling back to the raw name lets the `AbsoluteFile` overload
+  # register it as a pseudo-path (like `command line`/`stdin`) instead of crashing
+  # the whole `nim m` child with an unhandled OSError.
+  try:
+    result = AbsoluteFile expandFilename(filename)
+  except OSError:
+    result = AbsoluteFile filename
+
 proc fileInfoIdx*(conf: ConfigRef; filename: RelativeFile; isKnownFile: var bool): FileIndex =
-  fileInfoIdx(conf, AbsoluteFile expandFilename(filename.string), isKnownFile)
+  fileInfoIdx(conf, expandOrPseudo(filename.string), isKnownFile)
 
 proc fileInfoIdx*(conf: ConfigRef; filename: RelativeFile): FileIndex =
   var dummy: bool = false
-  fileInfoIdx(conf, AbsoluteFile expandFilename(filename.string), dummy)
+  fileInfoIdx(conf, expandOrPseudo(filename.string), dummy)
 
 proc registerNifSuffix*(conf: ConfigRef; suffix: string; isKnownFile: var bool): FileIndex =
   result = conf.m.filenameToIndexTbl.getOrDefault(suffix, InvalidFileIdx)

compiler/options.nim

@@ -29,7 +29,7 @@ const
 
   nimEnableCovariance* = defined(nimEnableCovariance)
 
-  icFormatVersion* = "5"
+  icFormatVersion* = "6"
     ## 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`
@@ -49,6 +49,11 @@ const
     ## cdef directives with an always-present extern declaration, so the
     ## per-module merge stage can assign them a single owner; old `.c.nif`
     ## artifacts lack the wrappers.
+    ## v6: `signatureHash`/`hashType` of a builtin type class (`object`, `tuple`,
+    ## `proc`, ...) no longer mixes in the placeholder son's process-local type
+    ## id, so its hash is stable across the NIF boundary (was breaking
+    ## nim-serialization's auto-serialization lookup under IC). The sem-NIF
+    ## macrocache entries and baked generic-instance bodies hold the old hashes.
 
 type                          # please make sure we have under 32 options
                               # (improves code efficiency a lot!)

compiler/semdata.nim

@@ -396,16 +396,21 @@ proc addConverter*(c: PContext, conv: PSym) =
   assert conv != nil
   if inclSym(c.converters, conv):
     add(c.graph.ifaces[c.module.position].converters, conv)
+    # Record for IC: the loader rebuilds Iface.converters from the NIF's
+    # (repconverter ...) entries (moduleFromNifFile). This must capture not only
+    # converters DEFINED in this module (addConverterDef) but also ones IMPORTED
+    # from another module here (importer.addUnnamedIt re-adds a re-exported
+    # module's converters via this proc). Otherwise a loaded module's
+    # re-exported converters were invisible to importers and implicit
+    # conversions silently stopped matching at a consumer that reaches the
+    # converter only through this module's re-export chain (e.g. faststreams'
+    # `InputStreamHandle -> InputStream` via ssz_serialization, breaking
+    # `SSZ.decode`/`encode`). `inclSym` guards against duplicate log entries.
+    c.graph.opsLog.add LogEntry(kind: ConverterEntry, module: c.module.position,
+                                key: "", sym: conv)
 
 proc addConverterDef*(c: PContext, conv: PSym) =
   addConverter(c, conv)
-  # record the definition for IC: the loader rebuilds Iface.converters from
-  # the NIF's (repconverter ...) entries (moduleFromNifFile); without the log
-  # entry a loaded module's converters were invisible to importers and
-  # implicit conversions silently stopped matching (e.g. faststreams'
-  # InputStreamHandle -> InputStream at toml_serialization call sites)
-  c.graph.opsLog.add LogEntry(kind: ConverterEntry, module: c.module.position,
-                              key: "", sym: conv)
 
 proc addPureEnum*(c: PContext, e: PSym) =
   assert e != nil

compiler/sighashes.nim

@@ -335,6 +335,21 @@ proc hashType(c: var MD5Context, t: PType; flags: set[ConsiderFlag]; conf: Confi
     c &= char(t.kind)
     c.hashType(t.indexType, flags-{CoIgnoreRange}+{CoIgnoreRangeInArray}, conf)
     c.hashType(t.elementType, flags-{CoIgnoreRange}, conf)
+  of tyBuiltInTypeClass:
+    # A builtin type class (`object`, `tuple`, `proc`, `ref`, `seq`, ...) is
+    # identified solely by the *kind* of its single placeholder son plus a few
+    # flags/callConv (see `sameType`). That son is a fresh, field-less, sym-less
+    # type, so the generic `else` below would recurse into it and hash its
+    # process-local `t.id` — unstable across the NIF boundary. nim-serialization
+    # keys auto-serialization on `signatureHash(object)`/`tuple`/... and missed
+    # under IC because the registering and consuming modules minted different
+    # placeholder ids. Hash the class identity that `sameType` actually compares.
+    c &= char(t.kind)
+    let elem = t.elementType
+    c &= char(elem.kind)
+    for f in eqTypeFlags * elem.flags: c &= char(ord(f))
+    if elem.kind == tyProc and tfExplicitCallConv in elem.flags:
+      c &= char(elem.callConv)
   else:
     c &= char(t.kind)
     for a in t.kids: c.hashType(a, flags, conf)

koch.nim

@@ -617,7 +617,8 @@ proc runIcTestFile(inp: string) =
 # on a sibling helper (`timp` -> `myimp`, `tcompiletimeglobal` -> `mctglobal`),
 # which exercises the NIF import/load path the single-file tests do not.
 const icSuite = ["thallo", "tconverter", "timp", "tmiscs", "tparseutils",
-                 "tcompiletimeglobal", "tsighashstable", "tpureenum", "tgenericoffer"]
+                 "tcompiletimeglobal", "tsighashstable", "tpureenum", "tgenericoffer",
+                 "tconverterreexport"]
 
 proc icTest(args: string) =
   temp("")

tests/ic/mconvbase.nim

@@ -0,0 +1,16 @@
+# Helper for tconverterreexport.nim: defines a converter that a consumer reaches
+# only through a re-export chain (mconvbase <- mconvmid <- test). Models
+# faststreams' `implicitDeref: InputStreamHandle -> InputStream`.
+type
+  Handle* = object
+    x*: int
+  Real* = object
+    x*: int
+
+converter toReal*(h: Handle): Real = Real(x: h.x)
+
+type Reader* = object
+  v: int
+
+func init*(T: type Reader, r: Real): Reader = Reader(v: r.x + 100)
+func readVal*(r: Reader): int = r.v

tests/ic/mconvmid.nim

@@ -0,0 +1,13 @@
+# Helper for tconverterreexport.nim: re-exports mconvbase and provides a
+# `mixin`-using template whose expansion in the consumer relies on the
+# re-exported `toReal` converter (Handle -> Real) for `init`. Mirrors
+# ssz_serialization re-exporting serialization/faststreams and its `decode`
+# template needing the implicit stream conversion at `init(Reader, stream)`.
+import mconvbase
+export mconvbase
+
+template decode*(): auto =
+  mixin init, readVal
+  var h = Handle(x: 5)
+  var r = init(Reader, h)   # requires the re-exported converter toReal
+  readVal(r)

tests/ic/msighashstable.nim

@@ -36,13 +36,33 @@ func getAuto*(F: type DefaultFlavor, T: distinct type): bool {.compileTime.} =
   let table = F.getTable()
   table.hasKey(sig)
 
+func tcOrMember*(F: type DefaultFlavor, TC: distinct type, TM: distinct type): bool {.compileTime.} =
+  ## Is `TM` registered, or its parent type class `TC`? Models
+  ## `typeClassOrMemberAutoSerialize` — used to auto-serialize any `object`/`tuple`.
+  if F.getAuto(TM): return true
+  if F.getAuto(TC): return true
+  false
+
 template autoCheck*(F: distinct type, T: distinct type, body) =
   when not F.getAuto(T):
     {.error: "auto serialization not enabled for `" & typetraits.name(T) & "`".}
   else:
     body
 
+template autoCheckTC*(F: distinct type, TC: distinct type, M: distinct type, body) =
+  when not F.tcOrMember(TC, M):
+    {.error: "auto serialization not enabled for `" & typetraits.name(M) &
+      "` of typeclass `" & typetraits.name(TC) & "`".}
+  else:
+    body
+
 static:
   setAuto(DefaultFlavor, string)
   setAuto(DefaultFlavor, SomeInteger)
   setAuto(DefaultFlavor, seq)
+  # Builtin *type classes*: `object`/`tuple` are `tyBuiltInTypeClass`, whose
+  # `signatureHash` must not mix in the placeholder son's process-local type id
+  # or the lookup misses across the NIF boundary (the real nim-serialization bug
+  # surfaced by Nimbus: `MultiAddress`/`RequestId` "of typeclass `object`").
+  setAuto(DefaultFlavor, object)
+  setAuto(DefaultFlavor, tuple)

tests/ic/tconverterreexport.nim

@@ -0,0 +1,18 @@
+discard """
+output: '''105'''
+"""
+
+# Regression test: a converter imported into a module and re-exported from it
+# (mconvbase's `toReal` re-exported by mconvmid) must survive NIF serialization
+# in the re-exporting module's interface, so a consumer that reaches the
+# converter only through that re-export chain can still apply it implicitly.
+#
+# Before the fix only converters DEFINED in a module were logged for IC
+# (`addConverterDef`); converters merely imported/re-exported (`addConverter`
+# from `importer.addUnnamedIt`) were not, so under `nim ic` the re-exported
+# converter was invisible to importers and `init(Reader, Handle)` failed with a
+# type mismatch (the real-world symptom: faststreams `InputStreamHandle ->
+# InputStream` dropped, breaking `SSZ.decode`/`encode` in Nimbus).
+
+import mconvmid
+echo decode()

tests/ic/tsighashstable.nim

@@ -1,7 +1,9 @@
 discard """
 output: '''ok string
 ok int
-ok seq'''
+ok seq
+ok object
+ok tuple'''
 """
 
 # Regression test: `signatureHash(T)` must be stable across the NIF boundary so
@@ -28,6 +30,24 @@ proc writeSeq[T](F: type DefaultFlavor, v: seq[T]) =
   autoCheck(F, seq):
     echo "ok seq"
 
+type
+  Msg = object
+    a: int
+    b: string
+
+# `object`/`tuple` are `tyBuiltInTypeClass`: a concrete type is auto-serialized
+# by looking up its type class. The hash of the bare class keyword must match
+# between msighashstable's registration and this lookup.
+proc writeObj[T: object](F: type DefaultFlavor, v: T) =
+  autoCheckTC(F, object, typeof(v)):
+    echo "ok object"
+
+proc writeTup[T: tuple](F: type DefaultFlavor, v: T) =
+  autoCheckTC(F, tuple, typeof(v)):
+    echo "ok tuple"
+
 writeStr(DefaultFlavor, "hi")
 writeInt(DefaultFlavor, 42)
 writeSeq(DefaultFlavor, @[1, 2, 3])
+writeObj(DefaultFlavor, Msg(a: 1, b: "x"))
+writeTup(DefaultFlavor, (1, "x"))