new .redefine pragma for templates, warn on redefinition without it (#20211)

* test CI for template redefinitions

* adapt asyncmacro

* fix quote

* fix again

* try something else

* revert

* fix ioselectors_select, disable packages CI

* adapt more tests & simplify

* more

* more

* more

* rename to redefine, warn on implicit redefinition

* basic documentation [skip ci]

* Update compiler/lineinfos.nim

Co-authored-by: konsumlamm <44230978+konsumlamm@users.noreply.github.com>

Co-authored-by: konsumlamm <44230978+konsumlamm@users.noreply.github.com>

changelog.md

@@ -123,6 +123,12 @@
       Baz = object
     ```
 
+- Redefining templates with the same signature implicitly was previously
+  allowed to support certain macro code. A `{.redefine.}` pragma has been
+  added to make this work explicitly, and a warning is generated in the case
+  where it is implicit. This behavior only applies to templates, redefinition
+  is generally disallowed for other symbols.
+
 ## Compiler changes
 
 - The `gc` switch has been renamed to `mm` ("memory management") in order to reflect the

compiler/ast.nim

@@ -334,6 +334,7 @@ const
   sfEscapes* = sfProcvar              # param escapes
   sfBase* = sfDiscriminant
   sfCustomPragma* = sfRegister        # symbol is custom pragma template
+  sfTemplateRedefinition* = sfExportc # symbol is a redefinition of an earlier template
 
 const
   # getting ready for the future expr/stmt merge

compiler/condsyms.nim

@@ -140,3 +140,4 @@ proc initDefines*(symbols: StringTableRef) =
   defineSymbol("nimHasEffectsOf")
 
   defineSymbol("nimHasEnforceNoRaises")
+  defineSymbol("nimHasTemplateRedefinitionPragma")

compiler/lineinfos.nim

@@ -81,6 +81,7 @@ type
     warnCstringConv = "CStringConv",
     warnEffect = "Effect",
     warnCastSizes = "CastSizes"
+    warnTemplateRedefinition = "TemplateRedefinition",
     warnUser = "User",
     # hints
     hintSuccess = "Success", hintSuccessX = "SuccessX",
@@ -175,6 +176,7 @@ const
     warnCstringConv: "$1",
     warnEffect: "$1",
     warnCastSizes: "$1",
+    warnTemplateRedefinition: "template '$1' is implicitly redefined, consider adding an explicit .redefine pragma",
     warnUser: "$1",
     hintSuccess: "operation successful: $#",
     # keep in sync with `testament.isSuccess`

compiler/pragmas.nim

@@ -38,7 +38,7 @@ const
   converterPragmas* = procPragmas
   methodPragmas* = procPragmas+{wBase}-{wImportCpp}
   templatePragmas* = {wDeprecated, wError, wGensym, wInject, wDirty,
-    wDelegator, wExportNims, wUsed, wPragma}
+    wDelegator, wExportNims, wUsed, wPragma, wRedefine}
   macroPragmas* = declPragmas + {FirstCallConv..LastCallConv,
     wMagic, wNoSideEffect, wCompilerProc, wNonReloadable, wCore,
     wDiscardable, wGensym, wInject, wDelegator}
@@ -870,6 +870,9 @@ proc singlePragma(c: PContext, sym: PSym, n: PNode, i: var int,
       of wDirty:
         if sym.kind == skTemplate: incl(sym.flags, sfDirty)
         else: invalidPragma(c, it)
+      of wRedefine:
+        if sym.kind == skTemplate: incl(sym.flags, sfTemplateRedefinition)
+        else: invalidPragma(c, it)
       of wImportCpp:
         processImportCpp(c, sym, getOptionalStr(c, it, "$1"), it.info)
       of wCppNonPod:

compiler/semexprs.nim

@@ -2112,10 +2112,12 @@ proc semQuoteAst(c: PContext, n: PNode): PNode =
   ids[0] = newAnonSym(c, skParam, n.info).newSymNode
   processQuotations(c, quotedBlock, op, quotes, ids)
 
+  let dummyTemplateSym = newAnonSym(c, skTemplate, n.info)
+  incl(dummyTemplateSym.flags, sfTemplateRedefinition)
   var dummyTemplate = newProcNode(
     nkTemplateDef, quotedBlock.info, body = quotedBlock,
     params = c.graph.emptyNode,
-    name = newAnonSym(c, skTemplate, n.info).newSymNode,
+    name = dummyTemplateSym.newSymNode,
               pattern = c.graph.emptyNode, genericParams = c.graph.emptyNode,
               pragmas = c.graph.emptyNode, exceptions = c.graph.emptyNode)
 

compiler/semtempl.nim

@@ -691,6 +691,9 @@ proc semTemplateDef(c: PContext, n: PNode): PNode =
   if proto == nil:
     addInterfaceOverloadableSymAt(c, c.currentScope, s)
   elif not comesFromShadowscope:
+    if {sfTemplateRedefinition, sfGenSym} * s.flags == {}:
+      #wrongRedefinition(c, n.info, proto.name.s, proto.info)
+      message(c.config, n.info, warnTemplateRedefinition, s.name.s)
     symTabReplace(c.currentScope.symbols, proto, s)
   if n[patternPos].kind != nkEmpty:
     c.patterns.add(s)

compiler/wordrecg.nim

@@ -87,6 +87,7 @@ type
     wGlobal = "global", wCodegenDecl = "codegenDecl", wUnchecked = "unchecked",
     wGuard = "guard", wLocks = "locks", wPartial = "partial", wExplain = "explain",
     wLiftLocals = "liftlocals", wEnforceNoRaises = "enforceNoRaises",
+    wRedefine = "redefine",
 
     wAuto = "auto", wBool = "bool", wCatch = "catch", wChar = "char",
     wClass = "class", wCompl = "compl", wConstCast = "const_cast", wDefault = "default",

doc/manual.md

@@ -7050,6 +7050,22 @@ immediate pragma
 The immediate pragma is obsolete. See `Typed vs untyped parameters
 <#templates-typed-vs-untyped-parameters>`_.
 
+redefine pragma
+---------------
+
+Redefinition of template symbols with the same signature is allowed.
+This can be made explicit with the `redefine` pragma:
+
+```nim
+template foo: int = 1
+echo foo() # 1
+template foo: int {.redefine.} = 2
+echo foo() # 2
+# warning: implicit redefinition of template
+template foo: int = 3
+```
+
+This is mostly intended for macro generated code. 
 
 compilation option pragmas
 --------------------------

lib/pure/asyncmacro.nim

@@ -126,7 +126,9 @@ template await*(f: typed): untyped {.used.} =
     error "await expects Future[T], got " & $typeof(f)
 
 template await*[T](f: Future[T]): auto {.used.} =
-  template yieldFuture = yield FutureBase()
+  when not defined(nimHasTemplateRedefinitionPragma):
+    {.pragma: redefine.}
+  template yieldFuture {.redefine.} = yield FutureBase()
 
   when compiles(yieldFuture):
     var internalTmpFuture: FutureBase = f

lib/pure/ioselects/ioselectors_select.nim

@@ -398,18 +398,6 @@ proc contains*[T](s: Selector[T], fd: SocketHandle|int): bool {.inline.} =
       if s.fds[i].ident == fdi:
         return true
 
-when hasThreadSupport:
-  template withSelectLock[T](s: Selector[T], body: untyped) =
-    acquire(s.lock)
-    {.locks: [s.lock].}:
-      try:
-        body
-      finally:
-        release(s.lock)
-else:
-  template withSelectLock[T](s: Selector[T], body: untyped) =
-    body
-
 proc getData*[T](s: Selector[T], fd: SocketHandle|int): var T =
   s.withSelectLock():
     let fdi = int(fd)

lib/std/assertions.nim

@@ -85,7 +85,9 @@ template onFailedAssert*(msg, code: untyped): untyped {.dirty.} =
     onFailedAssert(msg):
       raise (ref MyError)(msg: msg, lineinfo: instantiationInfo(-2))
     doAssertRaises(MyError): doAssert false
-  template failedAssertImpl(msgIMPL: string): untyped {.dirty.} =
+  when not defined(nimHasTemplateRedefinitionPragma):
+    {.pragma: redefine.}
+  template failedAssertImpl(msgIMPL: string): untyped {.dirty, redefine.} =
     let msg = msgIMPL
     code
 
@@ -98,7 +100,7 @@ template doAssertRaises*(exception: typedesc, code: untyped) =
   var wrong = false
   const begin = "expected raising '" & astToStr(exception) & "', instead"
   const msgEnd = " by: " & astToStr(code)
-  template raisedForeign = raiseAssert(begin & " raised foreign exception" & msgEnd)
+  template raisedForeign {.gensym.} = raiseAssert(begin & " raised foreign exception" & msgEnd)
   when Exception is exception:
     try:
       if true:

tests/lookups/tredef.nim

@@ -4,7 +4,7 @@ foo(1, "test")
 proc bar(a: int, b: string) = discard
 bar(1, "test")
 
-template foo(a: int, b: string) = bar(a, b)
+template foo(a: int, b: string) {.redefine.} = bar(a, b)
 foo(1, "test")
 
 block:

tests/macros/tstructuredlogging.nim

@@ -66,7 +66,7 @@ macro mergeScopes(scopeHolders: typed, newBindings: untyped): untyped =
     newScopeDefinition.add newAssignment(newIdentNode(k), v)
 
   result = quote:
-    template scopeHolder = `newScopeDefinition`
+    template scopeHolder {.redefine.} = `newScopeDefinition`
 
 template scope(newBindings: untyped) {.dirty.} =
   mergeScopes(bindSym"scopeHolder", newBindings)

tests/parser/tstmtlists.nim

@@ -158,7 +158,7 @@ template dim2: int =
    else:
     int.high)
 
