mirrors/Nim
1
0
Fork 0
mirror of https://github.com/nim-lang/Nim.git synced 2026-01-04 12:07:51 +00:00
Code Issues Packages Projects Releases Wiki Activity

[bugfix] owned closures (#11544)

Browse Source 
(cherry picked from commit c65a5d754b)
This commit is contained in:
Andreas Rumpf
2019-06-20 07:40:45 +02:00
committed by narimiran
parent e0b41736b3
commit aacc97e854
11 changed files with 102 additions and 30 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
compiler/ast.nim
View File

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

2
compiler/ccgexprs.nim
View File

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

18
compiler/dfa.nim
View File

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

14
compiler/injectdestructors.nim
View File

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

62
compiler/lambdalifting.nim
View File

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

14
compiler/liftdestructors.nim
View File

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

3
compiler/lowerings.nim
View File

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

7
lib/core/runtime_v2.nim
View File

@@ -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.} =

2
tests/destructor/tuse_ownedref_after_move.nim
View File

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

2
tests/destructor/twidgets.nim
View File

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

6
tests/destructor/twidgets_unown.nim
View File

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