diff options
-rw-r--r-- | compiler/ast.nim | 2 | ||||
-rw-r--r-- | compiler/ccgexprs.nim | 2 | ||||
-rw-r--r-- | compiler/dfa.nim | 18 | ||||
-rw-r--r-- | compiler/injectdestructors.nim | 14 | ||||
-rw-r--r-- | compiler/lambdalifting.nim | 62 | ||||
-rw-r--r-- | compiler/liftdestructors.nim | 14 | ||||
-rw-r--r-- | compiler/lowerings.nim | 3 | ||||
-rw-r--r-- | lib/core/runtime_v2.nim | 7 | ||||
-rw-r--r-- | tests/destructor/tuse_ownedref_after_move.nim | 2 | ||||
-rw-r--r-- | tests/destructor/twidgets.nim | 2 | ||||
-rw-r--r-- | tests/destructor/twidgets_unown.nim | 6 |
11 files changed, 102 insertions, 30 deletions
diff --git a/compiler/ast.nim b/compiler/ast.nim index 197b7f22d..d6ab06ec7 100644 --- a/compiler/ast.nim +++ b/compiler/ast.nim @@ -279,6 +279,8 @@ type sfNonReloadable # symbol will be left as-is when hot code reloading is on - # meaning that it won't be renamed and/or changed in any way sfGeneratedOp # proc is a generated '='; do not inject destructors in it + # variable is generated closure environment; requires early + # destruction for --newruntime. TSymFlags* = set[TSymFlag] diff --git a/compiler/ccgexprs.nim b/compiler/ccgexprs.nim index bf39788fa..4eaa5a17d 100644 --- a/compiler/ccgexprs.nim +++ b/compiler/ccgexprs.nim @@ -360,7 +360,7 @@ proc genAssignment(p: BProc, dest, src: TLoc, flags: TAssignmentFlags) = else: linefmt(p, cpsStmts, "$1 = $2;$n", [rdLoc(dest), rdLoc(src)]) of tyPtr, tyPointer, tyChar, tyBool, tyEnum, tyCString, - tyInt..tyUInt64, tyRange, tyVar, tyLent: + tyInt..tyUInt64, tyRange, tyVar, tyLent, tyNil: linefmt(p, cpsStmts, "$1 = $2;$n", [rdLoc(dest), rdLoc(src)]) else: internalError(p.config, "genAssignment: " & $ty.kind) diff --git a/compiler/dfa.nim b/compiler/dfa.nim index 755e21cb7..db12a8c66 100644 --- a/compiler/dfa.nim +++ b/compiler/dfa.nim @@ -619,8 +619,8 @@ proc aliases(obj, field: PNode): bool = break return false -proc instrTargets*(ins: Instr; loc: PNode): bool = - assert ins.kind in {def, use} +proc useInstrTargets*(ins: Instr; loc: PNode): bool = + assert ins.kind == use if ins.sym != nil and loc.kind == nkSym: result = ins.sym == loc.sym else: @@ -633,6 +633,20 @@ proc instrTargets*(ins: Instr; loc: PNode): bool = # use x; question does it affect 'x.f'? Yes. result = aliases(ins.n, loc) or aliases(loc, ins.n) +proc defInstrTargets*(ins: Instr; loc: PNode): bool = + assert ins.kind == def + if ins.sym != nil and loc.kind == nkSym: + result = ins.sym == loc.sym + else: + result = ins.n == loc or sameTrees(ins.n, loc) + if not result: + # We can come here if loc is 'x.f' and ins.n is 'x' or the other way round. + # def x.f; question: does it affect the full 'x'? No. + # def x; question: does it affect the 'x.f'? Yes. + # use x.f; question: does it affect the full 'x'? No. + # use x; question does it affect 'x.f'? Yes. + result = aliases(ins.n, loc) + proc isAnalysableFieldAccess*(orig: PNode; owner: PSym): bool = var n = orig while true: diff --git a/compiler/injectdestructors.nim b/compiler/injectdestructors.nim index b1a5e63c0..b9fa1e3ae 100644 --- a/compiler/injectdestructors.nim +++ b/compiler/injectdestructors.nim @@ -165,12 +165,12 @@ proc isLastRead(location: PNode; c: var Con; pc, comesFrom: int): int = while pc < c.g.len: case c.g[pc].kind of def: - if instrTargets(c.g[pc], location): + if defInstrTargets(c.g[pc], location): # the path lead to a redefinition of 's' --> abandon it. return high(int) inc pc of use: - if instrTargets(c.g[pc], location): + if useInstrTargets(c.g[pc], location): c.otherRead = c.g[pc].n return -1 inc pc @@ -349,8 +349,10 @@ proc genSink(c: Con; t: PType; dest, ri: PNode): PNode = # we generate a fast assignment in this case: result = newTree(nkFastAsgn, dest) -proc genCopy(c: Con; t: PType; dest, ri: PNode): PNode = +proc genCopy(c: var Con; t: PType; dest, ri: PNode): PNode = if tfHasOwned in t.flags: + # try to improve the error message here: + if c.otherRead == nil: discard isLastRead(ri, c) checkForErrorPragma(c, t, ri, "=") let t = t.skipTypes({tyGenericInst, tyAlias, tySink}) result = genOp(c, t, attachedAsgn, dest, ri) @@ -410,7 +412,7 @@ proc destructiveMoveVar(n: PNode; c: var Con): PNode = add(v, vpart) result.add v - result.add genWasMoved(n, c) + result.add genWasMoved(skipConv(n), c) result.add tempAsNode proc sinkParamIsLastReadCheck(c: var Con, s: PNode) = @@ -611,7 +613,7 @@ proc moveOrCopy(dest, ri: PNode; c: var Con): PNode = of nkTupleConstr, nkClosure: result = genSink(c, dest.typ, dest, ri) let ri2 = copyTree(ri) - for i in 0..<ri.len: + for i in ord(ri.kind == nkClosure)..<ri.len: # everything that is passed to an tuple constructor is consumed, # so these all act like 'sink' parameters: if ri[i].kind == nkExprColonExpr: @@ -757,7 +759,7 @@ proc p(n: PNode; c: var Con): PNode = else: result = n of nkAsgn, nkFastAsgn: - if hasDestructor(n[0].typ) and n[1].kind notin {nkProcDef, nkDo, nkLambda, nkClosure}: + if hasDestructor(n[0].typ) and n[1].kind notin {nkProcDef, nkDo, nkLambda}: result = moveOrCopy(n[0], n[1], c) else: result = copyNode(n) diff --git a/compiler/lambdalifting.nim b/compiler/lambdalifting.nim index 9ea6d4f02..c3dee5d1e 100644 --- a/compiler/lambdalifting.nim +++ b/compiler/lambdalifting.nim @@ -324,6 +324,7 @@ proc asOwnedRef(c: DetectionPass; t: PType): PType = if optNimV2 in c.graph.config.globalOptions: assert t.kind == tyRef result = newType(tyOwned, t.owner) + result.flags.incl tfHasOwned result.rawAddSon t else: result = t @@ -490,6 +491,7 @@ type processed: IntSet envVars: Table[int, PNode] inContainer: int + unownedEnvVars: Table[int, PNode] # only required for --newruntime proc initLiftingPass(fn: PSym): LiftingPass = result.processed = initIntSet() @@ -514,9 +516,9 @@ proc accessViaEnvParam(g: ModuleGraph; n: PNode; owner: PSym): PNode = localError(g.config, n.info, "internal error: environment misses: " & s.name.s) result = n -proc newEnvVar(cache: IdentCache; owner: PSym; typ: PType): PNode = - var v = newSym(skVar, getIdent(cache, envName), owner, owner.info) - incl(v.flags, sfShadowed) +proc newEnvVar(cache: IdentCache; owner: PSym; typ: PType; info: TLineInfo): PNode = + var v = newSym(skVar, getIdent(cache, envName), owner, info) + v.flags = {sfShadowed, sfGeneratedOp} v.typ = typ result = newSymNode(v) when false: @@ -528,7 +530,7 @@ proc newEnvVar(cache: IdentCache; owner: PSym; typ: PType): PNode = result = newSymNode(v) proc setupEnvVar(owner: PSym; d: DetectionPass; - c: var LiftingPass): PNode = + c: var LiftingPass; info: TLineInfo): PNode = if owner.isIterator: return getHiddenParam(d.graph, owner).newSymNode result = c.envvars.getOrDefault(owner.id) @@ -536,8 +538,13 @@ proc setupEnvVar(owner: PSym; d: DetectionPass; let envVarType = d.ownerToType.getOrDefault(owner.id) if envVarType.isNil: localError d.graph.config, owner.info, "internal error: could not determine closure type" - result = newEnvVar(d.graph.cache, owner, asOwnedRef(d, envVarType)) + result = newEnvVar(d.graph.cache, owner, asOwnedRef(d, envVarType), info) c.envVars[owner.id] = result + if optNimV2 in d.graph.config.globalOptions: + var v = newSym(skVar, getIdent(d.graph.cache, envName & "Alt"), owner, info) + v.flags = {sfShadowed, sfGeneratedOp} + v.typ = envVarType + c.unownedEnvVars[owner.id] = newSymNode(v) proc getUpViaParam(g: ModuleGraph; owner: PSym): PNode = let p = getHiddenParam(g, owner) @@ -550,20 +557,34 @@ proc getUpViaParam(g: ModuleGraph; owner: PSym): PNode = result = rawIndirectAccess(result, upField, p.info) proc rawClosureCreation(owner: PSym; - d: DetectionPass; c: var LiftingPass): PNode = + d: DetectionPass; c: var LiftingPass; + info: TLineInfo): PNode = result = newNodeI(nkStmtList, owner.info) var env: PNode if owner.isIterator: env = getHiddenParam(d.graph, owner).newSymNode else: - env = setupEnvVar(owner, d, c) + env = setupEnvVar(owner, d, c, info) if env.kind == nkSym: var v = newNodeI(nkVarSection, env.info) addVar(v, env) result.add(v) + if optNimV2 in d.graph.config.globalOptions: + let unowned = c.unownedEnvVars[owner.id] + assert unowned != nil + addVar(v, unowned) + # add 'new' statement: result.add(newCall(getSysSym(d.graph, env.info, "internalNew"), env)) + if optNimV2 in d.graph.config.globalOptions: + let unowned = c.unownedEnvVars[owner.id] + assert unowned != nil + let env2 = copyTree(env) + env2.typ = unowned.typ + result.add newAsgnStmt(unowned, env2, env.info) + createTypeBoundOps(d.graph, nil, unowned.typ, env.info) + # add assignment statements for captured parameters: for i in 1..<owner.typ.n.len: let local = owner.typ.n[i].sym @@ -585,9 +606,16 @@ proc rawClosureCreation(owner: PSym; localError(d.graph.config, env.info, "internal error: cannot create up reference") # we are not in the sem'check phase anymore! so pass 'nil' for the PContext # and hope for the best: - when false: - if optNimV2 in d.graph.config.globalOptions: - createTypeBoundOps(d.graph, nil, env.typ, owner.info) + if optNimV2 in d.graph.config.globalOptions: + createTypeBoundOps(d.graph, nil, env.typ, owner.info) + +proc finishClosureCreation(owner: PSym; d: DetectionPass; c: LiftingPass; + info: TLineInfo; res: PNode) = + if optNimV2 in d.graph.config.globalOptions: + let unowned = c.unownedEnvVars[owner.id] + assert unowned != nil + let nilLit = newNodeIT(nkNilLit, info, unowned.typ) + res.add newAsgnStmt(unowned, nilLit, info) proc closureCreationForIter(iter: PNode; d: DetectionPass; c: var LiftingPass): PNode = @@ -610,7 +638,7 @@ proc closureCreationForIter(iter: PNode; let upField = lookupInRecord(v.typ.skipTypes({tyOwned, tyRef}).n, getIdent(d.graph.cache, upName)) if upField != nil: - let u = setupEnvVar(owner, d, c) + let u = setupEnvVar(owner, d, c, iter.info) if u.typ.skipTypes({tyOwned, tyRef}) == upField.typ.skipTypes({tyOwned, tyRef}): result.add(newAsgnStmt(rawIndirectAccess(vnode, upField, iter.info), u, iter.info)) @@ -620,7 +648,9 @@ proc closureCreationForIter(iter: PNode; proc accessViaEnvVar(n: PNode; owner: PSym; d: DetectionPass; c: var LiftingPass): PNode = - let access = setupEnvVar(owner, d, c) + var access = setupEnvVar(owner, d, c, n.info) + if optNimV2 in d.graph.config.globalOptions: + access = c.unownedEnvVars[owner.id] let obj = access.typ.skipTypes({tyOwned, tyRef}) let field = getFieldFromObj(obj, n.sym) if field != nil: @@ -646,7 +676,7 @@ proc symToClosure(n: PNode; owner: PSym; d: DetectionPass; result = closureCreationForIter(n, d, c) elif s.skipGenericOwner == owner: # direct dependency, so use the outer's env variable: - result = makeClosure(d.graph, s, setupEnvVar(owner, d, c), n.info) + result = makeClosure(d.graph, s, setupEnvVar(owner, d, c, n.info), n.info) else: let available = getHiddenParam(d.graph, owner) let wanted = getHiddenParam(d.graph, s).typ @@ -679,7 +709,8 @@ proc liftCapturedVars(n: PNode; owner: PSym; d: DetectionPass; if c.envvars.getOrDefault(s.id).isNil: s.transformedBody = body else: - s.transformedBody = newTree(nkStmtList, rawClosureCreation(s, d, c), body) + s.transformedBody = newTree(nkStmtList, rawClosureCreation(s, d, c, n.info), body) + finishClosureCreation(s, d, c, n.info, s.transformedBody) c.inContainer = oldInContainer if s.typ.callConv == ccClosure: @@ -816,7 +847,8 @@ proc liftLambdas*(g: ModuleGraph; fn: PSym, body: PNode; tooEarly: var bool; result = liftCapturedVars(body, fn, d, c) # echo renderTree(result, {renderIds}) if c.envvars.getOrDefault(fn.id) != nil: - result = newTree(nkStmtList, rawClosureCreation(fn, d, c), result) + result = newTree(nkStmtList, rawClosureCreation(fn, d, c, body.info), result) + finishClosureCreation(fn, d, c, body.info, result) else: result = body #if fn.name.s == "get2": diff --git a/compiler/liftdestructors.nim b/compiler/liftdestructors.nim index 8f688d732..026a28893 100644 --- a/compiler/liftdestructors.nim +++ b/compiler/liftdestructors.nim @@ -363,7 +363,13 @@ proc weakrefOp(c: var TLiftCtx; t: PType; body, x, y: PNode) = body.add genIf(c, x, callCodegenProc(c.g, "nimDecWeakRef", c.info, x)) body.add newAsgnStmt(x, y) of attachedDestructor: - body.add genIf(c, x, callCodegenProc(c.g, "nimDecWeakRef", c.info, x)) + # it's better to prepend the destruction of weak refs in order to + # prevent wrong "dangling refs exist" problems: + let des = genIf(c, x, callCodegenProc(c.g, "nimDecWeakRef", c.info, x)) + if body.len == 0: + body.add des + else: + body.sons.insert(des, 0) of attachedDeepCopy: assert(false, "cannot happen") proc ownedRefOp(c: var TLiftCtx; t: PType; body, x, y: PNode) = @@ -413,7 +419,11 @@ proc closureOp(c: var TLiftCtx; t: PType; body, x, y: PNode) = body.add genIf(c, xx, callCodegenProc(c.g, "nimDecWeakRef", c.info, xx)) body.add newAsgnStmt(x, y) of attachedDestructor: - body.add genIf(c, xx, callCodegenProc(c.g, "nimDecWeakRef", c.info, xx)) + let des = genIf(c, xx, callCodegenProc(c.g, "nimDecWeakRef", c.info, xx)) + if body.len == 0: + body.add des + else: + body.sons.insert(des, 0) of attachedDeepCopy: assert(false, "cannot happen") proc ownedClosureOp(c: var TLiftCtx; t: PType; body, x, y: PNode) = diff --git a/compiler/lowerings.nim b/compiler/lowerings.nim index b6a8d0e48..55917a2f5 100644 --- a/compiler/lowerings.nim +++ b/compiler/lowerings.nim @@ -155,6 +155,7 @@ proc rawAddField*(obj: PType; field: PSym) = assert field.kind == skField field.position = sonsLen(obj.n) addSon(obj.n, newSymNode(field)) + propagateToOwner(obj, field.typ) proc rawIndirectAccess*(a: PNode; field: PSym; info: TLineInfo): PNode = # returns a[].field as a node @@ -205,6 +206,7 @@ proc addField*(obj: PType; s: PSym; cache: IdentCache) = let t = skipIntLit(s.typ) field.typ = t assert t.kind != tyTyped + propagateToOwner(obj, t) field.position = sonsLen(obj.n) addSon(obj.n, newSymNode(field)) @@ -217,6 +219,7 @@ proc addUniqueField*(obj: PType; s: PSym; cache: IdentCache): PSym {.discardable let t = skipIntLit(s.typ) field.typ = t assert t.kind != tyTyped + propagateToOwner(obj, t) field.position = sonsLen(obj.n) addSon(obj.n, newSymNode(field)) result = field diff --git a/lib/core/runtime_v2.nim b/lib/core/runtime_v2.nim index 0165833b4..5689e3072 100644 --- a/lib/core/runtime_v2.nim +++ b/lib/core/runtime_v2.nim @@ -78,6 +78,13 @@ proc nimRawDispose(p: pointer) {.compilerRtl.} = proc nimDestroyAndDispose(p: pointer) {.compilerRtl.} = let d = cast[ptr PNimType](p)[].destructor if d != nil: cast[DestructorProc](d)(p) + when false: + cstderr.rawWrite cast[ptr PNimType](p)[].name + cstderr.rawWrite "\n" + if d == nil: + cstderr.rawWrite "bah, nil\n" + else: + cstderr.rawWrite "has destructor!\n" nimRawDispose(p) proc isObj(obj: PNimType, subclass: cstring): bool {.compilerproc.} = diff --git a/tests/destructor/tuse_ownedref_after_move.nim b/tests/destructor/tuse_ownedref_after_move.nim index 148696ee2..34b2f92e4 100644 --- a/tests/destructor/tuse_ownedref_after_move.nim +++ b/tests/destructor/tuse_ownedref_after_move.nim @@ -1,6 +1,6 @@ discard """ cmd: '''nim c --newruntime $file''' - errormsg: "'=' is not available for type <owned Widget>; requires a copy because it's not the last read of ':env.b1()'; another read is done here: tuse_ownedref_after_move.nim(53, 4)" + errormsg: "'=' is not available for type <owned Widget>; requires a copy because it's not the last read of ':envAlt.b1()'; another read is done here: tuse_ownedref_after_move.nim(53, 4)" line: 49 """ diff --git a/tests/destructor/twidgets.nim b/tests/destructor/twidgets.nim index 64fe8ab96..1d75a803d 100644 --- a/tests/destructor/twidgets.nim +++ b/tests/destructor/twidgets.nim @@ -63,7 +63,7 @@ proc main = var b = newButton("button", nil) let u: Button = b b.onclick = proc () = - b.caption = "clicked!" + u.caption = "clicked!" w.add b w.draw() diff --git a/tests/destructor/twidgets_unown.nim b/tests/destructor/twidgets_unown.nim index 5e53a0e5b..39d3c46df 100644 --- a/tests/destructor/twidgets_unown.nim +++ b/tests/destructor/twidgets_unown.nim @@ -2,7 +2,7 @@ discard """ cmd: '''nim c --newruntime $file''' output: '''button clicked! -3 3 alloc/dealloc pairs: 0''' +6 6 alloc/dealloc pairs: 0''' """ import core / allocators @@ -52,8 +52,10 @@ proc main = var b = newButton("button", nil) let u = unown b + var clicked = "clicked" b.onclick = proc () = - b.caption = "clicked!" + clicked.add "!" + u.caption = clicked w.add b w.draw() |