-template dim: int =
+template dim3: int =
   (
    if int.high == 0:
      int.high

tests/template/template_various.nim

@@ -274,10 +274,10 @@ parse9:
 block gensym1:
   template x: untyped = -1
   template t1() =
-    template x: untyped {.gensym.} = 1
+    template x: untyped {.gensym, redefine.} = 1
     echo x()  # 1
   template t2() =
-    template x: untyped = 1  # defaults to {.inject.}
+    template x: untyped {.redefine.} = 1  # defaults to {.inject.}
     echo x()  # -1  injected x not available during template definition
   t1()
   t2()

tests/template/tredefinition_override.nim

@@ -0,0 +1,33 @@
+{.push warningAsError[TemplateRedefinition]: on.}
+
+doAssert not (compiles do:
+  template foo(): int = 1
+  template foo(): int = 2)
+doAssert (compiles do:
+  template foo(): int = 1
+  template foo(): int {.redefine.} = 2)
+doAssert not (compiles do:
+  block:
+    template foo() =
+      template bar: string {.gensym.} = "a"
+      template bar: string {.gensym.} = "b"
+    foo())
+doAssert (compiles do:
+  block:
+    template foo() =
+      template bar: string {.gensym.} = "a"
+      template bar: string {.gensym, redefine.} = "b"
+    foo())
+
+block:
+  template foo(): int = 1
+  template foo(): int {.redefine.} = 2
+  doAssert foo() == 2
+block:
+  template foo(): string =
+    template bar: string {.gensym.} = "a"
+    template bar: string {.gensym, redefine.} = "b"
+    bar()
+  doAssert foo() == "b"
+
+{.pop.}

tests/template/utemplates.nim

@@ -22,7 +22,7 @@ block: # templates can be redefined multiple times
     if not cond: fail(msg)
 
   template assertionFailed(body: untyped) {.dirty.} =
-    template fail(msg: string): typed =
+    template fail(msg: string): typed {.redefine.} =
       body
 
   assertionFailed:

tests/trmacros/trmacros_various2.nim

@@ -33,8 +33,8 @@ block tpartial:
   proc p(x, y: int; cond: bool): int =
     result = if cond: x + y else: x - y
 
-  template optP{p(x, y, true)}(x, y): untyped = x - y
-  template optP{p(x, y, false)}(x, y): untyped = x + y
+  template optPTrue{p(x, y, true)}(x, y): untyped = x - y
+  template optPFalse{p(x, y, false)}(x, y): untyped = x + y
 
   echo p(2, 4, true)