Fix forward declaration issues in template/macro context (nim-lang#15091

)

* Fix forward declaration issues in template/macro context

* Correct forward declaration resolving for overloads

* Remove old dead code

* WIP consistent gensym ids

* Minimize diff

* Remove obsoleted hack

* Add templInstCounter to give unique IDs to template instantiations

* Remove obsoleted code

* Eh, init in myOpen, not myProcess...

* Remove optNimV019

* Add testcase for nim-lang#13484

changelog.md

@@ -264,6 +264,7 @@ proc mydiv(a, b): int {.raises: [].} =
 - Removed the `--oldNewlines` switch.
 - Removed the `--laxStrings` switch for mutating the internal zero terminator on strings.
 - Removed the `--oldast` switch.
+- Removed the `--oldgensym` switch
 - `$getType(untyped)` is now "untyped" instead of "expr", `$getType(typed)` is
   now "typed" instead of "stmt".
 - Sink inference is now disabled per default and has to enabled explicitly via

compiler/commands.nim

@@ -871,8 +871,6 @@ proc processSwitch*(switch, arg: string, pass: TCmdLinePass, info: TLineInfo;
   of "expandarc":
     expectArg(conf, switch, arg, pass, info)
     conf.arcToExpand[arg] = "T"
-  of "oldgensym":
-    processOnOffSwitchG(conf, {optNimV019}, arg, pass, info)
   of "useversion":
     expectArg(conf, switch, arg, pass, info)
     case arg

compiler/evaltempl.nim

@@ -21,6 +21,7 @@ type
                       # new symbol
     config: ConfigRef
     ic: IdentCache
+    instID: int
 
 proc copyNode(ctx: TemplCtx, a, b: PNode): PNode =
   result = copyNode(a)
@@ -53,8 +54,8 @@ proc evalTemplateAux(templ, actual: PNode, c: var TemplCtx, result: PNode) =
           #if x.kind == skParam and x.owner.kind == skModule:
           #  internalAssert c.config, false
           idTablePut(c.mapping, s, x)
-        if sfGenSym in s.flags and optNimV019 notin c.config.globalOptions:
-          result.add newIdentNode(getIdent(c.ic, x.name.s & "`gensym" & $x.id),
+        if sfGenSym in s.flags:
+          result.add newIdentNode(getIdent(c.ic, x.name.s & "`gensym" & $c.instID),
             if c.instLines: actual.info else: templ.info)
         else:
           result.add newSymNode(x, if c.instLines: actual.info else: templ.info)
@@ -166,7 +167,7 @@ proc wrapInComesFrom*(info: TLineInfo; sym: PSym; res: PNode): PNode =
 
 proc evalTemplate*(n: PNode, tmpl, genSymOwner: PSym;
                    conf: ConfigRef;
-                   ic: IdentCache;
+                   ic: IdentCache; instID: ref int;
                    fromHlo=false): PNode =
   inc(conf.evalTemplateCounter)
   if conf.evalTemplateCounter > evalTemplateLimit:
@@ -181,6 +182,7 @@ proc evalTemplate*(n: PNode, tmpl, genSymOwner: PSym;
   ctx.config = conf
   ctx.ic = ic
   initIdTable(ctx.mapping)
+  ctx.instID = instID[]
 
   let body = tmpl.getBody
   #echo "instantion of ", renderTree(body, {renderIds})
@@ -203,3 +205,5 @@ proc evalTemplate*(n: PNode, tmpl, genSymOwner: PSym;
   #if ctx.debugActive:
   #  echo "instantion of ", renderTree(result, {renderIds})
   dec(conf.evalTemplateCounter)
+  # The instID must be unique for every template instantiation, so we increment it here
+  inc instID[]

compiler/lookups.nim

@@ -316,8 +316,7 @@ proc qualifiedLookUp*(c: PContext, n: PNode, flags: set[TLookupFlag]): PSym =
       fixSpelling(n, ident, searchInScopes)
       errorUndeclaredIdentifier(c, n.info, ident.s)
       result = errorSym(c, n)
-    elif checkAmbiguity in flags and result != nil and
-        contains(c.ambiguousSymbols, result.id):
+    elif checkAmbiguity in flags and result != nil and result.id in c.ambiguousSymbols:
       errorUseQualifier(c, n.info, result)
   of nkSym:
     result = n.sym

compiler/options.nim

@@ -89,7 +89,6 @@ type                          # please make sure we have under 32 options
                               # implementation
     optOwnedRefs              # active if the Nim compiler knows about 'owned'.
     optMultiMethods
-    optNimV019
     optBenchmarkVM            # Enables cpuTime() in the VM
     optProduceAsm             # produce assembler code
     optPanics                 # turn panics (sysFatal) into a process termination

compiler/procfind.nim

@@ -28,40 +28,7 @@ proc equalGenericParams(procA, procB: PNode): bool =
       if not exprStructuralEquivalent(a.ast, b.ast): return
   result = true
 
-proc searchForProcOld*(c: PContext, scope: PScope, fn: PSym): PSym =
-  # Searches for a forward declaration or a "twin" symbol of fn
-  # in the symbol table. If the parameter lists are exactly
-  # the same the sym in the symbol table is returned, else nil.
-  var it: TIdentIter
-  result = initIdentIter(it, scope.symbols, fn.name)
-  if isGenericRoutine(fn):
-    # we simply check the AST; this is imprecise but nearly the best what
-    # can be done; this doesn't work either though as type constraints are
-    # not kept in the AST ..
-    while result != nil:
-      if result.kind == fn.kind and isGenericRoutine(result):
-        let genR = result.ast[genericParamsPos]
-        let genF = fn.ast[genericParamsPos]
-        if exprStructuralEquivalent(genR, genF) and
-           exprStructuralEquivalent(result.ast[paramsPos],
-                                    fn.ast[paramsPos]) and
-           equalGenericParams(genR, genF):
-            return
-      result = nextIdentIter(it, scope.symbols)
-  else:
-    while result != nil:
-      if result.kind == fn.kind and not isGenericRoutine(result):
-        case equalParams(result.typ.n, fn.typ.n)
-        of paramsEqual:
-          return
-        of paramsIncompatible:
-          localError(c.config, fn.info, "overloaded '$1' leads to ambiguous calls" % fn.name.s)
-          return
-        of paramsNotEqual:
-          discard
-      result = nextIdentIter(it, scope.symbols)
-
-proc searchForProcNew(c: PContext, scope: PScope, fn: PSym): PSym =
+proc searchForProc*(c: PContext, scope: PScope, fn: PSym): PSym =
   const flags = {ExactGenericParams, ExactTypeDescValues,
                  ExactConstraints, IgnoreCC}
   var it: TIdentIter
@@ -83,16 +50,6 @@ proc searchForProcNew(c: PContext, scope: PScope, fn: PSym): PSym =
         discard
     result = nextIdentIter(it, scope.symbols)
 
-proc searchForProc*(c: PContext, scope: PScope, fn: PSym): PSym =
-  result = searchForProcNew(c, scope, fn)
-  when false:
-    let old = searchForProcOld(c, scope, fn)
-    if old != result:
-      echo "Mismatch in searchForProc: ", fn.info
-      debug fn.typ
-      debug if result != nil: result.typ else: nil
-      debug if old != nil: old.typ else: nil
-
 when false:
   proc paramsFitBorrow(child, parent: PNode): bool =
     result = false

compiler/sem.nim

@@ -472,7 +472,7 @@ proc semMacroExpr(c: PContext, n, nOrig: PNode, sym: PSym,
 
   #if c.evalContext == nil:
   #  c.evalContext = c.createEvalContext(emStatic)
-  result = evalMacroCall(c.module, c.graph, n, nOrig, sym)
+  result = evalMacroCall(c.module, c.graph, c.templInstCounter, n, nOrig, sym)
   if efNoSemCheck notin flags:
     result = semAfterMacroCall(c, n, result, sym, flags)
   if c.config.macrosToExpand.hasKey(sym.name.s):
@@ -521,6 +521,7 @@ proc myOpen(graph: ModuleGraph; module: PSym): PPassContext {.nosinks.} =
   c.semTypeNode = semTypeNode
   c.instTypeBoundOp = sigmatch.instTypeBoundOp
   c.hasUnresolvedArgs = hasUnresolvedArgs
+  c.templInstCounter = new int
 
   pushProcCon(c, module)
   pushOwner(c, c.module)

compiler/semdata.nim

@@ -35,7 +35,6 @@ type
     inTryStmt*: int           # whether we are in a try statement; works also
                               # in standalone ``except`` and ``finally``
     next*: PProcCon           # used for stacking procedure contexts
-    wasForwarded*: bool       # whether the current proc has a separate header
     mappingExists*: bool
     mapping*: TIdTable
     caseContext*: seq[tuple[n: PNode, idx: int]]
@@ -85,6 +84,7 @@ type
                                # this is used so that generic instantiations
                                # can access private object fields
     instCounter*: int          # to prevent endless instantiations
+    templInstCounter*: ref int # gives every template instantiation a unique id
 
     ambiguousSymbols*: IntSet  # ids of all ambiguous symbols (cannot
                                # store this info in the syms themselves!)

compiler/semexprs.nim

@@ -33,7 +33,7 @@ proc semTemplateExpr(c: PContext, n: PNode, s: PSym,
   # Note: This is n.info on purpose. It prevents template from creating an info
   # context when called from an another template
   pushInfoContext(c.config, n.info, s.detailedInfo)
-  result = evalTemplate(n, s, getCurrOwner(c), c.config, c.cache, efFromHlo in flags)
+  result = evalTemplate(n, s, getCurrOwner(c), c.config, c.cache, c.templInstCounter, efFromHlo in flags)
   if efNoSemCheck notin flags: result = semAfterMacroCall(c, n, result, s, flags)
   popInfoContext(c.config)
 
@@ -1203,17 +1203,6 @@ proc semSym(c: PContext, n: PNode, sym: PSym, flags: TExprFlags): PNode =
     elif sfGenSym in s.flags:
       # the owner should have been set by now by addParamOrResult
       internalAssert c.config, s.owner != nil
-      if c.p.wasForwarded:
-        # gensym'ed parameters that nevertheless have been forward declared
-        # need a special fixup:
-        let realParam = c.p.owner.typ.n[s.position+1]
-        internalAssert c.config, realParam.kind == nkSym and realParam.sym.kind == skParam
-        return newSymNode(c.p.owner.typ.n[s.position+1].sym, n.info)
-      elif c.p.owner.kind == skMacro:
-        # gensym'ed macro parameters need a similar hack (see bug #1944):
-        var u = searchInScopes(c, s.name)
-        internalAssert c.config, u != nil and u.kind == skParam and u.owner == s.owner
-        return newSymNode(u, n.info)
     result = newSymNode(s, n.info)
   of skVar, skLet, skResult, skForVar:
     if s.magic == mNimvm:

compiler/semstmts.nim

@@ -1898,6 +1898,10 @@ proc semProcAux(c: PContext, n: PNode, kind: TSymKind,
     incl(s.typ.flags, tfNoSideEffect)
   var proto: PSym = if isAnon: nil
                     else: searchForProc(c, oldScope, s)
+  if proto == nil and sfForward in s.flags:
+    #This is a definition that shares its sym with its forward declaration (generated by a macro),
+    #if the symbol is also gensymmed we won't find it with searchForProc, so we check here
+    proto = s
   if proto == nil:
     if s.kind == skIterator:
       if s.typ.callConv != ccClosure:
@@ -1943,7 +1947,6 @@ proc semProcAux(c: PContext, n: PNode, kind: TSymKind,
       addGenericParamListToScope(c, proto.ast[genericParamsPos])
     addParams(c, proto.typ.n, proto.kind)
     proto.info = s.info       # more accurate line information
-    s.typ = proto.typ
     proto.options = s.options
     s = proto
     n[genericParamsPos] = proto.ast[genericParamsPos]
@@ -1983,7 +1986,6 @@ proc semProcAux(c: PContext, n: PNode, kind: TSymKind,
       if n[genericParamsPos].kind == nkEmpty or usePseudoGenerics:
         if not usePseudoGenerics and s.magic == mNone: paramsTypeCheck(c, s.typ)
 
-        c.p.wasForwarded = proto != nil
         maybeAddResult(c, s, n)
         # semantic checking also needed with importc in case used in VM
         s.ast[bodyPos] = hloBody(c, semProcBody(c, n[bodyPos]))

compiler/semtempl.nim

@@ -145,9 +145,6 @@ proc getIdentNode(c: var TemplCtx, n: PNode): PNode =
     illFormedAst(n, c.c.config)
     result = n
 
-template oldCheck(cx: TemplCtx; cond: bool): bool =
-  (optNimV019 notin cx.c.config.globalOptions or cond)
-
 proc isTemplParam(c: TemplCtx, n: PNode): bool {.inline.} =
   result = n.kind == nkSym and n.sym.kind == skParam and
            n.sym.owner == c.owner and sfTemplateParam in n.sym.flags
@@ -215,21 +212,7 @@ proc addLocalDecl(c: var TemplCtx, n: var PNode, k: TSymKind) =
         closeScope(c)
     let ident = getIdentNode(c, n)
     if not isTemplParam(c, ident):
-      # fix #2670, consider:
-      #
-      # when b:
-      #    var a = "hi"
-      # else:
-      #    var a = 5
-      # echo a
-      #
-      # We need to ensure that both 'a' produce the same gensym'ed symbol.
-      # So we need only check the *current* scope.
-      let s = localSearchInScope(c.c, considerQuotedIdent(c.c, ident))
-      if s != nil and s.owner == c.owner and sfGenSym in s.flags:
-        onUse(n.info, s)
-        replaceIdentBySym(c.c, n, newSymNode(s, n.info))
-      elif n.kind != nkSym:
+      if n.kind != nkSym:
         let local = newGenSym(k, ident, c)
         addPrelimDecl(c.c, local)
         styleCheckDef(c.c.config, n.info, local)
@@ -560,16 +543,16 @@ proc semTemplBody(c: var TemplCtx, n: PNode): PNode =
     if n.kind == nkDotExpr:
       result = n
       result[0] = semTemplBody(c, n[0])
-      if optNimV019 notin c.c.config.globalOptions: inc c.noGenSym
+      inc c.noGenSym
       result[1] = semTemplBody(c, n[1])
-      if optNimV019 notin c.c.config.globalOptions: dec c.noGenSym
+      dec c.noGenSym
     else:
       result = semTemplBodySons(c, n)
   of nkExprColonExpr, nkExprEqExpr:
     if n.len == 2:
-      if optNimV019 notin c.c.config.globalOptions: inc c.noGenSym
+      inc c.noGenSym
       result[0] = semTemplBody(c, n[0])
-      if optNimV019 notin c.c.config.globalOptions: dec c.noGenSym
+      dec c.noGenSym
       result[1] = semTemplBody(c, n[1])
     else:
       result = semTemplBodySons(c, n)

compiler/types.nim

@@ -936,7 +936,7 @@ proc equalParams(a, b: PNode): TParamsEquality =
         discard
       of paramsIncompatible:
         result = paramsIncompatible
-      if (m.name.id != n.name.id):
+      if m.name.id != n.name.id:
         # BUGFIX
         return paramsNotEqual # paramsIncompatible;
       # continue traversal! If not equal, we can return immediately; else

compiler/vm.nim

@@ -1233,7 +1233,7 @@ proc rawExecute(c: PCtx, start: int, tos: PStackFrame): TFullReg =
           let node = regs[rb+i].regToNode
           node.info = c.debug[pc]
           macroCall.add(node)
-        var a = evalTemplate(macroCall, prc, genSymOwner, c.config, c.cache)
+        var a = evalTemplate(macroCall, prc, genSymOwner, c.config, c.cache, c.templInstCounter)
         if a.kind == nkStmtList and a.len == 1: a = a[0]
         a.recSetFlagIsRef
         ensureKind(rkNode)
@@ -2232,7 +2232,7 @@ proc errorNode(owner: PSym, n: PNode): PNode =
   result.typ = newType(tyError, owner)
   result.typ.flags.incl tfCheckedForDestructor
 
-proc evalMacroCall*(module: PSym; g: ModuleGraph;
+proc evalMacroCall*(module: PSym; g: ModuleGraph; templInstCounter: ref int;
                     n, nOrig: PNode, sym: PSym): PNode =
   if g.config.errorCounter > 0: return errorNode(module, n)
 
@@ -2253,6 +2253,7 @@ proc evalMacroCall*(module: PSym; g: ModuleGraph;
   c.mode = emStaticStmt
   c.comesFromHeuristic.line = 0'u16
   c.callsite = nOrig
+  c.templInstCounter = templInstCounter
   let start = genProc(c, sym)
 
   var tos = PStackFrame(prc: sym, comesFrom: 0, next: nil)

compiler/vmdef.nim

@@ -262,6 +262,7 @@ type
     graph*: ModuleGraph
     oldErrorCount*: int
     profiler*: Profiler
+    templInstCounter*: ref int # gives every template instantiation a unique ID, needed here for getAst
 
   PStackFrame* = ref TStackFrame
   TStackFrame* = object

testament/important_packages.nim

@@ -12,7 +12,7 @@ var packages2*: seq[tuple[name, cmd: string; hasDeps: bool; url: string, useHead
 # pkg1 "alea", true
 pkg1 "argparse"
 pkg1 "arraymancer", true, "nim c tests/tests_cpu.nim"
-pkg1 "ast_pattern_matching", false, "nim c -r --oldgensym:on tests/test1.nim"
+#pkg1 "ast_pattern_matching", false, "nim c -r --oldgensym:on tests/test1.nim"
 pkg1 "awk", true
 pkg1 "bigints"
 pkg1 "binaryheap", false, "nim c -r binaryheap.nim"
@@ -95,7 +95,7 @@ pkg2 "optionsutils"
 pkg2 "ormin", true, "nim c -o:orminn ormin.nim"
 pkg2 "parsetoml"
 pkg2 "patty"
-pkg2 "plotly", true, "nim c --oldgensym:on examples/all.nim"
+pkg2 "plotly", true, "nim c examples/all.nim"
 pkg2 "pnm"
 pkg2 "polypbren"
 pkg2 "prologue", true, "nim c -d:release -r tests/test_compile/test_compile.nim"