Fix forward declarations in shadow scope contexts (#15386)

* Fix forward declarations in shadow scope contexts

* Add testcase for #15385

* Less empty lines

* Fix tests

* Inline isShadowScope

* Add original testcase (with reduced amount of iterations)

* Add testcase without forward decl

compiler/lookups.nim

@@ -91,13 +91,15 @@ proc skipAlias*(s: PSym; n: PNode; conf: ConfigRef): PSym =
       message(conf, n.info, warnDeprecated, "use " & result.name.s & " instead; " &
               s.name.s & " is deprecated")
 
+proc isShadowScope*(s: PScope): bool {.inline.} = s.parent != nil and s.parent.depthLevel == s.depthLevel
+
 proc localSearchInScope*(c: PContext, s: PIdent): PSym =
-  result = strTableGet(c.currentScope.symbols, s)
+  var scope = c.currentScope
-  var shadow = c.currentScope
+  result = strTableGet(scope.symbols, s)
-  while result == nil and shadow.parent != nil and shadow.depthLevel == shadow.parent.depthLevel:
+  while result == nil and scope.isShadowScope:
     # We are in a shadow scope, check in the parent too
-    result = strTableGet(shadow.parent.symbols, s)
+    scope = scope.parent
-    shadow = shadow.parent
+    result = strTableGet(scope.symbols, s)
 
 proc searchInScopes*(c: PContext, s: PIdent): PSym =
   for scope in walkScopes(c.currentScope):

compiler/procfind.nim

@@ -11,7 +11,7 @@
 # This is needed for proper handling of forward declarations.
 
 import
-  ast, astalgo, msgs, semdata, types, trees, strutils
+  ast, astalgo, msgs, semdata, types, trees, strutils, lookups
 
 proc equalGenericParams(procA, procB: PNode): bool =
   if procA.len != procB.len: return false
@@ -28,7 +28,7 @@ proc equalGenericParams(procA, procB: PNode): bool =
       if not exprStructuralEquivalent(a.ast, b.ast): return
   result = true
 
-proc searchForProc*(c: PContext, scope: PScope, fn: PSym): PSym =
+proc searchForProcAux(c: PContext, scope: PScope, fn: PSym): PSym =
   const flags = {ExactGenericParams, ExactTypeDescValues,
                  ExactConstraints, IgnoreCC}
   var it: TIdentIter
@@ -50,6 +50,14 @@ proc searchForProc*(c: PContext, scope: PScope, fn: PSym): PSym =
         discard
     result = nextIdentIter(it, scope.symbols)
 
+proc searchForProc*(c: PContext, scope: PScope, fn: PSym): tuple[proto: PSym, comesFromShadowScope: bool] =
+  var scope = scope
+  result.proto = searchForProcAux(c, scope, fn)
+  while result.proto == nil and scope.isShadowScope:
+    scope = scope.parent
+    result.proto = searchForProcAux(c, scope, fn)
+    result.comesFromShadowScope = true
+
 when false:
   proc paramsFitBorrow(child, parent: PNode): bool =
     result = false

compiler/semstmts.nim

@@ -1898,7 +1898,7 @@ proc semProcAux(c: PContext, n: PNode, kind: TSymKind,
   elif s.kind == skFunc:
     incl(s.flags, sfNoSideEffect)
     incl(s.typ.flags, tfNoSideEffect)
-  var proto: PSym = if isAnon: nil
+  var (proto, comesFromShadowScope) = if isAnon: (nil, false)
                                       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),
@@ -1941,6 +1941,7 @@ proc semProcAux(c: PContext, n: PNode, kind: TSymKind,
     onDefResolveForward(n[namePos].info, proto)
     if sfForward notin proto.flags and proto.magic == mNone:
       wrongRedefinition(c, n.info, proto.name.s, proto.info)
+    if not comesFromShadowScope:
       excl(proto.flags, sfForward)
       incl(proto.flags, sfWasForwarded)
     closeScope(c)         # close scope with wrong parameter symbols

compiler/semtempl.nim

@@ -662,10 +662,10 @@ proc semTemplateDef(c: PContext, n: PNode): PNode =
       localError(c.config, n[bodyPos].info, errImplOfXNotAllowed % s.name.s)
   elif n[bodyPos].kind == nkEmpty:
     localError(c.config, n.info, "implementation of '$1' expected" % s.name.s)
-  var proto = searchForProc(c, c.currentScope, s)
+  var (proto, comesFromShadowscope) = searchForProc(c, c.currentScope, s)
   if proto == nil:
     addInterfaceOverloadableSymAt(c, c.currentScope, s)
-  else:
+  elif not comesFromShadowscope:
     symTabReplace(c.currentScope.symbols, proto, s)
   if n[patternPos].kind != nkEmpty:
     c.patterns.add(s)

tests/macros/tmacros_issues.nim

@@ -39,6 +39,7 @@ true
 false
 true
 false
+1.0
 '''
 """
 
@@ -403,3 +404,79 @@ testEvenOdd5()
 # it works, because the forward decl and definition share the symbol and the compiler is forgiving here
 #testEvenOdd6() #Don't test it though, the compiler may become more strict in the future
 
+# bug #15385
+var captured_funcs {.compileTime.}: seq[NimNode] = @[]
+
+macro aad*(fns: varargs[typed]): typed =
+  result = newStmtList()
+  for fn in fns:
+    captured_funcs.add fn[0]
+    result.add fn
+
+func exp*(x: float): float ## get different error if you remove forward declaration
+
+func exp*(x: float): float {.aad.} =
+  var x1 = min(max(x, -708.4), 709.8)
+  var result: float   ## looks weird because it is taken from template expansion
+  result = x1 + 1.0
+  result
+
+template check_accuracy(f: untyped, rng: Slice[float], n: int, verbose = false): auto =
+
+  proc check_accuracy: tuple[avg_ulp: float, max_ulp: int] {.gensym.} =
+    let k = (rng.b - rng.a) / (float) n
+    var
+      res, x: float
+      i, max_ulp = 0
+      avg_ulp = 0.0
+
+    x = rng.a
+    while (i < n):
+      res = f(x)
+      i.inc
+      x = x + 0.001
+    (avg_ulp, max_ulp)
+  check_accuracy()
+
+discard check_accuracy(exp, -730.0..709.4, 4)
+
+# And without forward decl
+macro aad2*(fns: varargs[typed]): typed =
+  result = newStmtList()
+  for fn in fns:
+    captured_funcs.add fn[0]
+    result.add fn
+
+func exp2*(x: float): float {.aad2.} =
+  var x1 = min(max(x, -708.4), 709.8)
+  var result: float   ## looks weird because it is taken from template expansion
+  result = x1 + 1.0
+  result
+
+template check_accuracy2(f: untyped, rng: Slice[float], n: int, verbose = false): auto =
+
+  proc check_accuracy2: tuple[avg_ulp: float, max_ulp: int] {.gensym.} =
+    let k = (rng.b - rng.a) / (float) n
+    var
+      res, x: float
+      i, max_ulp = 0
+      avg_ulp = 0.0
+
+    x = rng.a
+    while (i < n):
+      res = f(x)
+      i.inc
+      x = x + 0.001
+    (avg_ulp, max_ulp)
+  check_accuracy2()
+
+discard check_accuracy2(exp2, -730.0..709.4, 4)
+
+# And minimized:
+macro aadMin(fn: typed): typed = fn
+
+func expMin: float
+
+func expMin: float {.aadMin.} = 1
+
+echo expMin()