From 4b31d7bb83e2cfdeaab2500f83d0b6728310b549 Mon Sep 17 00:00:00 2001 From: Andrii Riabushenko Date: Thu, 29 Nov 2018 23:33:48 +0000 Subject: [PATCH 01/26] move moves --- compiler/destroyer.nim | 191 ++++++++++++++++++--------- tests/destructor/tmove_objconstr.nim | 33 ++++- 2 files changed, 161 insertions(+), 63 deletions(-) diff --git a/compiler/destroyer.nim b/compiler/destroyer.nim index 2d21a6019..8471d70d1 100644 --- a/compiler/destroyer.nim +++ b/compiler/destroyer.nim @@ -408,9 +408,12 @@ proc passCopyToSink(n: PNode; c: var Con): PNode = else: result.add newTree(nkAsgn, tmp, p(n, c)) result.add tmp - + proc pArg(arg: PNode; c: var Con; isSink: bool): PNode = - if isSink: + if arg.typ == nil: + # typ is nil if we are in if/case branch with noreturn + result = copyTree(arg) + elif isSink: if arg.kind in nkCallKinds: # recurse but skip the call expression in order to prevent # destructor injections: Rule 5.1 is different from rule 5.4! @@ -420,7 +423,7 @@ proc pArg(arg: PNode; c: var Con; isSink: bool): PNode = result.add arg[0] for i in 1.. 5: raise newException(ValueError, "new error") + else: newMySeq(x, 1.0), + b:0, + c: if y > 0: move(cc) else: newMySeq(1, 3.0)) let (seq1, seq2) = myfunc(2, 3) doAssert seq1.len == 2 @@ -122,4 +129,26 @@ doAssert seq3.len == 2 doAssert seq3[0] == 1.0 var seq4, seq5: MySeqNonCopyable -(seq4, i, seq5) = myfunc2(2, 3) \ No newline at end of file +(seq4, i, seq5) = myfunc2(2, 3) + +seq4 = block: + var tmp = newMySeq(4, 1.0) + tmp[0] = 3.0 + tmp + +doAssert seq4[0] == 3.0 + +import macros + +seq4 = + if i > 0: newMySeq(2, 5.0) + elif i < -100: raise newException(ValueError, "Parse Error") + else: newMySeq(2, 3.0) + +seq4 = + case (char) i: + of 'A', {'W'..'Z'}: newMySeq(2, 5.0) + of 'B': quit(-1) + else: + let (x1, x2, x3) = myfunc2(2, 3) + x3 \ No newline at end of file From 9bba790534be76c6b50033d54d061eb076f35d9d Mon Sep 17 00:00:00 2001 From: Andrii Riabushenko Date: Thu, 29 Nov 2018 23:36:06 +0000 Subject: [PATCH 02/26] fix spacing --- compiler/destroyer.nim | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/destroyer.nim b/compiler/destroyer.nim index 8471d70d1..b3ec6d510 100644 --- a/compiler/destroyer.nim +++ b/compiler/destroyer.nim @@ -408,7 +408,7 @@ proc passCopyToSink(n: PNode; c: var Con): PNode = else: result.add newTree(nkAsgn, tmp, p(n, c)) result.add tmp - + proc pArg(arg: PNode; c: var Con; isSink: bool): PNode = if arg.typ == nil: # typ is nil if we are in if/case branch with noreturn From c0b91bc02457158505a690e1e7bf87b185484f4d Mon Sep 17 00:00:00 2001 From: Andrii Riabushenko Date: Thu, 29 Nov 2018 23:43:19 +0000 Subject: [PATCH 03/26] revert debug statements --- compiler/destroyer.nim | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/compiler/destroyer.nim b/compiler/destroyer.nim index ca17057b5..b81f6c0fc 100644 --- a/compiler/destroyer.nim +++ b/compiler/destroyer.nim @@ -632,7 +632,7 @@ proc p(n: PNode; c: var Con): PNode = recurse(n, result) proc injectDestructorCalls*(g: ModuleGraph; owner: PSym; n: PNode): PNode = - when true: # defined(nimDebugDestroys): + when false: # defined(nimDebugDestroys): echo "injecting into ", n var c: Con c.owner = owner @@ -668,7 +668,8 @@ proc injectDestructorCalls*(g: ModuleGraph; owner: PSym; n: PNode): PNode = else: result.add body - when true: - echo "------------------------------------" - echo owner.name.s, " transformed to: " - echo result + when defined(nimDebugDestroys): + if true: + echo "------------------------------------" + echo owner.name.s, " transformed to: " + echo result From e90f70af423caa6ffa9b5dd3c59b38c85a9a3f03 Mon Sep 17 00:00:00 2001 From: Andrii Riabushenko Date: Fri, 30 Nov 2018 09:36:04 +0000 Subject: [PATCH 04/26] Improve approach --- compiler/destroyer.nim | 45 +++++++++++++++++++++--------------------- 1 file changed, 23 insertions(+), 22 deletions(-) diff --git a/compiler/destroyer.nim b/compiler/destroyer.nim index b81f6c0fc..ee930018a 100644 --- a/compiler/destroyer.nim +++ b/compiler/destroyer.nim @@ -411,10 +411,12 @@ proc passCopyToSink(n: PNode; c: var Con): PNode = result.add tmp proc pArg(arg: PNode; c: var Con; isSink: bool): PNode = - if arg.typ == nil: - # typ is nil if we are in if/case branch with noreturn - result = copyTree(arg) - elif isSink: + template pArgIfTyped(arg_part: PNode): PNode = + # typ is nil if we are in if/case expr branch with noreturn + if arg_part.typ == nil: copyTree(arg_part) + else: pArg(arg_part, c, isSink) + + if isSink: if arg.kind in nkCallKinds: # recurse but skip the call expression in order to prevent # destructor injections: Rule 5.1 is different from rule 5.4! @@ -446,9 +448,9 @@ proc pArg(arg: PNode; c: var Con; isSink: bool): PNode = var branch = copyNode(arg[i]) if arg[i].kind in {nkElifBranch, nkElifExpr}: branch.add p(arg[i][0], c) - branch.add pArg(arg[i][1], c, isSink) + branch.add pArgIfTyped(arg[i][1]) else: - branch.add pArg(arg[i][0], c, isSink) + branch.add pArgIfTyped(arg[i][0]) result.add branch elif arg.kind == nkCaseStmt: result = copyNode(arg) @@ -457,14 +459,14 @@ proc pArg(arg: PNode; c: var Con; isSink: bool): PNode = var branch: PNode if arg[i].kind == nkOfbranch: branch = arg[i] # of branch conditions are constants - branch[^1] = pArg(arg[i][^1], c, isSink) + branch[^1] = pArgIfTyped(arg[i][^1]) elif arg[i].kind in {nkElifBranch, nkElifExpr}: branch = copyNode(arg[i]) branch.add p(arg[i][0], c) - branch.add pArg(arg[i][1], c, isSink) + branch.add pArgIfTyped(arg[i][1]) else: branch = copyNode(arg[i]) - branch.add pArg(arg[i][0], c, isSink) + branch.add pArgIfTyped(arg[i][0]) result.add branch else: # an object that is not temporary but passed to a 'sink' parameter @@ -474,11 +476,12 @@ proc pArg(arg: PNode; c: var Con; isSink: bool): PNode = result = p(arg, c) proc moveOrCopy(dest, ri: PNode; c: var Con): PNode = - if ri.typ == nil: - # typ is nil if we are in if/case branch with noreturn - result = copyTree(ri) - else: - case ri.kind + template moveOrCopyIfTyped(ri_part: PNode): PNode = + # typ is nil if we are in if/case expr branch with noreturn + if ri_part.typ == nil: copyTree(ri_part) + else: moveOrCopy(dest, ri_part, c) + + case ri.kind of nkCallKinds: result = genSink(c, dest.typ, dest, ri) # watch out and no not transform 'ri' twice if it's a call: @@ -505,18 +508,16 @@ proc moveOrCopy(dest, ri: PNode; c: var Con): PNode = of nkBlockExpr, nkBlockStmt: result = newNodeI(nkBlockStmt, ri.info) result.add ri[0] ## add label - for i in 1..ri.len-2: - result.add p(ri[i], c) - result.add moveOrCopy(dest, ri[^1], c) + result.add moveOrCopy(dest, ri[1], c) of nkIfExpr, nkIfStmt: result = newNodeI(nkIfStmt, ri.info) for i in 0.. Date: Fri, 30 Nov 2018 09:37:26 +0000 Subject: [PATCH 05/26] reduce changes --- compiler/destroyer.nim | 160 ++++++++++++++++++++--------------------- 1 file changed, 80 insertions(+), 80 deletions(-) diff --git a/compiler/destroyer.nim b/compiler/destroyer.nim index ee930018a..a4bed4674 100644 --- a/compiler/destroyer.nim +++ b/compiler/destroyer.nim @@ -482,93 +482,93 @@ proc moveOrCopy(dest, ri: PNode; c: var Con): PNode = else: moveOrCopy(dest, ri_part, c) case ri.kind - of nkCallKinds: + of nkCallKinds: + result = genSink(c, dest.typ, dest, ri) + # watch out and no not transform 'ri' twice if it's a call: + let ri2 = copyNode(ri) + let parameters = ri[0].typ + let L = if parameters != nil: parameters.len else: 0 + ri2.add ri[0] + for i in 1.. Date: Fri, 30 Nov 2018 09:47:55 +0000 Subject: [PATCH 06/26] add array constructors --- compiler/destroyer.nim | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/compiler/destroyer.nim b/compiler/destroyer.nim index a4bed4674..a26a4d379 100644 --- a/compiler/destroyer.nim +++ b/compiler/destroyer.nim @@ -426,7 +426,7 @@ proc pArg(arg: PNode; c: var Con; isSink: bool): PNode = result.add arg[0] for i in 1.. Date: Wed, 5 Dec 2018 19:29:14 +0000 Subject: [PATCH 07/26] add test --- compiler/destroyer.nim | 33 ++++++++++++++++------------ tests/destructor/tmove_objconstr.nim | 11 +++++++++- 2 files changed, 29 insertions(+), 15 deletions(-) diff --git a/compiler/destroyer.nim b/compiler/destroyer.nim index a26a4d379..352b4aaad 100644 --- a/compiler/destroyer.nim +++ b/compiler/destroyer.nim @@ -413,7 +413,7 @@ proc passCopyToSink(n: PNode; c: var Con): PNode = proc pArg(arg: PNode; c: var Con; isSink: bool): PNode = template pArgIfTyped(arg_part: PNode): PNode = # typ is nil if we are in if/case expr branch with noreturn - if arg_part.typ == nil: copyTree(arg_part) + if arg_part.typ == nil: p(arg_part, c) else: pArg(arg_part, c, isSink) if isSink: @@ -478,7 +478,7 @@ proc pArg(arg: PNode; c: var Con; isSink: bool): PNode = proc moveOrCopy(dest, ri: PNode; c: var Con): PNode = template moveOrCopyIfTyped(ri_part: PNode): PNode = # typ is nil if we are in if/case expr branch with noreturn - if ri_part.typ == nil: copyTree(ri_part) + if ri_part.typ == nil: p(ri_part, c) else: moveOrCopy(dest, ri_part, c) case ri.kind @@ -537,29 +537,32 @@ proc moveOrCopy(dest, ri: PNode; c: var Con): PNode = result.add branch of nkBracket: # array constructor + result = genSink(c, dest.typ, dest, ri) let ri2 = copyTree(ri) for i in 0.. 1: newMySeq(3, 1.0) else: newMySeq(0, 0.0)] +var seqOfSeq2 = @[newMySeq(2, 5.0), newMySeq(3, 1.0)] \ No newline at end of file From 69347f6c95b35e8969cf650b59644b20ef1528de Mon Sep 17 00:00:00 2001 From: Andrii Riabushenko Date: Wed, 5 Dec 2018 21:33:15 +0000 Subject: [PATCH 08/26] implement everything --- compiler/ast.nim | 2 +- compiler/ccgexprs.nim | 8 --- compiler/destroyer.nim | 101 ++++++++++++++------------------ compiler/semexprs.nim | 3 +- lib/pure/collections/tables.nim | 46 +++++++++++++++ 5 files changed, 93 insertions(+), 67 deletions(-) diff --git a/compiler/ast.nim b/compiler/ast.nim index 7cf35450b..2b595aee1 100644 --- a/compiler/ast.nim +++ b/compiler/ast.nim @@ -627,7 +627,7 @@ type mIsPartOf, mAstToStr, mParallel, mSwap, mIsNil, mArrToSeq, mCopyStr, mCopyStrLast, mNewString, mNewStringOfCap, mParseBiggestFloat, - mMove, mWasMoved, mDestroy, + mMove, mDestroy, mReset, mArray, mOpenArray, mRange, mSet, mSeq, mOpt, mVarargs, mRef, mPtr, mVar, mDistinct, mVoid, mTuple, diff --git a/compiler/ccgexprs.nim b/compiler/ccgexprs.nim index 34836d843..bf1494b3d 100644 --- a/compiler/ccgexprs.nim +++ b/compiler/ccgexprs.nim @@ -1901,13 +1901,6 @@ proc binaryFloatArith(p: BProc, e: PNode, d: var TLoc, m: TMagic) = proc skipAddr(n: PNode): PNode = result = if n.kind in {nkAddr, nkHiddenAddr}: n[0] else: n -proc genWasMoved(p: BProc; n: PNode) = - var a: TLoc - initLocExpr(p, n[1].skipAddr, a) - resetLoc(p, a) - #linefmt(p, cpsStmts, "#nimZeroMem((void*)$1, sizeof($2));$n", - # addrLoc(p.config, a), getTypeDesc(p.module, a.t)) - proc genMove(p: BProc; n: PNode; d: var TLoc) = if d.k == locNone: getTemp(p, n.typ, d) var a: TLoc @@ -2046,7 +2039,6 @@ proc genMagicExpr(p: BProc, e: PNode, d: var TLoc, op: TMagic) = initLocExpr(p, e.sons[2], b) genDeepCopy(p, a, b) of mDotDot, mEqCString: genCall(p, e, d) - of mWasMoved: genWasMoved(p, e) of mMove: genMove(p, e, d) of mDestroy: discard "ignore calls to the default destructor" of mSlice: diff --git a/compiler/destroyer.nim b/compiler/destroyer.nim index 352b4aaad..4703c31dd 100644 --- a/compiler/destroyer.nim +++ b/compiler/destroyer.nim @@ -100,12 +100,12 @@ Rule Pattern Transformed into finally: `=destroy`(x) 1.2 var x: sink T; stmts var x: sink T; stmts; ensureEmpty(x) 2 x = f() `=sink`(x, f()) -3 x = lastReadOf z `=sink`(x, z); wasMoved(z) +3 x = lastReadOf z `=sink`(x, z); 4.1 y = sinkParam `=sink`(y, sinkParam) 4.2 x = y `=`(x, y) # a copy 5.1 f_sink(g()) f_sink(g()) 5.2 f_sink(y) f_sink(copy y); # copy unless we can see it's the last read -5.3 f_sink(move y) f_sink(y); wasMoved(y) # explicit moves empties 'y' +5.3 f_sink(move y) f_sink(y); # explicit moves empties 'y' 5.4 f_noSink(g()) var tmp = bitwiseCopy(g()); f(tmp); `=destroy`(tmp) Remarks: Rule 1.2 is not yet implemented because ``sink`` is currently @@ -116,7 +116,7 @@ Remarks: Rule 1.2 is not yet implemented because ``sink`` is currently import intsets, ast, astalgo, msgs, renderer, magicsys, types, idents, trees, - strutils, options, dfa, lowerings, tables, modulegraphs, + strutils, options, dfa, lowerings, tables, modulegraphs, msgs, lineinfos, parampatterns const @@ -127,20 +127,13 @@ type owner: PSym g: ControlFlowGraph jumpTargets: IntSet - tmpObj: PType - tmp: PSym - destroys, topLevelVars: PNode + topLevelVars: PNode + destroys: OrderedTable[int, tuple[enabled: bool, destroy_call: PNode]] # Symbol to destructor table toDropBit: Table[int, PSym] graph: ModuleGraph emptyNode: PNode otherRead: PNode -proc getTemp(c: var Con; typ: PType; info: TLineInfo): PNode = - # XXX why are temps fields in an object here? - let f = newSym(skField, getIdent(c.graph.cache, ":d" & $c.tmpObj.n.len), c.owner, info) - f.typ = typ - rawAddField c.tmpObj, f - result = rawDirectAccess(c.tmp, f) proc isHarmlessVar*(s: PSym; c: Con): bool = # 's' is harmless if it used only once and its @@ -333,6 +326,22 @@ proc dropBit(c: var Con; s: PSym): PSym = result = c.toDropBit.getOrDefault(s.id) assert result != nil +proc addDestructor(c: var Con; s: PSym; destructor_call: PNode) = + let alreadyIn = c.destroys.hasKeyOrPut(s.id, (true, destructor_call)) + if alreadyIn: + let lineInfo = if s.ast != nil: s.ast.info else: c.owner.info + internalError(c.graph.config, lineInfo, "Destructor call for sym " & s.name.s & " is already injected") + +proc disableDestructor(c: var Con; s: PSym) = + ## disable destructor, but do not delete such that it can be enabled back again later + c.destroys.with_value(s.id, value): + value.enabled = false + +proc enableDestructor(c: var Con; s: PSym) = + ## if destructor does not exist then ignore, otherwise make sure destructor is enabled + c.destroys.with_value(s.id, value): + value.enabled = true + proc registerDropBit(c: var Con; s: PSym) = let result = newSym(skTemp, getIdent(c.graph.cache, s.name.s & "_AliveBit"), c.owner, s.info) result.typ = getSysType(c.graph, s.info, tyBool) @@ -343,8 +352,14 @@ proc registerDropBit(c: var Con; s: PSym) = # if not sinkParam_AliveBit: `=destroy`(sinkParam) let t = s.typ.skipTypes({tyGenericInst, tyAlias, tySink}) if t.destructor != nil: - c.destroys.add newTree(nkIfStmt, - newTree(nkElifBranch, newSymNode result, genDestroy(c, t, newSymNode s))) + c.addDestructor(s, newTree(nkIfStmt, + newTree(nkElifBranch, newSymNode result, genDestroy(c, t, newSymNode s)))) + +proc getTemp(c: var Con; typ: PType; info: TLineInfo): PNode = + let sym = newSym(skTemp, getIdent(c.graph.cache, ":tmpD"), c.owner, info) + sym.typ = typ + result = newSymNode(sym) + c.addTopVar(result) proc p(n: PNode; c: var Con): PNode @@ -370,31 +385,6 @@ proc genMagicCall(n: PNode; c: var Con; magicname: string; m: TMagic): PNode = result.add(newSymNode(createMagic(c.graph, magicname, m))) result.add n -proc genWasMoved(n: PNode; c: var Con): PNode = - # The mWasMoved builtin does not take the address. - result = genMagicCall(n, c, "wasMoved", mWasMoved) - -proc destructiveMoveVar(n: PNode; c: var Con): PNode = - # generate: (let tmp = v; reset(v); tmp) - # XXX: Strictly speaking we can only move if there is a ``=sink`` defined - # or if no ``=sink`` is defined and also no assignment. - result = newNodeIT(nkStmtListExpr, n.info, n.typ) - - var temp = newSym(skLet, getIdent(c.graph.cache, "blitTmp"), c.owner, n.info) - temp.typ = n.typ - var v = newNodeI(nkLetSection, n.info) - let tempAsNode = newSymNode(temp) - - var vpart = newNodeI(nkIdentDefs, tempAsNode.info, 3) - vpart.sons[0] = tempAsNode - vpart.sons[1] = c.emptyNode - vpart.sons[2] = n - add(v, vpart) - - result.add v - result.add genWasMoved(n, c) - result.add tempAsNode - proc passCopyToSink(n: PNode; c: var Con): PNode = result = newNodeIT(nkStmtListExpr, n.info, n.typ) let tmp = getTemp(c, n.typ, n.info) @@ -415,7 +405,7 @@ proc pArg(arg: PNode; c: var Con; isSink: bool): PNode = # typ is nil if we are in if/case expr branch with noreturn if arg_part.typ == nil: p(arg_part, c) else: pArg(arg_part, c, isSink) - + if isSink: if arg.kind in nkCallKinds: # recurse but skip the call expression in order to prevent @@ -431,9 +421,9 @@ proc pArg(arg: PNode; c: var Con; isSink: bool): PNode = result = arg elif arg.kind == nkSym and arg.sym.kind in InterestingSyms and isLastRead(arg, c): # if x is a variable and it its last read we eliminate its - # destructor invokation, but don't. We need to reset its memory - # to disable its destructor which we have not elided: - result = destructiveMoveVar(arg, c) + # destructor invocation + c.disableDestructor(arg.sym) + result = arg elif arg.kind == nkSym and isSinkParam(arg.sym): # mark the sink parameter as used: result = destructiveMoveSink(arg, c) @@ -566,9 +556,8 @@ proc moveOrCopy(dest, ri: PNode; c: var Con): PNode = of nkSym: if ri.sym.kind != skParam and isLastRead(ri, c): # Rule 3: `=sink`(x, z); wasMoved(z) - var snk = genSink(c, dest.typ, dest, ri) - snk.add p(ri, c) - result = newTree(nkStmtList, snk, genMagicCall(ri, c, "wasMoved", mWasMoved)) + result = genSink(c, dest.typ, dest, ri) + result.add p(ri, c) elif isSinkParam(ri.sym): result = genSink(c, dest.typ, dest, ri) result.add destructiveMoveSink(ri, c) @@ -599,7 +588,7 @@ proc p(n: PNode; c: var Con): PNode = # move the variable declaration to the top of the frame: c.addTopVar v # make sure it's destroyed at the end of the proc: - c.destroys.add genDestroy(c, v.typ, v) + c.addDestructor(v.sym, genDestroy(c, v.typ, v)) if ri.kind != nkEmpty: let r = moveOrCopy(v, ri, c) result.add r @@ -625,12 +614,13 @@ proc p(n: PNode; c: var Con): PNode = sinkExpr.add n result.add sinkExpr result.add tmp - c.destroys.add genDestroy(c, n.typ, tmp) + c.addDestructor(tmp.sym, genDestroy(c, n.typ, tmp)) else: result = n of nkAsgn, nkFastAsgn: if hasDestructor(n[0].typ): result = moveOrCopy(n[0], n[1], c) + c.enableDestructor(n[0].sym) else: result = copyNode(n) recurse(n, result) @@ -646,12 +636,9 @@ proc injectDestructorCalls*(g: ModuleGraph; owner: PSym; n: PNode): PNode = # echo "injecting into ", n var c: Con c.owner = owner - c.tmp = newSym(skTemp, getIdent(g.cache, ":d"), owner, n.info) - c.tmpObj = createObj(g, owner, n.info) - c.tmp.typ = c.tmpObj - c.destroys = newNodeI(nkStmtList, n.info) c.topLevelVars = newNodeI(nkVarSection, n.info) - c.toDropBit = initTable[int, PSym]() + c.toDropBit = initTable[int, PSym](16) + c.destroys = initOrderedTable[int, (bool, PNode)](16) c.graph = g c.emptyNode = newNodeI(nkEmpty, n.info) let cfg = constructCfg(owner, n) @@ -668,13 +655,15 @@ proc injectDestructorCalls*(g: ModuleGraph; owner: PSym; n: PNode): PNode = let param = params[i].sym if param.typ.kind == tySink: registerDropBit(c, param) let body = p(n, c) - if c.tmp.typ.n.len > 0: - c.addTopVar(newSymNode c.tmp) result = newNodeI(nkStmtList, n.info) if c.topLevelVars.len > 0: result.add c.topLevelVars if c.destroys.len > 0: - result.add newTryFinally(body, c.destroys) + var destroy_list = newNodeI(nkStmtList, n.info) + for val in c.destroys.values: + if val.enabled: + destroy_list.add val.destroy_call + result.add newTryFinally(body, destroy_list) else: result.add body diff --git a/compiler/semexprs.nim b/compiler/semexprs.nim index 669862c56..517356434 100644 --- a/compiler/semexprs.nim +++ b/compiler/semexprs.nim @@ -614,8 +614,7 @@ proc analyseIfAddressTakenInCall(c: PContext, n: PNode) = const FakeVarParams = {mNew, mNewFinalize, mInc, ast.mDec, mIncl, mExcl, mSetLengthStr, mSetLengthSeq, mAppendStrCh, mAppendStrStr, mSwap, - mAppendSeqElem, mNewSeq, mReset, mShallowCopy, mDeepCopy, mMove, - mWasMoved} + mAppendSeqElem, mNewSeq, mReset, mShallowCopy, mDeepCopy, mMove} # get the real type of the callee # it may be a proc var with a generic alias type, so we skip over them diff --git a/lib/pure/collections/tables.nim b/lib/pure/collections/tables.nim index f46a368b1..9597d18fd 100644 --- a/lib/pure/collections/tables.nim +++ b/lib/pure/collections/tables.nim @@ -248,6 +248,7 @@ template withValue*[A, B](t: var Table[A, B], key: A, else: body2 + iterator allValues*[A, B](t: Table[A, B]; key: A): B = ## iterates over any value in the table ``t`` that belongs to the given ``key``. var h: Hash = genHash(key) and high(t.data) @@ -879,6 +880,51 @@ proc del*[A, B](t: var OrderedTableRef[A, B], key: A) = ## if the key does not exist. t[].del(key) + +template withValue*[A, B](t: var OrderedTable[A, B], key: A, value, body: untyped) = + ## retrieves the value at ``t[key]``. + ## ``value`` can be modified in the scope of the ``withValue`` call. + ## + ## .. code-block:: nim + ## + ## orderedTable.withValue(key, value) do: + ## # block is executed only if ``key`` in ``t`` + ## value.name = "username" + ## value.uid = 1000 + ## + mixin rawGet + var hc: Hash + var index = rawGet(t, key, hc) + let hasKey = index >= 0 + if hasKey: + var value {.inject.} = addr(t.data[index].val) + body + +template withValue*[A, B](t: var OrderedTable[A, B], key: A, + value, body1, body2: untyped) = + ## retrieves the value at ``t[key]``. + ## ``value`` can be modified in the scope of the ``withValue`` call. + ## + ## .. code-block:: nim + ## + ## orderedTable.withValue(key, value) do: + ## # block is executed only if ``key`` in ``t`` + ## value.name = "username" + ## value.uid = 1000 + ## do: + ## # block is executed when ``key`` not in ``t`` + ## raise newException(KeyError, "Key not found") + ## + mixin rawGet + var hc: Hash + var index = rawGet(t, key, hc) + let hasKey = index >= 0 + if hasKey: + var value {.inject.} = addr(t.data[index].val) + body1 + else: + body2 + # ------------------------------ count tables ------------------------------- type From f46573c1ed9f041d23a54d0911d7727d84c1a9cd Mon Sep 17 00:00:00 2001 From: Andrii Riabushenko Date: Wed, 5 Dec 2018 21:36:42 +0000 Subject: [PATCH 09/26] remove debug statements --- compiler/destroyer.nim | 18 +++++++++--------- lib/system.nim | 7 +------ 2 files changed, 10 insertions(+), 15 deletions(-) diff --git a/compiler/destroyer.nim b/compiler/destroyer.nim index 4703c31dd..82db5af7f 100644 --- a/compiler/destroyer.nim +++ b/compiler/destroyer.nim @@ -632,8 +632,8 @@ proc p(n: PNode; c: var Con): PNode = recurse(n, result) proc injectDestructorCalls*(g: ModuleGraph; owner: PSym; n: PNode): PNode = - #when true: # defined(nimDebugDestroys): - # echo "injecting into ", n + when false: # defined(nimDebugDestroys): + echo "injecting into ", n var c: Con c.owner = owner c.topLevelVars = newNodeI(nkVarSection, n.info) @@ -667,10 +667,10 @@ proc injectDestructorCalls*(g: ModuleGraph; owner: PSym; n: PNode): PNode = else: result.add body - #when defined(nimDebugDestroys): - if true: - echo "------------------------------------" - echo n - echo "-------" - echo owner.name.s, " transformed to: " - echo result + when defined(nimDebugDestroys): + if true: + echo "------------------------------------" + echo n + echo "-------" + echo owner.name.s, " transformed to: " + echo result diff --git a/lib/system.nim b/lib/system.nim index 9111ddd86..ec0dd629c 100644 --- a/lib/system.nim +++ b/lib/system.nim @@ -221,15 +221,10 @@ proc reset*[T](obj: var T) {.magic: "Reset", noSideEffect.} ## be called before any possible `object branch transition`:idx:. when defined(nimNewRuntime): - proc wasMoved*[T](obj: var T) {.magic: "WasMoved", noSideEffect.} = - ## resets an object `obj` to its initial (binary zero) value to signify - ## it was "moved" and to signify its destructor should do nothing and - ## ideally be optimized away. - discard proc move*[T](x: var T): T {.magic: "Move", noSideEffect.} = result = x - wasMoved(x) + reset(x) type range*{.magic: "Range".}[T] ## Generic type to construct range types. From 972707440ad0837d3de11cc6d60abbd3dc7ace75 Mon Sep 17 00:00:00 2001 From: Andrii Riabushenko Date: Wed, 5 Dec 2018 21:37:27 +0000 Subject: [PATCH 10/26] remove debug --- compiler/destroyer.nim | 2 -- 1 file changed, 2 deletions(-) diff --git a/compiler/destroyer.nim b/compiler/destroyer.nim index 82db5af7f..59ee5dcb6 100644 --- a/compiler/destroyer.nim +++ b/compiler/destroyer.nim @@ -670,7 +670,5 @@ proc injectDestructorCalls*(g: ModuleGraph; owner: PSym; n: PNode): PNode = when defined(nimDebugDestroys): if true: echo "------------------------------------" - echo n - echo "-------" echo owner.name.s, " transformed to: " echo result From 268461f06ea8b03c1b72c25163418076df22b88d Mon Sep 17 00:00:00 2001 From: Andrii Riabushenko Date: Wed, 5 Dec 2018 21:45:01 +0000 Subject: [PATCH 11/26] add comment --- compiler/destroyer.nim | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/compiler/destroyer.nim b/compiler/destroyer.nim index 59ee5dcb6..3811cccad 100644 --- a/compiler/destroyer.nim +++ b/compiler/destroyer.nim @@ -620,7 +620,8 @@ proc p(n: PNode; c: var Con): PNode = of nkAsgn, nkFastAsgn: if hasDestructor(n[0].typ): result = moveOrCopy(n[0], n[1], c) - c.enableDestructor(n[0].sym) + c.enableDestructor(n[0].sym) # last read to sink argument could have disabled the destructor + # but the variable is assigned again and new value should be destroyed else: result = copyNode(n) recurse(n, result) From 135191d9d661581db06a00f4396c024a8fd3a537 Mon Sep 17 00:00:00 2001 From: Andrii Riabushenko Date: Wed, 5 Dec 2018 22:17:19 +0000 Subject: [PATCH 12/26] collapse to tables into one --- compiler/destroyer.nim | 16 +++++++--------- lib/pure/collections/tables.nim | 1 - 2 files changed, 7 insertions(+), 10 deletions(-) diff --git a/compiler/destroyer.nim b/compiler/destroyer.nim index 3811cccad..56f941514 100644 --- a/compiler/destroyer.nim +++ b/compiler/destroyer.nim @@ -128,8 +128,8 @@ type g: ControlFlowGraph jumpTargets: IntSet topLevelVars: PNode - destroys: OrderedTable[int, tuple[enabled: bool, destroy_call: PNode]] # Symbol to destructor table - toDropBit: Table[int, PSym] + destroys: OrderedTable[int, tuple[enabled: bool, dropBit: PSym, destroy_call: PNode]] + # Symbol to destructor call table graph: ModuleGraph emptyNode: PNode otherRead: PNode @@ -323,11 +323,11 @@ proc addTopVar(c: var Con; v: PNode) = c.topLevelVars.add newTree(nkIdentDefs, v, c.emptyNode, c.emptyNode) proc dropBit(c: var Con; s: PSym): PSym = - result = c.toDropBit.getOrDefault(s.id) + result = c.destroys.getOrDefault(s.id).dropBit assert result != nil -proc addDestructor(c: var Con; s: PSym; destructor_call: PNode) = - let alreadyIn = c.destroys.hasKeyOrPut(s.id, (true, destructor_call)) +proc addDestructor(c: var Con; s: PSym; destructor_call: PNode, dropBit: PSym = nil) = + let alreadyIn = c.destroys.hasKeyOrPut(s.id, (true, dropBit, destructor_call)) if alreadyIn: let lineInfo = if s.ast != nil: s.ast.info else: c.owner.info internalError(c.graph.config, lineInfo, "Destructor call for sym " & s.name.s & " is already injected") @@ -347,13 +347,12 @@ proc registerDropBit(c: var Con; s: PSym) = result.typ = getSysType(c.graph, s.info, tyBool) let trueVal = newIntTypeNode(nkIntLit, 1, result.typ) c.topLevelVars.add newTree(nkIdentDefs, newSymNode result, c.emptyNode, trueVal) - c.toDropBit[s.id] = result # generate: # if not sinkParam_AliveBit: `=destroy`(sinkParam) let t = s.typ.skipTypes({tyGenericInst, tyAlias, tySink}) if t.destructor != nil: c.addDestructor(s, newTree(nkIfStmt, - newTree(nkElifBranch, newSymNode result, genDestroy(c, t, newSymNode s)))) + newTree(nkElifBranch, newSymNode result, genDestroy(c, t, newSymNode s))), result) proc getTemp(c: var Con; typ: PType; info: TLineInfo): PNode = let sym = newSym(skTemp, getIdent(c.graph.cache, ":tmpD"), c.owner, info) @@ -638,8 +637,7 @@ proc injectDestructorCalls*(g: ModuleGraph; owner: PSym; n: PNode): PNode = var c: Con c.owner = owner c.topLevelVars = newNodeI(nkVarSection, n.info) - c.toDropBit = initTable[int, PSym](16) - c.destroys = initOrderedTable[int, (bool, PNode)](16) + c.destroys = initOrderedTable[int, (bool, PSym, PNode)](16) c.graph = g c.emptyNode = newNodeI(nkEmpty, n.info) let cfg = constructCfg(owner, n) diff --git a/lib/pure/collections/tables.nim b/lib/pure/collections/tables.nim index 9597d18fd..ce87deea0 100644 --- a/lib/pure/collections/tables.nim +++ b/lib/pure/collections/tables.nim @@ -248,7 +248,6 @@ template withValue*[A, B](t: var Table[A, B], key: A, else: body2 - iterator allValues*[A, B](t: Table[A, B]; key: A): B = ## iterates over any value in the table ``t`` that belongs to the given ``key``. var h: Hash = genHash(key) and high(t.data) From 4799589fb3f7f21b2981a7b256fb479054f8492b Mon Sep 17 00:00:00 2001 From: Andrii Riabushenko Date: Fri, 7 Dec 2018 21:59:43 +0000 Subject: [PATCH 13/26] undo some changes --- compiler/ast.nim | 2 +- compiler/ccgexprs.nim | 7 +++++++ compiler/semexprs.nim | 3 ++- lib/system.nim | 7 ++++++- 4 files changed, 16 insertions(+), 3 deletions(-) diff --git a/compiler/ast.nim b/compiler/ast.nim index 2b595aee1..7cf35450b 100644 --- a/compiler/ast.nim +++ b/compiler/ast.nim @@ -627,7 +627,7 @@ type mIsPartOf, mAstToStr, mParallel, mSwap, mIsNil, mArrToSeq, mCopyStr, mCopyStrLast, mNewString, mNewStringOfCap, mParseBiggestFloat, - mMove, mDestroy, + mMove, mWasMoved, mDestroy, mReset, mArray, mOpenArray, mRange, mSet, mSeq, mOpt, mVarargs, mRef, mPtr, mVar, mDistinct, mVoid, mTuple, diff --git a/compiler/ccgexprs.nim b/compiler/ccgexprs.nim index bf1494b3d..0cb3d3ad5 100644 --- a/compiler/ccgexprs.nim +++ b/compiler/ccgexprs.nim @@ -1901,6 +1901,13 @@ proc binaryFloatArith(p: BProc, e: PNode, d: var TLoc, m: TMagic) = proc skipAddr(n: PNode): PNode = result = if n.kind in {nkAddr, nkHiddenAddr}: n[0] else: n +proc genWasMoved(p: BProc; n: PNode) = + var a: TLoc + initLocExpr(p, n[1].skipAddr, a) + resetLoc(p, a) + #linefmt(p, cpsStmts, "#nimZeroMem((void*)$1, sizeof($2));$n", + # addrLoc(p.config, a), getTypeDesc(p.module, a.t)) + proc genMove(p: BProc; n: PNode; d: var TLoc) = if d.k == locNone: getTemp(p, n.typ, d) var a: TLoc diff --git a/compiler/semexprs.nim b/compiler/semexprs.nim index 517356434..669862c56 100644 --- a/compiler/semexprs.nim +++ b/compiler/semexprs.nim @@ -614,7 +614,8 @@ proc analyseIfAddressTakenInCall(c: PContext, n: PNode) = const FakeVarParams = {mNew, mNewFinalize, mInc, ast.mDec, mIncl, mExcl, mSetLengthStr, mSetLengthSeq, mAppendStrCh, mAppendStrStr, mSwap, - mAppendSeqElem, mNewSeq, mReset, mShallowCopy, mDeepCopy, mMove} + mAppendSeqElem, mNewSeq, mReset, mShallowCopy, mDeepCopy, mMove, + mWasMoved} # get the real type of the callee # it may be a proc var with a generic alias type, so we skip over them diff --git a/lib/system.nim b/lib/system.nim index ec0dd629c..9111ddd86 100644 --- a/lib/system.nim +++ b/lib/system.nim @@ -221,10 +221,15 @@ proc reset*[T](obj: var T) {.magic: "Reset", noSideEffect.} ## be called before any possible `object branch transition`:idx:. when defined(nimNewRuntime): + proc wasMoved*[T](obj: var T) {.magic: "WasMoved", noSideEffect.} = + ## resets an object `obj` to its initial (binary zero) value to signify + ## it was "moved" and to signify its destructor should do nothing and + ## ideally be optimized away. + discard proc move*[T](x: var T): T {.magic: "Move", noSideEffect.} = result = x - reset(x) + wasMoved(x) type range*{.magic: "Range".}[T] ## Generic type to construct range types. From 961b7a9f6672c5422738364bea45a19060a8de5e Mon Sep 17 00:00:00 2001 From: Andrii Riabushenko Date: Fri, 7 Dec 2018 22:00:55 +0000 Subject: [PATCH 14/26] undo more changes --- compiler/ccgexprs.nim | 1 + 1 file changed, 1 insertion(+) diff --git a/compiler/ccgexprs.nim b/compiler/ccgexprs.nim index 0cb3d3ad5..34836d843 100644 --- a/compiler/ccgexprs.nim +++ b/compiler/ccgexprs.nim @@ -2046,6 +2046,7 @@ proc genMagicExpr(p: BProc, e: PNode, d: var TLoc, op: TMagic) = initLocExpr(p, e.sons[2], b) genDeepCopy(p, a, b) of mDotDot, mEqCString: genCall(p, e, d) + of mWasMoved: genWasMoved(p, e) of mMove: genMove(p, e, d) of mDestroy: discard "ignore calls to the default destructor" of mSlice: From 948c625f95d061e7f2608d9ead20d1fd05a64cd0 Mon Sep 17 00:00:00 2001 From: Andrii Riabushenko Date: Fri, 7 Dec 2018 22:11:34 +0000 Subject: [PATCH 15/26] undo more stuff --- compiler/destroyer.nim | 61 ++++++++++++++++++++------------- lib/pure/collections/tables.nim | 44 ------------------------ 2 files changed, 37 insertions(+), 68 deletions(-) diff --git a/compiler/destroyer.nim b/compiler/destroyer.nim index 56f941514..ebe8d1185 100644 --- a/compiler/destroyer.nim +++ b/compiler/destroyer.nim @@ -100,12 +100,12 @@ Rule Pattern Transformed into finally: `=destroy`(x) 1.2 var x: sink T; stmts var x: sink T; stmts; ensureEmpty(x) 2 x = f() `=sink`(x, f()) -3 x = lastReadOf z `=sink`(x, z); +3 x = lastReadOf z `=sink`(x, z); wasMoved(z) 4.1 y = sinkParam `=sink`(y, sinkParam) 4.2 x = y `=`(x, y) # a copy 5.1 f_sink(g()) f_sink(g()) 5.2 f_sink(y) f_sink(copy y); # copy unless we can see it's the last read -5.3 f_sink(move y) f_sink(y); # explicit moves empties 'y' +5.3 f_sink(move y) f_sink(y); # wasMoved(z) # explicit moves empties 'y' 5.4 f_noSink(g()) var tmp = bitwiseCopy(g()); f(tmp); `=destroy`(tmp) Remarks: Rule 1.2 is not yet implemented because ``sink`` is currently @@ -128,7 +128,7 @@ type g: ControlFlowGraph jumpTargets: IntSet topLevelVars: PNode - destroys: OrderedTable[int, tuple[enabled: bool, dropBit: PSym, destroy_call: PNode]] + destroys: OrderedTable[int, tuple[dropBit: PSym, destroy_call: PNode]] # Symbol to destructor call table graph: ModuleGraph emptyNode: PNode @@ -327,21 +327,11 @@ proc dropBit(c: var Con; s: PSym): PSym = assert result != nil proc addDestructor(c: var Con; s: PSym; destructor_call: PNode, dropBit: PSym = nil) = - let alreadyIn = c.destroys.hasKeyOrPut(s.id, (true, dropBit, destructor_call)) + let alreadyIn = c.destroys.hasKeyOrPut(s.id, (dropBit, destructor_call)) if alreadyIn: let lineInfo = if s.ast != nil: s.ast.info else: c.owner.info internalError(c.graph.config, lineInfo, "Destructor call for sym " & s.name.s & " is already injected") -proc disableDestructor(c: var Con; s: PSym) = - ## disable destructor, but do not delete such that it can be enabled back again later - c.destroys.with_value(s.id, value): - value.enabled = false - -proc enableDestructor(c: var Con; s: PSym) = - ## if destructor does not exist then ignore, otherwise make sure destructor is enabled - c.destroys.with_value(s.id, value): - value.enabled = true - proc registerDropBit(c: var Con; s: PSym) = let result = newSym(skTemp, getIdent(c.graph.cache, s.name.s & "_AliveBit"), c.owner, s.info) result.typ = getSysType(c.graph, s.info, tyBool) @@ -384,6 +374,31 @@ proc genMagicCall(n: PNode; c: var Con; magicname: string; m: TMagic): PNode = result.add(newSymNode(createMagic(c.graph, magicname, m))) result.add n +proc genWasMoved(n: PNode; c: var Con): PNode = + # The mWasMoved builtin does not take the address. + result = genMagicCall(n, c, "wasMoved", mWasMoved) + +proc destructiveMoveVar(n: PNode; c: var Con): PNode = + # generate: (let tmp = v; reset(v); tmp) + # XXX: Strictly speaking we can only move if there is a ``=sink`` defined + # or if no ``=sink`` is defined and also no assignment. + result = newNodeIT(nkStmtListExpr, n.info, n.typ) + + var temp = newSym(skLet, getIdent(c.graph.cache, "blitTmp"), c.owner, n.info) + temp.typ = n.typ + var v = newNodeI(nkLetSection, n.info) + let tempAsNode = newSymNode(temp) + + var vpart = newNodeI(nkIdentDefs, tempAsNode.info, 3) + vpart.sons[0] = tempAsNode + vpart.sons[1] = c.emptyNode + vpart.sons[2] = n + add(v, vpart) + + result.add v + result.add genWasMoved(n, c) + result.add tempAsNode + proc passCopyToSink(n: PNode; c: var Con): PNode = result = newNodeIT(nkStmtListExpr, n.info, n.typ) let tmp = getTemp(c, n.typ, n.info) @@ -420,9 +435,9 @@ proc pArg(arg: PNode; c: var Con; isSink: bool): PNode = result = arg elif arg.kind == nkSym and arg.sym.kind in InterestingSyms and isLastRead(arg, c): # if x is a variable and it its last read we eliminate its - # destructor invocation - c.disableDestructor(arg.sym) - result = arg + # destructor invokation, but don't. We need to reset its memory + # to disable its destructor which we have not elided: + result = destructiveMoveVar(arg, c) elif arg.kind == nkSym and isSinkParam(arg.sym): # mark the sink parameter as used: result = destructiveMoveSink(arg, c) @@ -580,14 +595,15 @@ proc p(n: PNode; c: var Con): PNode = if it.kind == nkVarTuple and hasDestructor(ri.typ): let x = lowerTupleUnpacking(c.graph, it, c.owner) result.add p(x, c) - elif it.kind == nkIdentDefs and hasDestructor(it[0].typ) and not isUnpackedTuple(it[0].sym): + elif it.kind == nkIdentDefs and hasDestructor(it[0].typ): for j in 0..L-2: let v = it[j] doAssert v.kind == nkSym # move the variable declaration to the top of the frame: c.addTopVar v # make sure it's destroyed at the end of the proc: - c.addDestructor(v.sym, genDestroy(c, v.typ, v)) + if not isUnpackedTuple(it[0].sym): + c.addDestructor(v.sym, genDestroy(c, v.typ, v)) if ri.kind != nkEmpty: let r = moveOrCopy(v, ri, c) result.add r @@ -619,8 +635,6 @@ proc p(n: PNode; c: var Con): PNode = of nkAsgn, nkFastAsgn: if hasDestructor(n[0].typ): result = moveOrCopy(n[0], n[1], c) - c.enableDestructor(n[0].sym) # last read to sink argument could have disabled the destructor - # but the variable is assigned again and new value should be destroyed else: result = copyNode(n) recurse(n, result) @@ -637,7 +651,7 @@ proc injectDestructorCalls*(g: ModuleGraph; owner: PSym; n: PNode): PNode = var c: Con c.owner = owner c.topLevelVars = newNodeI(nkVarSection, n.info) - c.destroys = initOrderedTable[int, (bool, PSym, PNode)](16) + c.destroys = initOrderedTable[int, (PSym, PNode)](32) c.graph = g c.emptyNode = newNodeI(nkEmpty, n.info) let cfg = constructCfg(owner, n) @@ -660,8 +674,7 @@ proc injectDestructorCalls*(g: ModuleGraph; owner: PSym; n: PNode): PNode = if c.destroys.len > 0: var destroy_list = newNodeI(nkStmtList, n.info) for val in c.destroys.values: - if val.enabled: - destroy_list.add val.destroy_call + destroy_list.add val.destroy_call result.add newTryFinally(body, destroy_list) else: result.add body diff --git a/lib/pure/collections/tables.nim b/lib/pure/collections/tables.nim index ce87deea0..e7a4d1de0 100644 --- a/lib/pure/collections/tables.nim +++ b/lib/pure/collections/tables.nim @@ -880,50 +880,6 @@ proc del*[A, B](t: var OrderedTableRef[A, B], key: A) = t[].del(key) -template withValue*[A, B](t: var OrderedTable[A, B], key: A, value, body: untyped) = - ## retrieves the value at ``t[key]``. - ## ``value`` can be modified in the scope of the ``withValue`` call. - ## - ## .. code-block:: nim - ## - ## orderedTable.withValue(key, value) do: - ## # block is executed only if ``key`` in ``t`` - ## value.name = "username" - ## value.uid = 1000 - ## - mixin rawGet - var hc: Hash - var index = rawGet(t, key, hc) - let hasKey = index >= 0 - if hasKey: - var value {.inject.} = addr(t.data[index].val) - body - -template withValue*[A, B](t: var OrderedTable[A, B], key: A, - value, body1, body2: untyped) = - ## retrieves the value at ``t[key]``. - ## ``value`` can be modified in the scope of the ``withValue`` call. - ## - ## .. code-block:: nim - ## - ## orderedTable.withValue(key, value) do: - ## # block is executed only if ``key`` in ``t`` - ## value.name = "username" - ## value.uid = 1000 - ## do: - ## # block is executed when ``key`` not in ``t`` - ## raise newException(KeyError, "Key not found") - ## - mixin rawGet - var hc: Hash - var index = rawGet(t, key, hc) - let hasKey = index >= 0 - if hasKey: - var value {.inject.} = addr(t.data[index].val) - body1 - else: - body2 - # ------------------------------ count tables ------------------------------- type From d9815574c0ac38cf832515c0034ff2be96da24d9 Mon Sep 17 00:00:00 2001 From: Andrii Riabushenko Date: Fri, 7 Dec 2018 22:14:46 +0000 Subject: [PATCH 16/26] more undo --- compiler/destroyer.nim | 2 +- lib/pure/collections/tables.nim | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/compiler/destroyer.nim b/compiler/destroyer.nim index ebe8d1185..158e8cc4a 100644 --- a/compiler/destroyer.nim +++ b/compiler/destroyer.nim @@ -105,7 +105,7 @@ Rule Pattern Transformed into 4.2 x = y `=`(x, y) # a copy 5.1 f_sink(g()) f_sink(g()) 5.2 f_sink(y) f_sink(copy y); # copy unless we can see it's the last read -5.3 f_sink(move y) f_sink(y); # wasMoved(z) # explicit moves empties 'y' +5.3 f_sink(move y) f_sink(y); wasMoved(y) # explicit moves empties 'y' 5.4 f_noSink(g()) var tmp = bitwiseCopy(g()); f(tmp); `=destroy`(tmp) Remarks: Rule 1.2 is not yet implemented because ``sink`` is currently diff --git a/lib/pure/collections/tables.nim b/lib/pure/collections/tables.nim index e7a4d1de0..f46a368b1 100644 --- a/lib/pure/collections/tables.nim +++ b/lib/pure/collections/tables.nim @@ -879,7 +879,6 @@ proc del*[A, B](t: var OrderedTableRef[A, B], key: A) = ## if the key does not exist. t[].del(key) - # ------------------------------ count tables ------------------------------- type From 43c70a6b12e24a6eed6e8b93cf21f225cef912da Mon Sep 17 00:00:00 2001 From: Andrii Riabushenko Date: Fri, 7 Dec 2018 22:25:32 +0000 Subject: [PATCH 17/26] improve test --- compiler/destroyer.nim | 14 +++++++++----- tests/destructor/tmove_objconstr.nim | 11 ++++++++--- 2 files changed, 17 insertions(+), 8 deletions(-) diff --git a/compiler/destroyer.nim b/compiler/destroyer.nim index 158e8cc4a..74a796e7c 100644 --- a/compiler/destroyer.nim +++ b/compiler/destroyer.nim @@ -441,11 +441,15 @@ proc pArg(arg: PNode; c: var Con; isSink: bool): PNode = elif arg.kind == nkSym and isSinkParam(arg.sym): # mark the sink parameter as used: result = destructiveMoveSink(arg, c) - elif arg.kind in {nkStmtListExpr, nkBlockExpr, nkBlockStmt}: - result = copyNode(arg) - for i in 0..arg.len-2: - result.add p(arg[i], c) - result.add pArg(arg[^1], c, isSink) + elif arg.kind in {nkBlockExpr, nkBlockStmt}: + result = copyNode(arg) + result.add arg[0] + result.add pArg(arg[1], c, isSink) + elif arg.kind == nkStmtListExpr: + result = copyNode(arg) + for i in 0..arg.len-2: + result.add p(arg[i], c) + result.add pArg(arg[^1], c, isSink) elif arg.kind in {nkIfExpr, nkIfStmt}: result = copyNode(arg) for i in 0.. 5: raise newException(ValueError, "new error") else: newMySeq(x, 1.0), - b:0, - c: if y > 0: move(cc) else: newMySeq(1, 3.0)) + b: 0, + c: block: + var tmp = if y > 0: move(cc) else: newMySeq(1, 3.0) + tmp[0] = 5 + tmp + ) + let (seq1, seq2) = myfunc(2, 3) doAssert seq1.len == 2 @@ -160,4 +165,4 @@ seq4 = var ii = 1 let arr2 = [newMySeq(2, 5.0), if i > 1: newMySeq(3, 1.0) else: newMySeq(0, 0.0)] -var seqOfSeq2 = @[newMySeq(2, 5.0), newMySeq(3, 1.0)] \ No newline at end of file +var seqOfSeq2 = @[newMySeq(2, 5.0), newMySeq(3, 1.0)] From 0fdd7629b423d77178052c98f42d0683d3ec17ba Mon Sep 17 00:00:00 2001 From: Andrii Riabushenko Date: Sat, 8 Dec 2018 19:18:00 +0000 Subject: [PATCH 18/26] remove dropbits in favour of destructive moves --- compiler/destroyer.nim | 71 ++++++++++-------------------------------- 1 file changed, 16 insertions(+), 55 deletions(-) diff --git a/compiler/destroyer.nim b/compiler/destroyer.nim index 74a796e7c..75cf86124 100644 --- a/compiler/destroyer.nim +++ b/compiler/destroyer.nim @@ -128,8 +128,7 @@ type g: ControlFlowGraph jumpTargets: IntSet topLevelVars: PNode - destroys: OrderedTable[int, tuple[dropBit: PSym, destroy_call: PNode]] - # Symbol to destructor call table + destroys: PNode graph: ModuleGraph emptyNode: PNode otherRead: PNode @@ -322,28 +321,6 @@ proc genDestroy(c: Con; t: PType; dest: PNode): PNode = proc addTopVar(c: var Con; v: PNode) = c.topLevelVars.add newTree(nkIdentDefs, v, c.emptyNode, c.emptyNode) -proc dropBit(c: var Con; s: PSym): PSym = - result = c.destroys.getOrDefault(s.id).dropBit - assert result != nil - -proc addDestructor(c: var Con; s: PSym; destructor_call: PNode, dropBit: PSym = nil) = - let alreadyIn = c.destroys.hasKeyOrPut(s.id, (dropBit, destructor_call)) - if alreadyIn: - let lineInfo = if s.ast != nil: s.ast.info else: c.owner.info - internalError(c.graph.config, lineInfo, "Destructor call for sym " & s.name.s & " is already injected") - -proc registerDropBit(c: var Con; s: PSym) = - let result = newSym(skTemp, getIdent(c.graph.cache, s.name.s & "_AliveBit"), c.owner, s.info) - result.typ = getSysType(c.graph, s.info, tyBool) - let trueVal = newIntTypeNode(nkIntLit, 1, result.typ) - c.topLevelVars.add newTree(nkIdentDefs, newSymNode result, c.emptyNode, trueVal) - # generate: - # if not sinkParam_AliveBit: `=destroy`(sinkParam) - let t = s.typ.skipTypes({tyGenericInst, tyAlias, tySink}) - if t.destructor != nil: - c.addDestructor(s, newTree(nkIfStmt, - newTree(nkElifBranch, newSymNode result, genDestroy(c, t, newSymNode s))), result) - proc getTemp(c: var Con; typ: PType; info: TLineInfo): PNode = let sym = newSym(skTemp, getIdent(c.graph.cache, ":tmpD"), c.owner, info) sym.typ = typ @@ -359,16 +336,6 @@ template recurse(n, dest) = proc isSinkParam(s: PSym): bool {.inline.} = result = s.kind == skParam and s.typ.kind == tySink -proc destructiveMoveSink(n: PNode; c: var Con): PNode = - # generate: (chckMove(sinkParam_AliveBit); sinkParam_AliveBit = false; sinkParam) - result = newNodeIT(nkStmtListExpr, n.info, n.typ) - let bit = newSymNode dropBit(c, n.sym) - if optMoveCheck in c.owner.options: - result.add callCodegenProc(c.graph, "chckMove", bit.info, bit) - result.add newTree(nkAsgn, bit, - newIntTypeNode(nkIntLit, 0, getSysType(c.graph, n.info, tyBool))) - result.add n - proc genMagicCall(n: PNode; c: var Con; magicname: string; m: TMagic): PNode = result = newNodeI(nkCall, n.info) result.add(newSymNode(createMagic(c.graph, magicname, m))) @@ -433,14 +400,11 @@ proc pArg(arg: PNode; c: var Con; isSink: bool): PNode = elif arg.kind in {nkBracket, nkObjConstr, nkTupleConstr, nkBracket, nkCharLit..nkFloat128Lit}: discard "object construction to sink parameter: nothing to do" result = arg - elif arg.kind == nkSym and arg.sym.kind in InterestingSyms and isLastRead(arg, c): - # if x is a variable and it its last read we eliminate its - # destructor invokation, but don't. We need to reset its memory - # to disable its destructor which we have not elided: + elif arg.kind == nkSym and (isSinkParam(arg.sym) or + arg.sym.kind in InterestingSyms and isLastRead(arg, c)): + # it is the last read be final consumption. We need to reset the memory + # to disable the destructor which we have not elided: result = destructiveMoveVar(arg, c) - elif arg.kind == nkSym and isSinkParam(arg.sym): - # mark the sink parameter as used: - result = destructiveMoveSink(arg, c) elif arg.kind in {nkBlockExpr, nkBlockStmt}: result = copyNode(arg) result.add arg[0] @@ -572,13 +536,11 @@ proc moveOrCopy(dest, ri: PNode; c: var Con): PNode = ri2[i] = pArg(ri[i], c, isSink = true) result.add ri2 of nkSym: - if ri.sym.kind != skParam and isLastRead(ri, c): + if isSinkParam(ri.sym) or (ri.sym.kind != skParam and isLastRead(ri, c)): # Rule 3: `=sink`(x, z); wasMoved(z) - result = genSink(c, dest.typ, dest, ri) - result.add p(ri, c) - elif isSinkParam(ri.sym): - result = genSink(c, dest.typ, dest, ri) - result.add destructiveMoveSink(ri, c) + var snk = genSink(c, dest.typ, dest, ri) + snk.add ri + result = newTree(nkStmtList, snk, genMagicCall(ri, c, "wasMoved", mWasMoved)) else: result = genCopy(c, dest.typ, dest, ri) result.add p(ri, c) @@ -607,7 +569,7 @@ proc p(n: PNode; c: var Con): PNode = c.addTopVar v # make sure it's destroyed at the end of the proc: if not isUnpackedTuple(it[0].sym): - c.addDestructor(v.sym, genDestroy(c, v.typ, v)) + c.destroys.add genDestroy(c, v.typ, v) if ri.kind != nkEmpty: let r = moveOrCopy(v, ri, c) result.add r @@ -633,7 +595,7 @@ proc p(n: PNode; c: var Con): PNode = sinkExpr.add n result.add sinkExpr result.add tmp - c.addDestructor(tmp.sym, genDestroy(c, n.typ, tmp)) + c.destroys.add genDestroy(c, n.typ, tmp) else: result = n of nkAsgn, nkFastAsgn: @@ -655,7 +617,7 @@ proc injectDestructorCalls*(g: ModuleGraph; owner: PSym; n: PNode): PNode = var c: Con c.owner = owner c.topLevelVars = newNodeI(nkVarSection, n.info) - c.destroys = initOrderedTable[int, (PSym, PNode)](32) + c.destroys = newNodeI(nkStmtList, n.info) c.graph = g c.emptyNode = newNodeI(nkEmpty, n.info) let cfg = constructCfg(owner, n) @@ -670,16 +632,15 @@ proc injectDestructorCalls*(g: ModuleGraph; owner: PSym; n: PNode): PNode = let params = owner.typ.n for i in 1 ..< params.len: let param = params[i].sym - if param.typ.kind == tySink: registerDropBit(c, param) + if param.typ.kind == tySink: + c.destroys.add genDestroy(c, param.typ.skipTypes({tyGenericInst, tyAlias, tySink}), params[i]) + let body = p(n, c) result = newNodeI(nkStmtList, n.info) if c.topLevelVars.len > 0: result.add c.topLevelVars if c.destroys.len > 0: - var destroy_list = newNodeI(nkStmtList, n.info) - for val in c.destroys.values: - destroy_list.add val.destroy_call - result.add newTryFinally(body, destroy_list) + result.add newTryFinally(body, c.destroys) else: result.add body From e5b9d89bcf15f34b54aeb95a8678b0223d1d9f7a Mon Sep 17 00:00:00 2001 From: Andrii Riabushenko Date: Sat, 8 Dec 2018 19:20:34 +0000 Subject: [PATCH 19/26] style improvements --- compiler/destroyer.nim | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/compiler/destroyer.nim b/compiler/destroyer.nim index 75cf86124..c5f49984e 100644 --- a/compiler/destroyer.nim +++ b/compiler/destroyer.nim @@ -116,7 +116,7 @@ Remarks: Rule 1.2 is not yet implemented because ``sink`` is currently import intsets, ast, astalgo, msgs, renderer, magicsys, types, idents, trees, - strutils, options, dfa, lowerings, tables, modulegraphs, msgs, + strutils, options, dfa, lowerings, tables, modulegraphs, lineinfos, parampatterns const @@ -127,8 +127,7 @@ type owner: PSym g: ControlFlowGraph jumpTargets: IntSet - topLevelVars: PNode - destroys: PNode + destroys, topLevelVars: PNode graph: ModuleGraph emptyNode: PNode otherRead: PNode @@ -616,8 +615,8 @@ proc injectDestructorCalls*(g: ModuleGraph; owner: PSym; n: PNode): PNode = echo "injecting into ", n var c: Con c.owner = owner - c.topLevelVars = newNodeI(nkVarSection, n.info) c.destroys = newNodeI(nkStmtList, n.info) + c.topLevelVars = newNodeI(nkVarSection, n.info) c.graph = g c.emptyNode = newNodeI(nkEmpty, n.info) let cfg = constructCfg(owner, n) From ae24b872191bead0ed2ba456fb78903b9717f107 Mon Sep 17 00:00:00 2001 From: Andrii Riabushenko Date: Sat, 8 Dec 2018 23:04:38 +0000 Subject: [PATCH 20/26] Double sink checks --- compiler/destroyer.nim | 66 ++++++++++++++++++++++++++++++++---- tests/destructor/tmatrix.nim | 12 +++---- 2 files changed, 65 insertions(+), 13 deletions(-) diff --git a/compiler/destroyer.nim b/compiler/destroyer.nim index c5f49984e..bed60a3e0 100644 --- a/compiler/destroyer.nim +++ b/compiler/destroyer.nim @@ -116,7 +116,7 @@ Remarks: Rule 1.2 is not yet implemented because ``sink`` is currently import intsets, ast, astalgo, msgs, renderer, magicsys, types, idents, trees, - strutils, options, dfa, lowerings, tables, modulegraphs, + strutils, options, dfa, lowerings, tables, modulegraphs, msgs, lineinfos, parampatterns const @@ -128,6 +128,8 @@ type g: ControlFlowGraph jumpTargets: IntSet destroys, topLevelVars: PNode + tracingSinkedParams: bool # we aren't checking double sink for proc args in if, case, loops since they possibly not taken + alreadySinkedParams: Table[int, TLineInfo] graph: ModuleGraph emptyNode: PNode otherRead: PNode @@ -365,6 +367,16 @@ proc destructiveMoveVar(n: PNode; c: var Con): PNode = result.add genWasMoved(n, c) result.add tempAsNode +proc sinkParamConsumed(c: var Con, s: PNode, tracing = true) = + assert s.kind == nkSym + let isConsumed = + if c.tracingSinkedParams and tracing: + c.alreadySinkedParams.hasKeyOrPut(s.sym.id, s.info) + else: c.alreadySinkedParams.hasKey(s.sym.id) + if isConsumed: + localError(c.graph.config, s.info, "sink parameter `" & $s.sym.name.s & + "` is already consumed at " & toFileLineCol(c. graph.config, c.alreadySinkedParams[s.sym.id])) + proc passCopyToSink(n: PNode; c: var Con): PNode = result = newNodeIT(nkStmtListExpr, n.info, n.typ) let tmp = getTemp(c, n.typ, n.info) @@ -399,10 +411,14 @@ proc pArg(arg: PNode; c: var Con; isSink: bool): PNode = elif arg.kind in {nkBracket, nkObjConstr, nkTupleConstr, nkBracket, nkCharLit..nkFloat128Lit}: discard "object construction to sink parameter: nothing to do" result = arg - elif arg.kind == nkSym and (isSinkParam(arg.sym) or - arg.sym.kind in InterestingSyms and isLastRead(arg, c)): - # it is the last read be final consumption. We need to reset the memory - # to disable the destructor which we have not elided: + elif arg.kind == nkSym and isSinkParam(arg.sym): + # Sinked params can be consumed only once. We need to reset the memory + # to disable the destructor which we have not elided + result = destructiveMoveVar(arg, c) + sinkParamConsumed(c, arg) + elif arg.kind == nkSym and arg.sym.kind in InterestingSyms and isLastRead(arg, c): + # it is the last read, can be sinked. We need to reset the memory + # to disable the destructor which we have not elided result = destructiveMoveVar(arg, c) elif arg.kind in {nkBlockExpr, nkBlockStmt}: result = copyNode(arg) @@ -415,6 +431,7 @@ proc pArg(arg: PNode; c: var Con; isSink: bool): PNode = result.add pArg(arg[^1], c, isSink) elif arg.kind in {nkIfExpr, nkIfStmt}: result = copyNode(arg) + c.tracingSinkedParams = false for i in 0.. Date: Sat, 8 Dec 2018 23:05:45 +0000 Subject: [PATCH 21/26] fix --- compiler/destroyer.nim | 1 + 1 file changed, 1 insertion(+) diff --git a/compiler/destroyer.nim b/compiler/destroyer.nim index bed60a3e0..b14615e40 100644 --- a/compiler/destroyer.nim +++ b/compiler/destroyer.nim @@ -647,6 +647,7 @@ proc p(n: PNode; c: var Con): PNode = c.tracingSinkedParams = false for i in 1.. Date: Sun, 9 Dec 2018 18:32:43 +0000 Subject: [PATCH 22/26] use control flow graph for sink params --- compiler/destroyer.nim | 55 ++++++++++-------------------------- compiler/dfa.nim | 2 +- tests/destructor/tmatrix.nim | 10 +++++-- 3 files changed, 23 insertions(+), 44 deletions(-) diff --git a/compiler/destroyer.nim b/compiler/destroyer.nim index b14615e40..1d4cdcc9b 100644 --- a/compiler/destroyer.nim +++ b/compiler/destroyer.nim @@ -128,7 +128,6 @@ type g: ControlFlowGraph jumpTargets: IntSet destroys, topLevelVars: PNode - tracingSinkedParams: bool # we aren't checking double sink for proc args in if, case, loops since they possibly not taken alreadySinkedParams: Table[int, TLineInfo] graph: ModuleGraph emptyNode: PNode @@ -367,15 +366,14 @@ proc destructiveMoveVar(n: PNode; c: var Con): PNode = result.add genWasMoved(n, c) result.add tempAsNode -proc sinkParamConsumed(c: var Con, s: PNode, tracing = true) = - assert s.kind == nkSym - let isConsumed = - if c.tracingSinkedParams and tracing: - c.alreadySinkedParams.hasKeyOrPut(s.sym.id, s.info) - else: c.alreadySinkedParams.hasKey(s.sym.id) - if isConsumed: - localError(c.graph.config, s.info, "sink parameter `" & $s.sym.name.s & - "` is already consumed at " & toFileLineCol(c. graph.config, c.alreadySinkedParams[s.sym.id])) +proc sinkParamIsLastReadCheck(c: var Con, s: PNode) = + assert s.kind == nkSym and s.sym.kind == skParam + discard isLastRead(s, c) + if c.otherRead != nil: + localError(c.graph.config, c.otherRead.info, "sink parameter `" & $s.sym.name.s & + "` is already consumed at " & toFileLineCol(c. graph.config, s.info)) + else: + c.alreadySinkedParams[s.sym.id] = s.info proc passCopyToSink(n: PNode; c: var Con): PNode = result = newNodeIT(nkStmtListExpr, n.info, n.typ) @@ -414,8 +412,8 @@ proc pArg(arg: PNode; c: var Con; isSink: bool): PNode = elif arg.kind == nkSym and isSinkParam(arg.sym): # Sinked params can be consumed only once. We need to reset the memory # to disable the destructor which we have not elided + sinkParamIsLastReadCheck(c, arg) result = destructiveMoveVar(arg, c) - sinkParamConsumed(c, arg) elif arg.kind == nkSym and arg.sym.kind in InterestingSyms and isLastRead(arg, c): # it is the last read, can be sinked. We need to reset the memory # to disable the destructor which we have not elided @@ -431,7 +429,6 @@ proc pArg(arg: PNode; c: var Con; isSink: bool): PNode = result.add pArg(arg[^1], c, isSink) elif arg.kind in {nkIfExpr, nkIfStmt}: result = copyNode(arg) - c.tracingSinkedParams = false for i in 0.. Date: Sun, 9 Dec 2018 18:37:33 +0000 Subject: [PATCH 23/26] remove not used code --- compiler/destroyer.nim | 8 -------- 1 file changed, 8 deletions(-) diff --git a/compiler/destroyer.nim b/compiler/destroyer.nim index 1d4cdcc9b..32d6c253b 100644 --- a/compiler/destroyer.nim +++ b/compiler/destroyer.nim @@ -128,7 +128,6 @@ type g: ControlFlowGraph jumpTargets: IntSet destroys, topLevelVars: PNode - alreadySinkedParams: Table[int, TLineInfo] graph: ModuleGraph emptyNode: PNode otherRead: PNode @@ -372,8 +371,6 @@ proc sinkParamIsLastReadCheck(c: var Con, s: PNode) = if c.otherRead != nil: localError(c.graph.config, c.otherRead.info, "sink parameter `" & $s.sym.name.s & "` is already consumed at " & toFileLineCol(c. graph.config, s.info)) - else: - c.alreadySinkedParams[s.sym.id] = s.info proc passCopyToSink(n: PNode; c: var Con): PNode = result = newNodeIT(nkStmtListExpr, n.info, n.typ) @@ -625,10 +622,6 @@ proc p(n: PNode; c: var Con): PNode = recurse(n, result) of nkSym: result = n - if c.alreadySinkedParams.hasKey n.sym.id: - # non desctructive use before sink is fine, but after sink it is an error - localError(c.graph.config, n.info, "sink parameter `" & $n.sym.name.s & - "` is already consumed at " & toFileLineCol(c. graph.config, c.alreadySinkedParams[n.sym.id])) of nkNone..pred(nkSym), succ(nkSym) .. nkNilLit, nkTypeSection, nkProcDef, nkConverterDef, nkMethodDef, nkIteratorDef, nkMacroDef, nkTemplateDef, nkLambda, nkDo, nkFuncDef: @@ -644,7 +637,6 @@ proc injectDestructorCalls*(g: ModuleGraph; owner: PSym; n: PNode): PNode = c.owner = owner c.destroys = newNodeI(nkStmtList, n.info) c.topLevelVars = newNodeI(nkVarSection, n.info) - c.alreadySinkedParams = initTable[int, TLineInfo](8) c.graph = g c.emptyNode = newNodeI(nkEmpty, n.info) let cfg = constructCfg(owner, n) From 8a690fd5307bc485ad2ac5ef6885d7b4fcfb4507 Mon Sep 17 00:00:00 2001 From: Andrii Riabushenko Date: Sun, 9 Dec 2018 18:38:21 +0000 Subject: [PATCH 24/26] Remove not used code --- compiler/destroyer.nim | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/compiler/destroyer.nim b/compiler/destroyer.nim index 32d6c253b..a090bead2 100644 --- a/compiler/destroyer.nim +++ b/compiler/destroyer.nim @@ -620,10 +620,7 @@ proc p(n: PNode; c: var Con): PNode = else: result = copyNode(n) recurse(n, result) - of nkSym: - result = n - - of nkNone..pred(nkSym), succ(nkSym) .. nkNilLit, nkTypeSection, nkProcDef, nkConverterDef, nkMethodDef, + of nkNone .. nkNilLit, nkTypeSection, nkProcDef, nkConverterDef, nkMethodDef, nkIteratorDef, nkMacroDef, nkTemplateDef, nkLambda, nkDo, nkFuncDef: result = n else: From 8b7f416c37d88599fd8fb9f88480584f191562de Mon Sep 17 00:00:00 2001 From: Andrii Riabushenko Date: Sun, 9 Dec 2018 18:40:46 +0000 Subject: [PATCH 25/26] reduce changes --- compiler/destroyer.nim | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/destroyer.nim b/compiler/destroyer.nim index a090bead2..7d2bb7221 100644 --- a/compiler/destroyer.nim +++ b/compiler/destroyer.nim @@ -620,7 +620,7 @@ proc p(n: PNode; c: var Con): PNode = else: result = copyNode(n) recurse(n, result) - of nkNone .. nkNilLit, nkTypeSection, nkProcDef, nkConverterDef, nkMethodDef, + of nkNone..nkNilLit, nkTypeSection, nkProcDef, nkConverterDef, nkMethodDef, nkIteratorDef, nkMacroDef, nkTemplateDef, nkLambda, nkDo, nkFuncDef: result = n else: From d22fa090000689c550a644c37aefc21ba4f0dba1 Mon Sep 17 00:00:00 2001 From: Andrii Riabushenko Date: Mon, 10 Dec 2018 09:20:43 +0000 Subject: [PATCH 26/26] minor correction --- compiler/destroyer.nim | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/compiler/destroyer.nim b/compiler/destroyer.nim index 7d2bb7221..407204e51 100644 --- a/compiler/destroyer.nim +++ b/compiler/destroyer.nim @@ -367,8 +367,7 @@ proc destructiveMoveVar(n: PNode; c: var Con): PNode = proc sinkParamIsLastReadCheck(c: var Con, s: PNode) = assert s.kind == nkSym and s.sym.kind == skParam - discard isLastRead(s, c) - if c.otherRead != nil: + if not isLastRead(s, c): localError(c.graph.config, c.otherRead.info, "sink parameter `" & $s.sym.name.s & "` is already consumed at " & toFileLineCol(c. graph.config, s.info))