add switch, warning, and `bind` support for new generic injection behavior (#23102)

refs #23091, especially post merge comments

Unsure if `experimental` and `bind` are the perfect constructs to use
but they seem to get the job done here. Symbol nodes do not get marked
`nfOpenSym` if the `bind` statement is used for their symbol, and
`nfOpenSym` nodes do not get replaced by new local symbols if the
experimental switch is not enabled in the local context (meaning it also
works with `push experimental`). However this incurs a warning as the
fact that the node is marked `nfOpenSym` means we did not `bind` it, so
we might want to do that or turn on the experimental switch if we didn't
intend to bind it.

The experimental switch name is arbitrary and could be changed.

---------

Co-authored-by: Andreas Rumpf <rumpf_a@web.de>

8 files changed, 176 insertions, 6 deletions

diff --git a/changelog.md b/changelog.md
index db85e25a1..1bedb1cd6 100644
--- a/changelog.md
+++ b/changelog.md
@@ -57,6 +57,41 @@ slots when enlarging a sequence.
     let (a, (b, c)): (byte, (float, cstring)) = (1, (2, "abc"))
     ```
 
+- An experimental option `genericsOpenSym` has been added to allow captured
+  symbols in generic routine bodies to be replaced by symbols injected locally
+  by templates/macros at instantiation time. `bind` may be used to keep the
+  captured symbols over the injected ones regardless of enabling the option.
+  
+  Since this change may affect runtime behavior, the experimental switch
+  `genericsOpenSym` needs to be enabled, and a warning is given in the case
+  where an injected symbol would replace a captured symbol not bound by `bind`
+  and the experimental switch isn't enabled.
+
+  ```nim
+  const value = "captured"
+  template foo(x: int, body: untyped) =
+    let value {.inject.} = "injected"
+    body
+
+  proc old[T](): string =
+    foo(123):
+      return value # warning: a new `value` has been injected, use `bind` or turn on `experimental:genericsOpenSym`
+  echo old[int]() # "captured"
+
+  {.experimental: "genericsOpenSym".}
+
+  proc bar[T](): string =
+    foo(123):
+      return value
+  assert bar[int]() == "injected" # previously it would be "captured"
+
+  proc baz[T](): string =
+    bind value
+    foo(123):
+      return value
+  assert baz[int]() == "captured"
+  ```
+
 ## Compiler changes
 
 - `--nimcache` using a relative path as the argument in a config file is now relative to the config file instead of the current directory.
diff --git a/compiler/lineinfos.nim b/compiler/lineinfos.nim
index d21825be7..dc0b6c360 100644
--- a/compiler/lineinfos.nim
+++ b/compiler/lineinfos.nim
@@ -92,6 +92,7 @@ type
     warnStmtListLambda = "StmtListLambda",
     warnBareExcept = "BareExcept",
     warnImplicitDefaultValue = "ImplicitDefaultValue",
+    warnGenericsIgnoredInjection = "GenericsIgnoredInjection",
     warnStdPrefix = "StdPrefix"
     warnUser = "User",
     # hints
@@ -196,6 +197,7 @@ const
     warnStmtListLambda: "statement list expression assumed to be anonymous proc; this is deprecated, use `do (): ...` or `proc () = ...` instead",
     warnBareExcept: "$1",
     warnImplicitDefaultValue: "$1",
+    warnGenericsIgnoredInjection: "$1",
     warnStdPrefix: "$1 needs the 'std' prefix",
     warnUser: "$1",
     hintSuccess: "operation successful: $#",
diff --git a/compiler/options.nim b/compiler/options.nim
index 45ed8c23e..1ef096601 100644
--- a/compiler/options.nim
+++ b/compiler/options.nim
@@ -227,6 +227,7 @@ type
     strictDefs,
     strictCaseObjects,
     inferGenericTypes,
+    genericsOpenSym,
     vtables
 
   LegacyFeature* = enum
diff --git a/compiler/semexprs.nim b/compiler/semexprs.nim
index 93574e217..b46965875 100644
--- a/compiler/semexprs.nim
+++ b/compiler/semexprs.nim
@@ -3076,8 +3076,18 @@ proc semExpr(c: PContext, n: PNode, flags: TExprFlags = {}, expectedType: PType
         var o = s2.owner
         while o != nil:
           if o == c.p.owner:
-            result = semExpr(c, id, flags, expectedType)
-            return
+            if genericsOpenSym in c.features:
+              result = semExpr(c, id, flags, expectedType)
+              return
+            else:
+              message(c.config, n.info, warnGenericsIgnoredInjection,
+                "a new symbol '" & s.name.s & "' has been injected during " &
+                "instantiation of " & c.p.owner.name.s & ", " &
+                "however " & getSymRepr(c.config, s) & " captured at " &
+                "the proc declaration will be used instead; " &
+                "either enable --experimental:genericsOpenSym to use the " &
+                "injected symbol or `bind` this captured symbol explicitly")
+              break
           o = o.owner
     # because of the changed symbol binding, this does not mean that we
     # don't have to check the symbol for semantics here again!
diff --git a/compiler/semgnrc.nim b/compiler/semgnrc.nim
index a96bc484b..03af12df2 100644
--- a/compiler/semgnrc.nim
+++ b/compiler/semgnrc.nim
@@ -56,6 +56,9 @@ template isMixedIn(sym): bool =
                                s.magic == mNone and
                                s.kind in OverloadableSyms)
 
+template canOpenSym(s): bool =
+  {withinMixin, withinConcept} * flags == {withinMixin} and s.id notin ctx.toBind
+
 proc semGenericStmtSymbol(c: PContext, n: PNode, s: PSym,
                           ctx: var GenericCtx; flags: TSemGenericFlags,
                           fromDotExpr=false): PNode =
@@ -69,7 +72,7 @@ proc semGenericStmtSymbol(c: PContext, n: PNode, s: PSym,
         result.transitionSonsKind(nkClosedSymChoice)
     else:
       result = symChoice(c, n, s, scOpen)
-      if {withinMixin, withinConcept} * flags == {withinMixin} and result.kind == nkSym:
+      if result.kind == nkSym and canOpenSym(result.sym):
         result.flags.incl nfOpenSym
         result.typ = nil
   case s.kind
@@ -99,7 +102,7 @@ proc semGenericStmtSymbol(c: PContext, n: PNode, s: PSym,
         result = n
     else:
       result = newSymNodeTypeDesc(s, c.idgen, n.info)
-      if {withinMixin, withinConcept} * flags == {withinMixin}:
+      if canOpenSym(result.sym):
         result.flags.incl nfOpenSym
         result.typ = nil
     onUse(n.info, s)
@@ -110,7 +113,7 @@ proc semGenericStmtSymbol(c: PContext, n: PNode, s: PSym,
     if (s.typ != nil) and
        (s.typ.flags * {tfGenericTypeParam, tfImplicitTypeParam} == {}):
       result = newSymNodeTypeDesc(s, c.idgen, n.info)
-      if {withinMixin, withinConcept} * flags == {withinMixin}:
+      if canOpenSym(result.sym):
         result.flags.incl nfOpenSym
         result.typ = nil
     else:
@@ -118,7 +121,7 @@ proc semGenericStmtSymbol(c: PContext, n: PNode, s: PSym,
     onUse(n.info, s)
   else:
     result = newSymNode(s, n.info)
-    if {withinMixin, withinConcept} * flags == {withinMixin}:
+    if canOpenSym(result.sym):
       result.flags.incl nfOpenSym
       result.typ = nil
     onUse(n.info, s)
diff --git a/doc/manual_experimental.md b/doc/manual_experimental.md
index fc5ff1959..765a69a0f 100644
--- a/doc/manual_experimental.md
+++ b/doc/manual_experimental.md
@@ -2521,6 +2521,46 @@ NimFunctor()(1)
 Notice we use the overload of `()` to have the same semantics in Nim, but on the `importcpp` we import the functor as a function. 
 This allows to easy interop with functions that accepts for example a `const` operator in its signature. 
 
+
+Injected symbols in generic procs
+=================================
+
+With the experimental option `genericsOpenSym`, captured symbols in generic
+routine bodies may be replaced by symbols injected locally by templates/macros
+at instantiation time. `bind` may be used to keep the captured symbols over
+the injected ones regardless of enabling the option.
+  
+Since this change may affect runtime behavior, the experimental switch
+`genericsOpenSym` needs to be enabled, and a warning is given in the case
+where an injected symbol would replace a captured symbol not bound by `bind`
+and the experimental switch isn't enabled.
+
+```nim
+const value = "captured"
+template foo(x: int, body: untyped) =
+  let value {.inject.} = "injected"
+  body
+
+proc old[T](): string =
+  foo(123):
+    return value # warning: a new `value` has been injected, use `bind` or turn on `experimental:genericsOpenSym`
+echo old[int]() # "captured"
+
+{.experimental: "genericsOpenSym".}
+
+proc bar[T](): string =
+  foo(123):
+    return value
+assert bar[int]() == "injected" # previously it would be "captured"
+
+proc baz[T](): string =
+  bind value
+  foo(123):
+    return value
+assert baz[int]() == "captured"
+```
+
+
 VTable for methods
 ==================
 
diff --git a/tests/generics/tmacroinjectedsym.nim b/tests/generics/tmacroinjectedsym.nim
index a98c1edb1..d36d34cdd 100644
--- a/tests/generics/tmacroinjectedsym.nim
+++ b/tests/generics/tmacroinjectedsym.nim
@@ -1,3 +1,5 @@
+{.experimental: "genericsOpenSym".}
+
 block: # issue #22605, normal call syntax
   const error = "bad"
 
@@ -16,6 +18,15 @@ block: # issue #22605, normal call syntax
 
   doAssert g(int) == "good"
 
+  proc g2(T: type): string =
+    bind error # use the bad version on purpose
+    let x = valueOr 123:
+      return $error
+
+    "ok"
+
+  doAssert g2(int) == "bad"
+
 block: # issue #22605, method call syntax
   const error = "bad"
 
@@ -34,6 +45,15 @@ block: # issue #22605, method call syntax
 
   doAssert g(int) == "good"
 
+  proc g2(T: type): string =
+    bind error # use the bad version on purpose
+    let x = 123.valueOr:
+      return $error
+
+    "ok"
+
+  doAssert g2(int) == "bad"
+
 block: # issue #22605, original complex example
   type Xxx = enum
     error
@@ -84,3 +104,12 @@ block: # issue #22605, original complex example
     "ok"
 
   doAssert g(int) == "f"
+
+  proc g2(T: type): string =
+    bind error # use the bad version on purpose
+    let x = f().valueOr:
+      return $error
+
+    "ok"
+
+  doAssert g2(int) == "error"
diff --git a/tests/generics/tmacroinjectedsymwarning.nim b/tests/generics/tmacroinjectedsymwarning.nim
new file mode 100644
index 000000000..7adb759e8
--- /dev/null
+++ b/tests/generics/tmacroinjectedsymwarning.nim
@@ -0,0 +1,50 @@
+type Xxx = enum
+  error
+  value
+
+type
+  Result[T, E] = object
+    when T is void:
+      when E is void:
+        oResultPrivate*: bool
+      else:
+        case oResultPrivate*: bool
+        of false:
+          eResultPrivate*: E
+        of true:
+          discard
+    else:
+      when E is void:
+        case oResultPrivate*: bool
+        of false:
+          discard
+        of true:
+          vResultPrivate*: T
+      else:
+        case oResultPrivate*: bool
+        of false:
+          eResultPrivate*: E
+        of true:
+          vResultPrivate*: T
+
+template valueOr[T: not void, E](self: Result[T, E], def: untyped): untyped =
+  let s = (self) # TODO avoid copy
+  case s.oResultPrivate
+  of true:
+    s.vResultPrivate
+  of false:
+    when E isnot void:
+      template error: untyped {.used, inject.} = s.eResultPrivate
+    def
+
+proc f(): Result[int, cstring] =
+  Result[int, cstring](oResultPrivate: false, eResultPrivate: "f")
+
+proc g(T: type): string =
+  let x = f().valueOr:
+    return $error #[tt.Warning
+            ^ a new symbol 'error' has been injected during instantiation of g, however 'error' [enumField declared in tmacroinjectedsymwarning.nim(2, 3)] captured at the proc declaration will be used instead; either enable --experimental:genericsOpenSym to use the injected symbol or `bind` this captured symbol explicitly [GenericsIgnoredInjection]]#
+
+  "ok"
+
+discard g(int)