jyapayne/Nim
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Fix forward declaration issues in template/macro context (#15091)

Browse source 
* 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 #13484
This commit is contained in:
Clyybber 2020-07-29 16:17:20 +02:00 • committed by GitHub
commit 2629d619a1
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
17 changed files with 270 additions and 95 deletions
Download patch file Download diff file Expand all files Collapse all files

1
changelog.md
View file

@ -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

2
compiler/commands.nim
View file

@ -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

10
compiler/evaltempl.nim
View file

@ -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[]

3
compiler/lookups.nim
View file

@ -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

1
compiler/options.nim
View file

@ -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

45
compiler/procfind.nim
View file

@ -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

3
compiler/sem.nim
View file

@ -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)

2
compiler/semdata.nim
View file

@ -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!)

13
compiler/semexprs.nim
View file

@ -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:

6
compiler/semstmts.nim
View file

@ -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]))

27
compiler/semtempl.nim
View file

@ -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)

2
compiler/types.nim
View file

@ -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

5
compiler/vm.nim
View file

@ -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)

1
compiler/vmdef.nim
View file

@ -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

4
testament/important_packages.nim
View file

@ -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"

114
tests/macros/tmacros_issues.nim
View file

@ -35,6 +35,10 @@ test
foo1
foo2
foo3
true
false
true
false
'''
"""
@ -289,3 +293,113 @@ proc main() =
const fb = Foobar(a: a)
foobar(fb)
main()
# bug #13484
proc defForward(id, nid: NimNode): NimNode =
result = newProc(id, @[newIdentNode("bool"), newIdentDefs(nid, newIdentNode("int"))], body=newEmptyNode())
proc defEven(evenid, oddid, nid: NimNode): NimNode =
result = quote do:
proc `evenid`(`nid`: int): bool =
if `nid` == 0:
return true
else:
return `oddid`(`nid` - 1)
proc defOdd(evenid, oddid, nid: NimNode): NimNode =
result = quote do:
proc `oddid`(`nid`: int): bool =
if `nid` == 0:
return false
else:
return `evenid`(`nid` - 1)
proc callNode(funid, param: NimNode): NimNode =
result = quote do:
`funid`(`param`)
macro testEvenOdd3(): untyped =
let
evenid = newIdentNode("even3")
oddid = newIdentNode("odd3")
nid = newIdentNode("n")
oddForward = defForward(oddid, nid)
even = defEven(evenid, oddid, nid)
odd = defOdd(evenid, oddid, nid)
callEven = callNode(evenid, newLit(42))
callOdd = callNode(oddid, newLit(42))
result = quote do:
`oddForward`
`even`
`odd`
echo `callEven`
echo `callOdd`
macro testEvenOdd4(): untyped =
let
evenid = newIdentNode("even4")
oddid = newIdentNode("odd4")
nid = newIdentNode("n")
oddForward = defForward(oddid, nid)
even = defEven(evenid, oddid, nid)
odd = defOdd(evenid, oddid, nid)
callEven = callNode(evenid, newLit(42))
callOdd = callNode(oddid, newLit(42))
# rewrite the body of proc node.
oddForward[6] = newStmtList()
result = quote do:
`oddForward`
`even`
`odd`
echo `callEven`
echo `callOdd`
macro testEvenOdd5(): untyped =
let
evenid = genSym(nskProc, "even5")
oddid = genSym(nskProc, "odd5")
nid = newIdentNode("n")
oddForward = defForward(oddid, nid)
even = defEven(evenid, oddid, nid)
odd = defOdd(evenid, oddid, nid)
callEven = callNode(evenid, newLit(42))
callOdd = callNode(oddid, newLit(42))
result = quote do:
`oddForward`
`even`
`odd`
echo `callEven`
echo `callOdd`
macro testEvenOdd6(): untyped =
let
evenid = genSym(nskProc, "even6")
oddid = genSym(nskProc, "odd6")
nid = newIdentNode("n")
oddForward = defForward(oddid, nid)
even = defEven(evenid, oddid, nid)
odd = defOdd(evenid, oddid, nid)
callEven = callNode(evenid, newLit(42))
callOdd = callNode(oddid, newLit(42))
# rewrite the body of proc node.
oddForward[6] = newStmtList()
result = quote do:
`oddForward`
`even`
`odd`
echo `callEven`
echo `callOdd`
# it works
testEvenOdd3()
# it causes an error (redefinition of odd4), which is correct
assert not compiles testEvenOdd4()
# it caused an error (still forwarded: odd5)
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

126
tests/template/tparams_gensymed.nim
View file

@ -195,3 +195,129 @@ mystate_machine:
state_push(S1)
echo state_current()
state_pop()
# bug #15075
block: #Doesn't work
template genGenTempl: untyped =
proc loop(locals: int)
proc loop(locals: int) = discard
genGenTempl()
let pool = loop
block: #Doesn't work
macro genGenMacro: untyped =
quote do:
proc loop(locals: int)
proc loop(locals: int) = discard
genGenMacro()
let pool = loop
block: #This works
proc loop(locals: int)
proc loop(locals: int) = discard
let pool = loop
#Now somewhat recursive:
type Cont = ref object of RootObj
fn*: proc(c: Cont): Cont {.nimcall.}
block: #Doesn't work
template genGenTempl(): untyped =
proc loop(locals: Cont): Cont
proc loop(locals: Cont): Cont =
return Cont(fn: loop)
proc doServer(): Cont =
return Cont(fn: loop)
genGenTempl()
discard doServer()
block: #Doesn't work
macro genGenMacro(): untyped =
quote:
proc loop(locals: Cont): Cont
proc loop(locals: Cont): Cont =
return Cont(fn: loop)
proc doServer(): Cont =
return Cont(fn: loop)
genGenMacro()
discard doServer()
block: #This works
proc loop(locals: Cont): Cont
proc loop(locals: Cont): Cont =
return Cont(fn: loop)
proc doServer(): Cont =
return Cont(fn: loop)
discard doServer()
#And fully recursive:
block: #Doesn't work
template genGenTempl: untyped =
proc loop(locals: int)
proc loop(locals: int) = loop(locals)
genGenTempl()
let pool = loop
block: #Doesn't work
macro genGenMacro: untyped =
quote do:
proc loop(locals: int)
proc loop(locals: int) = loop(locals)
genGenMacro()
let pool = loop
block: #This works
proc loop(locals: int)
proc loop(locals: int) = loop(locals)
let pool = loop
block:
template genAndCallLoop: untyped =
proc loop() {.gensym.}
proc loop() {.gensym.} =
discard
loop()
genAndCallLoop
block: #Fully recursive and gensymmed:
template genGenTempl: untyped =
proc loop(locals: int) {.gensym.}
proc loop(locals: int) {.gensym.} = loop(locals)
let pool = loop
genGenTempl()
block: #Make sure gensymmed symbol doesn't overwrite the forward decl
proc loop()
proc loop() = discard
template genAndCallLoop: untyped =
proc loop() {.gensym.} =
discard
loop()
genAndCallLoop()
template genLoopDecl: untyped =
proc loop()
template genLoopDef: untyped =
proc loop() = discard
block:
genLoopDecl
genLoopDef
loop()
block:
proc loop()
genLoopDef
loop()
block:
genLoopDecl
proc loop() = discard
loop()
block: #Gensymmed sym sharing forward decl
macro genGenMacro: untyped =
let sym = genSym(nskProc, "loop")
nnkStmtList.newTree(
newProc(sym, body = newEmptyNode()),
newCall(sym),
newProc(sym, body = newStmtList()),
)
genGenMacro