stop gensym identifiers hijacking routine decl names in templates (#23392)

fixes #23326

In a routine declaration node in a template, if the routine is marked as
`gensym`, the compiler adds it as a new symbol to a preliminary scope of
the template. If it's not marked as gensym, then it searches the
preliminary scope of the template for the name of the routine, then when
it matches a template parameter or a gensym identifier, the compiler
replaces the name node with a symbol node of the found symbol.

This makes sense for the template parameter since it has to be replaced
later, but not really for the gensym identifier, as it doesn't allow us
to inject a routine with the same name as an identifier previously
declared as gensym (the problem in #23326 is when this is in another
`when` branch).

However this is the only channel to reuse a gensym symbol in a
declaration, so maybe removing it has side effects. For example if we
have:

```nim
proc foo(x: int) {.gensym.} = discard
proc foo(x: float) {.gensym.} = discard
```

it will not behave the same as

```nim
proc foo(x: int) {.gensym.} = discard
proc foo(x: float) = discard
```

behaved previously, which maybe allowed overloading over the gensym'd
symbols.

A note to the "undeclared identifier" error message has also been added
for a potential error code that implicitly depended on the old behavior
might give, namely ``undeclared identifier: 'abc`gensym123'``, which
happens when in a template an identifier is first declared gensym in
code that doesn't compile, then as a routine which injects by default,
then the identifier is used.

(cherry picked from commit 73b0b0d31c)
This commit is contained in:
metagn
2024-04-09 15:37:34 +03:00
committed by narimiran
parent a798356838
commit b1bfba9a31
5 changed files with 71 additions and 5 deletions

View File

@@ -566,7 +566,11 @@ proc errorUndeclaredIdentifier*(c: PContext; info: TLineInfo; name: string, extr
if name == "_":
err = "the special identifier '_' is ignored in declarations and cannot be used"
else:
err = "undeclared identifier: '" & name & "'" & extra
err = "undeclared identifier: '" & name & "'"
if "`gensym" in name:
err.add "; if declared in a template, this identifier may be inconsistently marked inject or gensym"
if extra.len != 0:
err.add extra
if c.recursiveDep.len > 0:
err.add "\nThis might be caused by a recursive module dependency:\n"
err.add c.recursiveDep

View File

@@ -260,7 +260,7 @@ proc semRoutineInTemplName(c: var TemplCtx, n: PNode): PNode =
if n.kind == nkIdent:
let s = qualifiedLookUp(c.c, n, {})
if s != nil:
if s.owner == c.owner and (s.kind == skParam or sfGenSym in s.flags):
if s.owner == c.owner and s.kind == skParam:
incl(s.flags, sfUsed)
result = newSymNode(s, n.info)
onUse(n.info, s)

View File

@@ -60,7 +60,7 @@ pkg "compactdict"
pkg "comprehension", "nimble test", "https://github.com/alehander92/comprehension"
pkg "cowstrings"
pkg "criterion", allowFailure = true # needs testing binary
pkg "datamancer"
pkg "datamancer", "nimble install -y arraymancer@#HEAD; nimble test"
pkg "dashing", "nim c tests/functional.nim"
pkg "delaunay"
pkg "docopt"
@@ -72,7 +72,7 @@ pkg "fragments", "nim c -r fragments/dsl.nim", allowFailure = true # pending htt
pkg "fusion"
pkg "gara"
pkg "glob"
pkg "ggplotnim", "nim c -d:noCairo -r tests/tests.nim"
pkg "ggplotnim", "nimble install -y arraymancer@#HEAD; nim c -d:noCairo -r tests/tests.nim"
pkg "gittyup", "nimble test", "https://github.com/disruptek/gittyup", allowFailure = true
pkg "gnuplot", "nim c gnuplot.nim"
# pkg "gram", "nim c -r --mm:arc --define:danger tests/test.nim", "https://github.com/disruptek/gram"
@@ -124,7 +124,7 @@ pkg "nimx", "nim c test/main.nim", allowFailure = true
pkg "nitter", "nim c src/nitter.nim", "https://github.com/zedeus/nitter"
pkg "norm", "testament r tests/common/tmodel.nim"
pkg "npeg", "nimble testarc"
pkg "numericalnim", "nimble nimCI"
pkg "numericalnim", "nimble install -y arraymancer@#HEAD; nimble nimCI"
pkg "optionsutils"
pkg "ormin", "nim c -o:orminn ormin.nim"
pkg "parsetoml"

View File

@@ -0,0 +1,25 @@
discard """
cmd: "nim check --hints:off $file"
"""
block:
template foo =
when false:
let x = 123
else:
template x: untyped = 456
echo x #[tt.Error
^ undeclared identifier: 'x`gensym0'; if declared in a template, this identifier may be inconsistently marked inject or gensym]#
foo()
block:
template foo(y: static bool) =
block:
when y:
let x {.gensym.} = 123
else:
let x {.inject.} = 456
echo x #[tt.Error
^ undeclared identifier: 'x']#
foo(false)
foo(true)

View File

@@ -0,0 +1,37 @@
# issue #23326
type Result*[E] = object
e*: E
proc error*[E](v: Result[E]): E = discard
template valueOr*[E](self: Result[E], def: untyped): int =
when E isnot void:
when false:
# Comment line below to make it work
template error(): E {.used, gensym.} = s.e
discard
else:
template error(): E {.used, inject.} =
self.e
def
else:
def
block:
let rErr = Result[string](e: "a")
let rErrV = rErr.valueOr:
ord(error[0])
block:
template foo(x: static bool): untyped =
when x:
let a = 123
else:
template a: untyped {.gensym.} = 456
a
doAssert foo(false) == 456
doAssert foo(true) == 123