From 3fbb3bfd3f440c059d6290c12834a38a61da98f2 Mon Sep 17 00:00:00 2001 From: Andreas Rumpf Date: Thu, 5 Dec 2019 16:59:06 +0100 Subject: ARC related bugfixes and refactorings (#12781) --- compiler/ast.nim | 20 +- compiler/ccgstmts.nim | 13 +- compiler/cgen.nim | 2 +- compiler/injectdestructors.nim | 427 +++++++++++++++++++---------------------- compiler/semtypinst.nim | 4 +- 5 files changed, 219 insertions(+), 247 deletions(-) (limited to 'compiler') diff --git a/compiler/ast.nim b/compiler/ast.nim index 9fbcab144..208e99c4a 100644 --- a/compiler/ast.nim +++ b/compiler/ast.nim @@ -1456,7 +1456,7 @@ proc isGCedMem*(t: PType): bool {.inline.} = result = t.kind in {tyString, tyRef, tySequence} or t.kind == tyProc and t.callConv == ccClosure -proc propagateToOwner*(owner, elem: PType) = +proc propagateToOwner*(owner, elem: PType; propagateHasAsgn = true) = const HaveTheirOwnEmpty = {tySequence, tyOpt, tySet, tyPtr, tyRef, tyProc} owner.flags = owner.flags + (elem.flags * {tfHasMeta, tfTriggersCompileTime}) if tfNotNil in elem.flags: @@ -1472,19 +1472,13 @@ proc propagateToOwner*(owner, elem: PType) = if elem.isMetaType: owner.flags.incl tfHasMeta - if tfHasAsgn in elem.flags: + let mask = elem.flags * {tfHasAsgn, tfHasOwned} + if mask != {} and propagateHasAsgn: let o2 = owner.skipTypes({tyGenericInst, tyAlias, tySink}) if o2.kind in {tyTuple, tyObject, tyArray, tySequence, tyOpt, tySet, tyDistinct, tyOpenArray, tyVarargs}: - o2.flags.incl tfHasAsgn - owner.flags.incl tfHasAsgn - - if tfHasOwned in elem.flags: - let o2 = owner.skipTypes({tyGenericInst, tyAlias, tySink}) - if o2.kind in {tyTuple, tyObject, tyArray, - tySequence, tyOpt, tySet, tyDistinct, tyOpenArray, tyVarargs}: - o2.flags.incl tfHasOwned - owner.flags.incl tfHasOwned + o2.flags.incl mask + owner.flags.incl mask if owner.kind notin {tyProc, tyGenericInst, tyGenericBody, tyGenericInvocation, tyPtr}: @@ -1494,11 +1488,11 @@ proc propagateToOwner*(owner, elem: PType) = # ensure this doesn't bite us in sempass2. owner.flags.incl tfHasGCedMem -proc rawAddSon*(father, son: PType) = +proc rawAddSon*(father, son: PType; propagateHasAsgn = true) = when not defined(nimNoNilSeqs): if isNil(father.sons): father.sons = @[] father.sons.add(son) - if not son.isNil: propagateToOwner(father, son) + if not son.isNil: propagateToOwner(father, son, propagateHasAsgn) proc rawAddSonNoPropagationOfTypeFlags*(father, son: PType) = when not defined(nimNoNilSeqs): diff --git a/compiler/ccgstmts.nim b/compiler/ccgstmts.nim index 59db5cb72..34fb06af8 100644 --- a/compiler/ccgstmts.nim +++ b/compiler/ccgstmts.nim @@ -1041,7 +1041,7 @@ proc genTry(p: BProc, t: PNode, d: var TLoc) = linefmt(p, cpsStmts, "#popSafePoint();$n", []) genRestoreFrameAfterException(p) elif 1 < t.len and t[1].kind == nkExceptBranch: - startBlock(p, "if (#getCurrentException()) {$n") + startBlock(p, "if (#nimBorrowCurrentException()) {$n") else: startBlock(p) p.nestedTryStmts[^1].inExcept = true @@ -1068,7 +1068,7 @@ proc genTry(p: BProc, t: PNode, d: var TLoc) = else: genTypeInfo(p.module, t[i][j].typ, t[i][j].info) let memberName = if p.module.compileToCpp: "m_type" else: "Sup.m_type" - appcg(p.module, orExpr, "#isObj(#getCurrentException()->$1, $2)", [memberName, checkFor]) + appcg(p.module, orExpr, "#isObj(#nimBorrowCurrentException()->$1, $2)", [memberName, checkFor]) if i > 1: line(p, cpsStmts, "else ") startBlock(p, "if ($1) {$n", [orExpr]) @@ -1082,7 +1082,14 @@ proc genTry(p: BProc, t: PNode, d: var TLoc) = endBlock(p) # end of else block if i < t.len and t[i].kind == nkFinally: p.finallySafePoints.add(safePoint) - genSimpleBlock(p, t[i][0]) + startBlock(p) + genStmts(p, t[i][0]) + # pretend we handled the exception in a 'finally' so that we don't + # re-raise the unhandled one but instead keep the old one (it was + # not popped either): + if not quirkyExceptions and getCompilerProc(p.module.g.graph, "nimLeaveFinally") != nil: + linefmt(p, cpsStmts, "if ($1.status != 0) #nimLeaveFinally();$n", [safePoint]) + endBlock(p) discard pop(p.finallySafePoints) if not quirkyExceptions: linefmt(p, cpsStmts, "if ($1.status != 0) #reraiseException();$n", [safePoint]) diff --git a/compiler/cgen.nim b/compiler/cgen.nim index 12acd7825..a456b7d51 100644 --- a/compiler/cgen.nim +++ b/compiler/cgen.nim @@ -1884,7 +1884,7 @@ proc shouldRecompile(m: BModule; code: Rope, cfile: Cfile): bool = if not moduleHasChanged(m.g.graph, m.module): result = false elif not equalsFile(code, cfile.cname): - if false: + when false: #m.config.symbolFiles == readOnlySf: #isDefined(m.config, "nimdiff"): if fileExists(cfile.cname): copyFile(cfile.cname.string, cfile.cname.string & ".backup") diff --git a/compiler/injectdestructors.nim b/compiler/injectdestructors.nim index de723b776..7a8c18f13 100644 --- a/compiler/injectdestructors.nim +++ b/compiler/injectdestructors.nim @@ -170,6 +170,7 @@ proc genOp(c: Con; t: PType; kind: TTypeAttachedOp; dest, ri: PNode): PNode = op = canon.attachedOps[kind] if op == nil: + #echo dest.typ.id globalError(c.graph.config, dest.info, "internal error: '" & AttachedOpToStr[kind] & "' operator not found for type " & typeToString(t)) elif op.ast[genericParamsPos].kind != nkEmpty: @@ -250,22 +251,25 @@ 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) + if not hasDestructor(n.typ): + result = copyTree(n) + else: + 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 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[0] = tempAsNode - vpart[1] = c.emptyNode - vpart[2] = n - v.add(vpart) + var vpart = newNodeI(nkIdentDefs, tempAsNode.info, 3) + vpart[0] = tempAsNode + vpart[1] = c.emptyNode + vpart[2] = n + v.add(vpart) - result.add v - result.add genWasMoved(skipConv(n), c) - result.add tempAsNode + result.add v + result.add genWasMoved(skipConv(n), c) + result.add tempAsNode proc sinkParamIsLastReadCheck(c: var Con, s: PNode) = assert s.kind == nkSym and s.sym.kind == skParam @@ -273,8 +277,12 @@ proc sinkParamIsLastReadCheck(c: var Con, s: PNode) = localError(c.graph.config, c.otherRead.info, "sink parameter `" & $s.sym.name.s & "` is already consumed at " & toFileLineCol(c. graph.config, s.info)) -proc p(n: PNode; c: var Con; consumed = false): PNode -proc pArg(arg: PNode; c: var Con; isSink: bool): PNode +type ProcessMode = enum + normal + consumed + sinkArg + +proc p(n: PNode; c: var Con; mode: ProcessMode = normal): PNode proc moveOrCopy(dest, ri: PNode; c: var Con): PNode proc isClosureEnv(n: PNode): bool = n.kind == nkSym and n.sym.name.s[0] == ':' @@ -366,6 +374,10 @@ template handleNested(n: untyped, processCall: untyped) = branch.add if node.typ == nil: p(node, c) #noreturn else: processCall result.add branch + of nkWhen: # This should be a "when nimvm" node. + result = copyTree(n) + template node: untyped = n[1][0] + result[1][0] = processCall else: assert(false) proc ensureDestruction(arg: PNode; c: var Con): PNode = @@ -384,75 +396,6 @@ proc ensureDestruction(arg: PNode; c: var Con): PNode = else: result = arg -proc pArg(arg: PNode; c: var Con; isSink: bool): PNode = - 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! - result = copyNode(arg) - let parameters = arg[0].typ - let L = if parameters != nil: parameters.len else: 0 - result.add pArg(arg[0], c, isSink = false) - for i in 1.. 0: - ri = genDefaultCall(v.typ, c, v.info) - if ri.kind != nkEmpty: - let r = moveOrCopy(v, ri, c) - result.add r - else: # keep the var but transform 'ri': - var v = copyNode(n) - var itCopy = copyNode(it) - for j in 0.. 0: + ri = genDefaultCall(v.typ, c, v.info) + if ri.kind != nkEmpty: + let r = moveOrCopy(v, ri, c) + result.add r + else: # keep the var but transform 'ri': + var v = copyNode(n) + var itCopy = copyNode(it) + for j in 0.. 0 and isDangerousSeq(ri.typ): result = genCopy(c, dest, ri) else: result = genSink(c, dest, ri) - result.add p(ri, c, consumed = true) + result.add p(ri, c, consumed) of nkObjConstr, nkTupleConstr, nkClosure, nkCharLit..nkNilLit: result = genSink(c, dest, ri) - result.add p(ri, c, consumed = true) + result.add p(ri, c, consumed) of nkSym: if isSinkParam(ri.sym): # Rule 3: `=sink`(x, z); wasMoved(z) @@ -674,18 +635,26 @@ proc moveOrCopy(dest, ri: PNode; c: var Con): PNode = result = newTree(nkStmtList, snk, genWasMoved(ri, c)) else: result = genCopy(c, dest, ri) - result.add p(ri, c, consumed = true) + result.add p(ri, c, consumed) of nkHiddenSubConv, nkHiddenStdConv, nkConv: - result = moveOrCopy(dest, ri[1], c) - if not sameType(ri.typ, ri[1].typ): + when false: + result = moveOrCopy(dest, ri[1], c) + if not sameType(ri.typ, ri[1].typ): + let copyRi = copyTree(ri) + copyRi[1] = result[^1] + result[^1] = copyRi + else: + result = genSink(c, dest, ri) + result.add p(ri, c, sinkArg) + of nkObjDownConv, nkObjUpConv: + when false: + result = moveOrCopy(dest, ri[0], c) let copyRi = copyTree(ri) - copyRi[1] = result[^1] + copyRi[0] = result[^1] result[^1] = copyRi - of nkObjDownConv, nkObjUpConv: - result = moveOrCopy(dest, ri[0], c) - let copyRi = copyTree(ri) - copyRi[0] = result[^1] - result[^1] = copyRi + else: + result = genSink(c, dest, ri) + result.add p(ri, c, sinkArg) of nkStmtListExpr, nkBlockExpr, nkIfExpr, nkCaseStmt: handleNested(ri): moveOrCopy(dest, node, c) else: @@ -697,7 +666,7 @@ proc moveOrCopy(dest, ri: PNode; c: var Con): PNode = result = newTree(nkStmtList, snk, genWasMoved(ri, c)) else: result = genCopy(c, dest, ri) - result.add p(ri, c, consumed = true) + result.add p(ri, c, consumed) proc computeUninit(c: var Con) = if not c.uninitComputed: diff --git a/compiler/semtypinst.nim b/compiler/semtypinst.nim index 786a9e4f8..6d9e9873e 100644 --- a/compiler/semtypinst.nim +++ b/compiler/semtypinst.nim @@ -307,6 +307,8 @@ proc instCopyType*(cl: var TReplTypeVars, t: PType): PType = if not (t.kind in tyMetaTypes or (t.kind == tyStatic and t.n == nil)): result.flags.excl tfInstClearedFlags + else: + result.flags.excl tfHasAsgn when false: if newDestructors: result.assignment = nil @@ -380,7 +382,7 @@ proc handleGenericInvocation(cl: var TReplTypeVars, t: PType): PType = for i in 1..