fixes #25693; continues the bugfix story

compiler/lookups.nim

@@ -459,6 +459,15 @@ proc openShadowScope*(c: PContext) =
                           symbols: initStrTable(),
                           depthLevel: c.scopeDepth)
 
+proc rememberShadowDefs*(c: PContext) =
+  ## bug #25693: a template/macro operand's local definitions are sem-checked in
+  ## a shadow scope that is then discarded. Record those definitions so that a
+  ## later re-emission (e.g. a captured `typed` fragment expanded more than once)
+  ## can be detected as a redefinition rather than silently miscompiled.
+  for s in c.currentScope.symbols:
+    if s.kind in {skVar, skLet, skForVar}:
+      c.shadowDiscardedDefs.incl s.id
+
 proc closeShadowScope*(c: PContext) =
   ## closes the shadow scope, but doesn't merge any of the symbols
   ## Does not check for unused symbols or missing forward decls since a macro

compiler/options.nim

@@ -262,6 +262,11 @@ type
     procParamTypeBackendAliases
       ## Keep the old proc type compatibility rules that ignore backend
       ## c type aliases.
+    injectedSymbolRedefinition
+      ## Allow a template to inject a symbol *definition* that is then emitted
+      ## more than once (e.g. a `typed` argument captured by a `{.dirty.}`
+      ## template and re-emitted). This is a redefinition and rejected by
+      ## default; enabling this restores the old, unsound behavior. See #25693.
 
   SymbolFilesOption* = enum
     disabledSf, writeOnlySf, readOnlySf, v2Sf, stressTest

compiler/sem.nim

@@ -247,6 +247,18 @@ proc newSymG*(kind: TSymKind, n: PNode, c: PContext): PSym =
     if result.kind notin {kind, skTemp}:
       localError(c.config, n.info, "cannot use symbol of kind '$1' as a '$2'" %
         [result.kind.toHumanStr, kind.toHumanStr])
+    # bug #25693: a local declared inside a template/macro operand (recorded in
+    # `shadowDiscardedDefs`) may be realized in a real scope at most once. A
+    # second realization - e.g. a `typed` argument captured by a `{.dirty.}`
+    # template and emitted by two `t()` calls - produces two definitions of the
+    # same symbol, which is a redefinition just like `var x = 0; var x = 1`.
+    if kind in {skVar, skLet, skForVar} and sfGenSym notin result.flags and
+        result.id in c.shadowDiscardedDefs:
+      if containsOrIncl(c.realizedDefs, result.id) and
+          injectedSymbolRedefinition notin c.config.legacyFeatures:
+        localError(c.config, n.info,
+          "redefinition of '$1'; it is injected by a template more than once" %
+          result.name.s)
     when false:
       if sfGenSym in result.flags and result.kind notin {skTemplate, skMacro, skParam}:
         # declarative context, so produce a fresh gensym:

compiler/semdata.nim

@@ -189,6 +189,13 @@ type
     inTypeofContext*: int
 
     semAsgnOpr*: proc (c: PContext; n: PNode; k: TNodeKind): PNode {.nimcall.}
+    shadowDiscardedDefs*: IntSet
+      # ids of local symbols that were declared inside a template/macro operand's
+      # shadow scope and then discarded; such a symbol may be realized in a real
+      # scope at most once. See bug #25693 and `rememberShadowDefs`.
+    realizedDefs*: IntSet
+      # ids from `shadowDiscardedDefs` that have already been realized as a
+      # definition; a second realization is a redefinition.
 
   TBorrowState* = enum
     bsNone, bsReturnNotMatch, bsNoDistinct, bsGeneric, bsNotSupported, bsMatch
@@ -343,6 +350,8 @@ proc newContext*(graph: ModuleGraph; module: PSym): PContext =
     userPragmas: initStrTable(),
     generics: @[],
     unknownIdents: initIntSet(),
+    shadowDiscardedDefs: initIntSet(),
+    realizedDefs: initIntSet(),
     cache: graph.cache,
     graph: graph,
     signatures: initStrTable(),

compiler/sigmatch.nim

@@ -2855,6 +2855,7 @@ proc matchesAux(c: PContext, n, nOrig: PNode, m: var TCandidate, marker: var Int
     if m.calleeSym != nil and m.calleeSym.kind notin {skTemplate, skMacro}:
       c.mergeShadowScope
     else:
+      c.rememberShadowDefs
       c.closeShadowScope
     m.state = csNoMatch
     m.firstMismatch.arg = a
@@ -2911,7 +2912,10 @@ proc matchesAux(c: PContext, n, nOrig: PNode, m: var TCandidate, marker: var Int
           setSon(m.call, formal.position + 1, container)
         else:
           incrIndexType(container.typ)
-        container.add n[a]
+        # bug #25693: like the scalar `tyUntyped` case in `paramTypesMatchAux`,
+        # a previous overload candidate may have sem-checked the operand in
+        # place; templates/macros expect the pristine AST, so use `nOrig`.
+        container.add nOrig[a]
     elif n[a].kind == nkExprEqExpr:
       # named param
       m.firstMismatch.kind = kUnknownNamedParam
@@ -3010,7 +3014,8 @@ proc matchesAux(c: PContext, n, nOrig: PNode, m: var TCandidate, marker: var Int
             setSon(m.call, formal.position + 1, container)
           else:
             incrIndexType(container.typ)
-          container.add n[a]
+          # bug #25693: see the leading isVarargsUntyped branch above.
+          container.add nOrig[a]
         else:
           m.baseTypeMatch = false
           m.typedescMatched = false
@@ -3062,6 +3067,7 @@ proc matchesAux(c: PContext, n, nOrig: PNode, m: var TCandidate, marker: var Int
     if m.state == csMatch and not (m.calleeSym != nil and m.calleeSym.kind in {skTemplate, skMacro}):
       c.mergeShadowScope
     else:
+      c.rememberShadowDefs
       c.closeShadowScope
 
     inc a

tests/template/toverload_over_untyped.nim

@@ -1,32 +1,55 @@
 discard """
-  output: "ok"
+  output: '''ok
+ok'''
 """
 
 # bug #25693
 
-template g(b: untyped) {.dirty.} =
-  template t: untyped = b
-
 proc d() = discard @[0]
-proc g(_: int) = discard
 
 proc f(a: var seq[int], _: string) =
   let p = @[0]
   d()
   a = p
 
-let q = "a"
-g:
-  var a: seq[int]
-  try:
-    f(a, q & "1")
-  except CatchableError:
-    discard
-  try:
-    f(a, q & "1")
-  except CatchableError:
-    discard
-block: t()
-block: t()
-echo "ok"
+block: # scalar `untyped` parameter
+  template g(b: untyped) {.dirty.} =
+    template t: untyped = b
 
+  proc g(_: int) = discard
+
+  let q = "a"
+  g:
+    var a: seq[int]
+    try:
+      f(a, q & "1")
+    except CatchableError:
+      discard
+    try:
+      f(a, q & "1")
+    except CatchableError:
+      discard
+  block: t()
+  block: t()
+  echo "ok"
+
+block: # `varargs[untyped]` parameter takes the same pristine-AST path
+  template g(b: varargs[untyped]) {.dirty.} =
+    template t: untyped = b
+
+  proc g(_: int) = discard
+
+  let q = "a"
+  g:
+    var a: seq[int]
+    try:
+      f(a, q & "1")
+    except CatchableError:
+      discard
+    try:
+      f(a, q & "1")
+    except CatchableError:
+      discard
+  block: t()
+  block: t()
+  echo "ok"