From 4b9eea2fcc5d030393ca9020fbbee33fcb48d41a Mon Sep 17 00:00:00 2001 From: Clyybber Date: Tue, 22 Sep 2020 18:24:13 +0200 Subject: [PATCH] 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 | 12 ++--- compiler/procfind.nim | 12 ++++- compiler/semstmts.nim | 9 ++-- compiler/semtempl.nim | 4 +- tests/macros/tmacros_issues.nim | 77 +++++++++++++++++++++++++++++++++ 5 files changed, 101 insertions(+), 13 deletions(-) diff --git a/compiler/lookups.nim b/compiler/lookups.nim index c0db259502..744f77cf8c 100644 --- a/compiler/lookups.nim +++ b/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 shadow = c.currentScope - while result == nil and shadow.parent != nil and shadow.depthLevel == shadow.parent.depthLevel: + var scope = c.currentScope + result = strTableGet(scope.symbols, s) + while result == nil and scope.isShadowScope: # We are in a shadow scope, check in the parent too - result = strTableGet(shadow.parent.symbols, s) - shadow = shadow.parent + scope = scope.parent + result = strTableGet(scope.symbols, s) proc searchInScopes*(c: PContext, s: PIdent): PSym = for scope in walkScopes(c.currentScope): diff --git a/compiler/procfind.nim b/compiler/procfind.nim index 1d897758a5..0bdb3dae6d 100644 --- a/compiler/procfind.nim +++ b/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 diff --git a/compiler/semstmts.nim b/compiler/semstmts.nim index e99ce8937f..53d381f5db 100644 --- a/compiler/semstmts.nim +++ b/compiler/semstmts.nim @@ -1898,8 +1898,8 @@ 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 - else: searchForProc(c, oldScope, s) + 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), #if the symbol is also gensymmed we won't find it with searchForProc, so we check here @@ -1941,8 +1941,9 @@ 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) - excl(proto.flags, sfForward) - incl(proto.flags, sfWasForwarded) + if not comesFromShadowScope: + excl(proto.flags, sfForward) + incl(proto.flags, sfWasForwarded) closeScope(c) # close scope with wrong parameter symbols openScope(c) # open scope for old (correct) parameter symbols if proto.ast[genericParamsPos].kind != nkEmpty: diff --git a/compiler/semtempl.nim b/compiler/semtempl.nim index a1afd2c6d5..1ec8c07b01 100644 --- a/compiler/semtempl.nim +++ b/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) diff --git a/tests/macros/tmacros_issues.nim b/tests/macros/tmacros_issues.nim index 882c0735dc..f534298882 100644 --- a/tests/macros/tmacros_issues.nim +++ b/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()