From cd946084abdf3ab64b2540e2f29c1034ae1511e5 Mon Sep 17 00:00:00 2001
From: metagn <metagngn@gmail.com>
Date: Tue, 16 Jul 2024 00:47:06 -0600
Subject: [PATCH] make routine implicitly gensym when other gensym symbol
 exists again (#23842)

fixes #23813, partially reverts #23392

Before #23392, if a `gensym` symbol was defined before a proc with the
same name in a template even with an `inject` annotation, the proc would
be `gensym`. After #23392 the proc was instead changed to be `inject` as
long as no `gensym` annotation was given. Now, to keep compatibility
with the old behavior, the behavior is changed back to infer the proc as
`gensym` when no `inject` annotation is given, however an explicit
`inject` annotation will still inject the proc. This is also documented
in the manual as the old behavior was undocumented and the new behavior
is slightly different.
---
 compiler/semtempl.nim                 | 12 +++++++-----
 doc/manual.md                         |  9 ++++++---
 tests/errmsgs/tinconsistentgensym.nim |  2 +-
 tests/template/tinnerouterproc.nim    | 12 ++++++++++++
 4 files changed, 26 insertions(+), 9 deletions(-)

diff --git a/compiler/semtempl.nim b/compiler/semtempl.nim
index f2083c85c4..c6f2fb60cc 100644
--- a/compiler/semtempl.nim
+++ b/compiler/semtempl.nim
@@ -256,25 +256,27 @@ proc semTemplSymbol(c: PContext, n: PNode, s: PSym; isField: bool): PNode =
     if not isField:
       styleCheckUse(c, n.info, s)
 
-proc semRoutineInTemplName(c: var TemplCtx, n: PNode): PNode =
+proc semRoutineInTemplName(c: var TemplCtx, n: PNode, explicitInject: bool): PNode =
   result = n
   if n.kind == nkIdent:
     let s = qualifiedLookUp(c.c, n, {})
     if s != nil:
-      if s.owner == c.owner and s.kind == skParam:
+      if s.owner == c.owner and (s.kind == skParam or
+          (sfGenSym in s.flags and not explicitInject)):
         incl(s.flags, sfUsed)
         result = newSymNode(s, n.info)
         onUse(n.info, s)
   else:
     for i in 0..<n.safeLen:
-      result[i] = semRoutineInTemplName(c, n[i])
+      result[i] = semRoutineInTemplName(c, n[i], explicitInject)
 
 proc semRoutineInTemplBody(c: var TemplCtx, n: PNode, k: TSymKind): PNode =
   result = n
   checkSonsLen(n, bodyPos + 1, c.c.config)
   if n.kind notin nkLambdaKinds:
     # routines default to 'inject':
-    if symBinding(n[pragmasPos]) == spGenSym:
+    let binding = symBinding(n[pragmasPos])
+    if binding == spGenSym:
       let (ident, hasParam) = getIdentReplaceParams(c, n[namePos])
       if not hasParam:
         var s = newGenSym(k, ident, c)
@@ -286,7 +288,7 @@ proc semRoutineInTemplBody(c: var TemplCtx, n: PNode, k: TSymKind): PNode =
       else:
         n[namePos] = ident
     else:
-      n[namePos] = semRoutineInTemplName(c, n[namePos])
+      n[namePos] = semRoutineInTemplName(c, n[namePos], binding == spInject)
   # open scope for parameters
   openScope(c)
   for i in patternPos..paramsPos-1:
diff --git a/doc/manual.md b/doc/manual.md
index 16c116328a..e471658a49 100644
--- a/doc/manual.md
+++ b/doc/manual.md
@@ -6216,9 +6216,12 @@ scope is controlled by the `inject`:idx: and `gensym`:idx: pragmas:
 `gensym`'ed symbols are not exposed but `inject`'ed symbols are.
 
 The default for symbols of entity `type`, `var`, `let` and `const`
-is `gensym` and for `proc`, `iterator`, `converter`, `template`,
-`macro` is `inject`. However, if the name of the entity is passed as a
-template parameter, it is an `inject`'ed symbol:
+is `gensym`. For `proc`, `iterator`, `converter`, `template`,
+`macro`, the default is `inject`, but if a `gensym` symbol with the same name
+is defined in the same syntax-level scope, it will be `gensym` by default.
+This can be overriden by marking the routine as `inject`. 
+
+If the name of the entity is passed as a template parameter, it is an `inject`'ed symbol:
 
   ```nim
   template withFile(f, fn, mode: untyped, actions: untyped): untyped =
diff --git a/tests/errmsgs/tinconsistentgensym.nim b/tests/errmsgs/tinconsistentgensym.nim
index 026c17fcaa..8e4c851068 100644
--- a/tests/errmsgs/tinconsistentgensym.nim
+++ b/tests/errmsgs/tinconsistentgensym.nim
@@ -7,7 +7,7 @@ block:
     when false:
       let x = 123
     else:
-      template x: untyped = 456
+      template x: untyped {.inject.} = 456
     echo x #[tt.Error
          ^ undeclared identifier: 'x`gensym0'; if declared in a template, this identifier may be inconsistently marked inject or gensym]#
   foo()
diff --git a/tests/template/tinnerouterproc.nim b/tests/template/tinnerouterproc.nim
index 1f15fb13e0..56e0d02df1 100644
--- a/tests/template/tinnerouterproc.nim
+++ b/tests/template/tinnerouterproc.nim
@@ -6,3 +6,15 @@ block: # #20002
     discard 3.bar # evaluates to 10 but only check if it compiles for now
   block:
     foo()
+
+block: # issue #23813
+  template r(body: untyped) =
+    proc x() {.gensym.} =
+      body
+  template g() =
+    r:
+      let y = 0
+    r:
+      proc y() = discard
+      y()
+  g()