diff --git a/compiler/ccgcalls.nim b/compiler/ccgcalls.nim index 80f02b6c36..2c5d306a3f 100644 --- a/compiler/ccgcalls.nim +++ b/compiler/ccgcalls.nim @@ -644,7 +644,33 @@ proc genNamedParamCall(p: BProc, ri: PNode, d: var TLoc) = pl.add(~"];$n") line(p, cpsStmts, pl) +proc notYetAlive(n: PNode): bool {.inline.} = + let r = getRoot(n) + result = r != nil and r.loc.lode == nil + +proc isInactiveDestructorCall(p: BProc, e: PNode): bool = + #[ Consider this example. + + var :tmpD_3281815 + try: + if true: + return + let args_3280013 = + wasMoved_3281816(:tmpD_3281815) + `=_3280036`(:tmpD_3281815, [1]) + :tmpD_3281815 + finally: + `=destroy_3280027`(args_3280013) + + We want to return early but the 'finally' section is traversed before + the 'let args = ...' statement. We exploit this to generate better + code for 'return'. ]# + result = e.len == 2 and e[0].kind == nkSym and + e[0].sym.name.s == "=destroy" and notYetAlive(e[1].skipAddr) + proc genCall(p: BProc, e: PNode, d: var TLoc) = + if p.withinBlockLeaveActions > 0 and isInactiveDestructorCall(p, e): + return if e[0].typ.skipTypes({tyGenericInst, tyAlias, tySink, tyOwned}).callConv == ccClosure: genClosureCall(p, nil, e, d) elif e[0].kind == nkSym and sfInfixCall in e[0].sym.flags: diff --git a/compiler/ccgexprs.nim b/compiler/ccgexprs.nim index 08b9e1ff60..c194ce3263 100644 --- a/compiler/ccgexprs.nim +++ b/compiler/ccgexprs.nim @@ -2066,10 +2066,14 @@ proc skipAddr(n: PNode): PNode = 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)]) + let n1 = n[1].skipAddr + if p.withinBlockLeaveActions > 0 and notYetAlive(n1): + discard + else: + initLocExpr(p, n1, 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) = var a: TLoc @@ -2593,10 +2597,12 @@ proc expr(p: BProc, n: PNode, d: var TLoc) = else: putLocIntoDest(p, d, sym.loc) of skTemp: - if sym.loc.r == nil: - # we now support undeclared 'skTemp' variables for easier - # transformations in other parts of the compiler: - assignLocalVar(p, n) + when false: + # this is more harmful than helpful. + if sym.loc.r == nil: + # we now support undeclared 'skTemp' variables for easier + # transformations in other parts of the compiler: + assignLocalVar(p, n) if sym.loc.r == nil or sym.loc.t == nil: #echo "FAILED FOR PRCO ", p.prc.name.s #echo renderTree(p.prc.ast, {renderIds}) diff --git a/compiler/ccgstmts.nim b/compiler/ccgstmts.nim index 82f1caadc0..5322723743 100644 --- a/compiler/ccgstmts.nim +++ b/compiler/ccgstmts.nim @@ -200,6 +200,7 @@ proc blockLeaveActions(p: BProc, howManyTrys, howManyExcepts: int) = var stack = newSeq[tuple[fin: PNode, inExcept: bool, label: Natural]](0) + inc p.withinBlockLeaveActions for i in 1..howManyTrys: let tryStmt = p.nestedTryStmts.pop if p.config.exc == excSetjmp: @@ -217,6 +218,8 @@ proc blockLeaveActions(p: BProc, howManyTrys, howManyExcepts: int) = if finallyStmt != nil: genStmts(p, finallyStmt[0]) + dec p.withinBlockLeaveActions + # push old elements again: for i in countdown(howManyTrys-1, 0): p.nestedTryStmts.add(stack[i]) @@ -861,10 +864,10 @@ proc genStringCase(p: BProc, t: PNode, d: var TLoc) = genCaseGeneric(p, t, d, "", "if (#eqStrings($1, $2)) goto $3;$n") proc branchHasTooBigRange(b: PNode): bool = - for i in 0.. RangeExpandLimit: + if (it.kind == nkRange) and + it[1].intVal - it[0].intVal > RangeExpandLimit: return true proc ifSwitchSplitPoint(p: BProc, n: PNode): int = @@ -988,9 +991,14 @@ proc genTryCpp(p: BProc, t: PNode, d: var TLoc) = let fin = if t[^1].kind == nkFinally: t[^1] else: nil p.nestedTryStmts.add((fin, false, 0.Natural)) - startBlock(p, "try {$n") - expr(p, t[0], d) - endBlock(p) + if t.kind == nkHiddenTryStmt: + lineCg(p, cpsStmts, "try {$n", []) + expr(p, t[0], d) + lineCg(p, cpsStmts, "}$n", []) + else: + startBlock(p, "try {$n") + expr(p, t[0], d) + endBlock(p) # First pass: handle Nim based exceptions: lineCg(p, cpsStmts, "catch (#Exception* T$1_) {$n", [etmp+1]) @@ -1335,13 +1343,13 @@ proc genTrySetjmp(p: BProc, t: PNode, d: var TLoc) = linefmt(p, cpsStmts, "$1.status = _setjmp($1.context);$n", [safePoint]) else: linefmt(p, cpsStmts, "$1.status = setjmp($1.context);$n", [safePoint]) - startBlock(p, "if ($1.status == 0) {$n", [safePoint]) + lineCg(p, cpsStmts, "if ($1.status == 0) {$n", [safePoint]) let fin = if t[^1].kind == nkFinally: t[^1] else: nil p.nestedTryStmts.add((fin, quirkyExceptions, 0.Natural)) expr(p, t[0], d) if not quirkyExceptions: linefmt(p, cpsStmts, "#popSafePoint();$n", []) - endBlock(p) + lineCg(p, cpsStmts, "}$n", []) startBlock(p, "else {$n") linefmt(p, cpsStmts, "#popSafePoint();$n", []) genRestoreFrameAfterException(p) diff --git a/compiler/cgen.nim b/compiler/cgen.nim index 99bfe91192..86860fbf46 100644 --- a/compiler/cgen.nim +++ b/compiler/cgen.nim @@ -1561,7 +1561,7 @@ proc registerModuleToMain(g: BModuleList; m: BModule) = if sfSystemModule in m.module.flags: if emulatedThreadVars(m.config) and m.config.target.targetOS != osStandalone: g.mainDatInit.add(ropecg(m, "\t#initThreadVarsEmulation();$N", [])) - if m.config.target.targetOS != osStandalone and m.config.selectedGC != gcNone: + if m.config.target.targetOS != osStandalone and m.config.selectedGC notin {gcNone, gcArc, gcOrc}: g.mainDatInit.add(ropecg(m, "\t#initStackBottomWith((void *)&inner);$N", [])) if m.s[cfsInitProc].len > 0: @@ -1666,6 +1666,10 @@ proc genInitCode(m: BModule) = writeSection(preInitProc, cpsInit, m.hcrOn) writeSection(preInitProc, cpsStmts) prc.addf("}$N", []) + when false: + m.initProc.blocks[0].sections[cpsLocals].add m.preInitProc.s(cpsLocals) + m.initProc.blocks[0].sections[cpsInit].prepend m.preInitProc.s(cpsInit) + m.initProc.blocks[0].sections[cpsStmts].prepend m.preInitProc.s(cpsStmts) # add new scope for following code, because old vcc compiler need variable # be defined at the top of the block diff --git a/compiler/cgendata.nim b/compiler/cgendata.nim index a20931be42..3384558f87 100644 --- a/compiler/cgendata.nim +++ b/compiler/cgendata.nim @@ -97,6 +97,7 @@ type # requires 'T x = T()' to become 'T x; x = T()' # (yes, C++ is weird like that) withinTryWithExcept*: int # required for goto based exception handling + withinBlockLeaveActions*: int # complex to explain sigConflicts*: CountTable[string] TTypeSeq* = seq[PType] diff --git a/compiler/injectdestructors.nim b/compiler/injectdestructors.nim index a2669be99e..e106869174 100644 --- a/compiler/injectdestructors.nim +++ b/compiler/injectdestructors.nim @@ -27,10 +27,101 @@ import lineinfos, parampatterns, sighashes, liftdestructors from trees import exprStructuralEquivalent, getRoot -from algorithm import reverse -const - scopeBasedDestruction = false +type + Scope = object # well we do scope-based memory management. \ + # a scope is comparable to an nkStmtListExpr like + # (try: statements; dest = y(); finally: destructors(); dest) + vars: seq[PSym] + wasMoved: seq[PNode] + final: seq[PNode] # finally section + needsTry: bool + parent: ptr Scope + +proc nestedScope(parent: var Scope): Scope = + Scope(vars: @[], wasMoved: @[], final: @[], needsTry: false, parent: addr(parent)) + +proc rememberParent(parent: var Scope; inner: Scope) {.inline.} = + parent.needsTry = parent.needsTry or inner.needsTry + +proc optimize(s: var Scope) = + # optimize away simple 'wasMoved(x); destroy(x)' pairs. + #[ Unfortunately this optimization is only really safe when no exceptions + are possible, see for example: + + proc main(inp: string; cond: bool) = + if cond: + try: + var s = ["hi", inp & "more"] + for i in 0..4: + echo s + consume(s) + wasMoved(s) + finally: + destroy(x) + + Now assume 'echo' raises, then we shouldn't do the 'wasMoved(s)' + ]# + # XXX: Investigate how to really insert 'wasMoved()' calls! + proc findCorrespondingDestroy(final: seq[PNode]; moved: PNode): int = + # remember that it's destroy(addr(x)) + for i in 0 ..< final.len: + if final[i] != nil and exprStructuralEquivalent(final[i][1].skipAddr, moved, strictSymEquality = true): + return i + return -1 + + var removed = 0 + for i in 0 ..< s.wasMoved.len: + let j = findCorrespondingDestroy(s.final, s.wasMoved[i][1]) + if j >= 0: + s.wasMoved[i] = nil + s.final[j] = nil + inc removed + if removed > 0: + template filterNil(field) = + var m = newSeq[PNode](s.field.len - removed) + var mi = 0 + for i in 0 ..< s.field.len: + if s.field[i] != nil: + m[mi] = s.field[i] + inc mi + assert mi == m.len + s.field = m + + filterNil(wasMoved) + filterNil(final) + +proc toTree(s: var Scope; ret: PNode; onlyCareAboutVars = false): PNode = + if not s.needsTry: optimize(s) + assert ret != nil + if s.vars.len == 0 and s.final.len == 0 and s.wasMoved.len == 0: + # trivial, nothing was done: + result = ret + else: + if isEmptyType(ret.typ): + result = newNodeI(nkStmtList, ret.info) + else: + result = newNodeIT(nkStmtListExpr, ret.info, ret.typ) + + if s.vars.len > 0: + let varSection = newNodeI(nkVarSection, ret.info) + for tmp in s.vars: + varSection.add newTree(nkIdentDefs, newSymNode(tmp), newNodeI(nkEmpty, ret.info), + newNodeI(nkEmpty, ret.info)) + result.add varSection + if onlyCareAboutVars: + result.add ret + s.vars.setLen 0 + elif s.needsTry: + var finSection = newNodeI(nkStmtList, ret.info) + for m in s.wasMoved: finSection.add m + for i in countdown(s.final.high, 0): finSection.add s.final[i] + result.add newTryFinally(ret, finSection) + else: + #assert isEmptyType(ret.typ) + result.add ret + for m in s.wasMoved: result.add m + for i in countdown(s.final.high, 0): result.add s.final[i] type Con = object @@ -38,14 +129,10 @@ type g: ControlFlowGraph jumpTargets: IntSet destroys, topLevelVars: PNode - scopeDestroys: seq[PNode] # used as a stack that pop from - # at strategic places which try to - # mimic the natural scope. graph: ModuleGraph emptyNode: PNode otherRead: PNode - inLoop, inSpawn, hasUnstructuredCf, inDangerousBranch: int - declaredVars: IntSet # variables we already moved to the top level + inLoop, inSpawn: int uninit: IntSet # set of uninit'ed vars uninitComputed: bool @@ -62,8 +149,8 @@ template dbg(body) = if c.owner.name.s == toDebug or toDebug == "always": body -proc p(n: PNode; c: var Con; mode: ProcessMode): PNode -proc moveOrCopy(dest, ri: PNode; c: var Con, isDecl = false): PNode +proc p(n: PNode; c: var Con; s: var Scope; mode: ProcessMode): PNode +proc moveOrCopy(dest, ri: PNode; c: var Con; s: var Scope; isDecl = false): PNode proc isLastRead(location: PNode; cfg: ControlFlowGraph; otherRead: var PNode; pc, until: int): int = var pc = pc @@ -265,15 +352,6 @@ proc genDestroy(c: Con; dest: PNode): PNode = let t = dest.typ.skipTypes({tyGenericInst, tyAlias, tySink}) result = genOp(c, t, attachedDestructor, dest, nil) -when false: - proc preventMoveRef(dest, ri: PNode): bool = - let lhs = dest.typ.skipTypes({tyGenericInst, tyAlias, tySink}) - var ri = ri - if ri.kind in nkCallKinds and ri[0].kind == nkSym and ri[0].sym.magic == mUnown: - ri = ri[1] - let rhs = ri.typ.skipTypes({tyGenericInst, tyAlias, tySink}) - result = lhs.kind == tyRef and rhs.kind == tyOwned - proc canBeMoved(c: Con; t: PType): bool {.inline.} = let t = t.skipTypes({tyGenericInst, tyAlias, tySink}) if optOwnedRefs in c.graph.config.globalOptions: @@ -284,8 +362,10 @@ proc canBeMoved(c: Con; t: PType): bool {.inline.} = proc isNoInit(dest: PNode): bool {.inline.} = result = dest.kind == nkSym and sfNoInit in dest.sym.flags -proc genSink(c: var Con; dest, ri: PNode, isDecl = false): PNode = - if isUnpackedTuple(dest) or (isDecl and c.inLoop <= 0) or (isAnalysableFieldAccess(dest, c.owner) and isFirstWrite(dest, c)) or isNoInit(dest): +proc genSink(c: var Con; s: var Scope; dest, ri: PNode, isDecl = false): PNode = + if isUnpackedTuple(dest) or (isDecl and c.inLoop <= 0) or + (isAnalysableFieldAccess(dest, c.owner) and isFirstWrite(dest, c)) or + isNoInit(dest): # optimize sink call into a bitwise memcopy result = newTree(nkFastAsgn, dest, ri) else: @@ -311,34 +391,34 @@ proc genCopy(c: var Con; dest, ri: PNode): PNode = checkForErrorPragma(c, t, ri, "=") result = genCopyNoCheck(c, dest, ri) -proc addTopVar(c: var Con; v: PNode) = - c.topLevelVars.add newTree(nkIdentDefs, v, c.emptyNode, c.emptyNode) +proc addTopVar(c: var Con; s: var Scope; v: PNode) = + s.vars.add v.sym -proc getTemp(c: var Con; typ: PType; info: TLineInfo): PNode = +proc getTemp(c: var Con; s: var Scope; typ: PType; info: TLineInfo): PNode = let sym = newSym(skTemp, getIdent(c.graph.cache, ":tmpD"), c.owner, info) sym.typ = typ + s.vars.add(sym) result = newSymNode(sym) -proc genDiscriminantAsgn(c: var Con; n: PNode): PNode = +proc genDiscriminantAsgn(c: var Con; s: var Scope; n: PNode): PNode = # discriminator is ordinal value that doesn't need sink destroy # but fields within active case branch might need destruction # tmp to support self assignments - let tmp = getTemp(c, n[1].typ, n.info) - c.addTopVar(tmp) + let tmp = getTemp(c, s, n[1].typ, n.info) result = newTree(nkStmtList) - result.add newTree(nkFastAsgn, tmp, p(n[1], c, consumed)) - result.add p(n[0], c, normal) + result.add newTree(nkFastAsgn, tmp, p(n[1], c, s, consumed)) + result.add p(n[0], c, s, normal) - let le = p(n[0], c, normal) + let le = p(n[0], c, s, normal) let leDotExpr = if le.kind == nkCheckedFieldExpr: le[0] else: le let objType = leDotExpr[0].typ if hasDestructor(objType): if objType.attachedOps[attachedDestructor] != nil and sfOverriden in objType.attachedOps[attachedDestructor].flags: - localError(c.graph.config, n.info, errGenerated, """Assignment to discriminant for object's with user defined destructor is not supported, object must have default destructor. + localError(c.graph.config, n.info, errGenerated, """Assignment to discriminant for objects with user defined destructor is not supported, object must have default destructor. It is best to factor out piece of object that needs custom destructor into separate object or not use discriminator assignment""") result.add newTree(nkFastAsgn, le, tmp) return @@ -353,9 +433,7 @@ It is best to factor out piece of object that needs custom destructor into separ notExpr.add newSymNode(createMagic(c.graph, "not", mNot)) notExpr.add cond result.add newTree(nkIfStmt, newTree(nkElifBranch, notExpr, genOp(c, branchDestructor, le))) - result.add newTree(nkFastAsgn, le, tmp) - else: - result.add newTree(nkFastAsgn, le, tmp) + result.add newTree(nkFastAsgn, le, tmp) proc genWasMoved(n: PNode; c: var Con): PNode = result = newNodeI(nkCall, n.info) @@ -369,7 +447,7 @@ proc genDefaultCall(t: PType; c: Con; info: TLineInfo): PNode = result.add(newSymNode(createMagic(c.graph, "default", mDefault))) result.typ = t -proc destructiveMoveVar(n: PNode; c: var Con): PNode = +proc destructiveMoveVar(n: PNode; c: var Con; s: var Scope): PNode = # generate: (let tmp = v; reset(v); tmp) if not hasDestructor(n.typ): result = copyTree(n) @@ -388,28 +466,21 @@ proc destructiveMoveVar(n: PNode; c: var Con): PNode = v.add(vpart) result.add v - result.add genWasMoved(skipConv(n), c) + let wasMovedCall = genWasMoved(skipConv(n), c) + result.add wasMovedCall result.add tempAsNode -proc sinkParamIsLastReadCheck(c: var Con, s: PNode) = - assert s.kind == nkSym and s.sym.kind == skParam - 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)) - proc isCapturedVar(n: PNode): bool = let root = getRoot(n) if root != nil: result = root.name.s[0] == ':' -proc passCopyToSink(n: PNode; c: var Con): PNode = +proc passCopyToSink(n: PNode; c: var Con; s: var Scope): PNode = result = newNodeIT(nkStmtListExpr, n.info, n.typ) - let tmp = getTemp(c, n.typ, n.info) - when not scopeBasedDestruction: - c.addTopVar(tmp) + let tmp = getTemp(c, s, n.typ, n.info) if hasDestructor(n.typ): result.add genWasMoved(tmp, c) var m = genCopy(c, tmp, n) - m.add p(n, c, normal) + m.add p(n, c, s, normal) result.add m if isLValue(n) and not isCapturedVar(n) and n.typ.skipTypes(abstractInst).kind != tyRef and c.inSpawn == 0: message(c.graph.config, n.info, hintPerformance, @@ -418,7 +489,7 @@ proc passCopyToSink(n: PNode; c: var Con): PNode = else: if c.graph.config.selectedGC in {gcArc, gcOrc}: assert(not containsGarbageCollectedRef(n.typ)) - result.add newTree(nkAsgn, tmp, p(n, c, normal)) + result.add newTree(nkAsgn, tmp, p(n, c, s, normal)) # Since we know somebody will take over the produced copy, there is # no need to destroy it. result.add tmp @@ -442,161 +513,24 @@ proc containsConstSeq(n: PNode): bool = if containsConstSeq(son): return true else: discard -proc handleTmpDestroys(c: var Con; body: PNode; t: PType; - oldHasUnstructuredCf, oldTmpDestroysLen: int) = - if c.hasUnstructuredCf == oldHasUnstructuredCf: - # no need for a try-finally statement: - if body.kind == nkStmtList: - for i in countdown(c.scopeDestroys.high, oldTmpDestroysLen): - body.add c.scopeDestroys[i] - elif isEmptyType(t): - var n = newNodeI(nkStmtList, body.info) - n.add body[^1] - for i in countdown(c.scopeDestroys.high, oldTmpDestroysLen): - n.add c.scopeDestroys[i] - body[^1] = n - elif body.kind == nkStmtListExpr and body.len > 0 and body[^1].kind == nkSym: - # special case: Do not translate (x; y; sym) into - # (x; y; tmp = sym; destroy(x); destroy(y); tmp ) - # but into - # (x; y; destroy(x); destroy(y); sym ) - let sym = body[^1] - body[^1] = c.scopeDestroys[^1] - for i in countdown(c.scopeDestroys.high - 1, oldTmpDestroysLen): - body.add c.scopeDestroys[i] - body.add sym - else: - # fun ahead: We have to transform (x; y; E()) into - # (x; y; tmp = E(); destroy(x); destroy(y); tmp ) - let t2 = body[^1].typ - let tmp = getTemp(c, t2, body.info) - when not scopeBasedDestruction: - c.addTopVar(tmp) - # the tmp does not have to be initialized - var n = newNodeIT(nkStmtListExpr, body.info, t2) - n.add newTree(nkFastAsgn, tmp, body[^1]) - for i in countdown(c.scopeDestroys.high, oldTmpDestroysLen): - n.add c.scopeDestroys[i] - n.add tmp - body[^1] = n - #c.scopeDestroys.add genDestroy(c, tmp) - else: - # unstructured control flow was used, use a 'try finally' to ensure - # destruction: - if isEmptyType(t): - var n = newNodeI(nkStmtList, body.info) - for i in countdown(c.scopeDestroys.high, oldTmpDestroysLen): - n.add c.scopeDestroys[i] - body[^1] = newTryFinally(body[^1], n) - else: - # fun ahead: We have to transform (x; y; E()) into - # ((try: tmp = (x; y; E()); finally: destroy(x); destroy(y)); tmp ) - let t2 = body[^1].typ - let tmp = getTemp(c, t2, body.info) - when not scopeBasedDestruction: - c.addTopVar(tmp) - # the tmp does not have to be initialized - var fin = newNodeI(nkStmtList, body.info) - for i in countdown(c.scopeDestroys.high, oldTmpDestroysLen): - fin.add c.scopeDestroys[i] - var n = newNodeIT(nkStmtListExpr, body.info, t2) - n.add newTryFinally(newTree(nkFastAsgn, tmp, body[^1]), fin) - n.add tmp - body[^1] = n - #c.scopeDestroys.add genDestroy(c, tmp) - - c.scopeDestroys.setLen oldTmpDestroysLen - -proc handleNested(n, dest: PNode; c: var Con; mode: ProcessMode): PNode = - template processCall(node: PNode): PNode = - if node.typ == nil or dest == nil: - p(node, c, mode) - else: - moveOrCopy(dest, node, c) - - proc handleScope(n, dest: PNode; t: PType; - takeOver: Natural; c: var Con; mode: ProcessMode): PNode = - let oldHasUnstructuredCf = c.hasUnstructuredCf - let oldTmpDestroysLen = c.scopeDestroys.len - result = shallowCopy(n) - for i in 0.. oldTmpDestroysLen: - handleTmpDestroys(c, result, t, oldHasUnstructuredCf, oldTmpDestroysLen) - else: - setLen(result.sons, last) - if c.scopeDestroys.len > oldTmpDestroysLen: - handleTmpDestroys(c, result, t, oldHasUnstructuredCf, oldTmpDestroysLen) - if result.kind != nkFinally: - result.add processCall(n[last]) - else: - result = newTree(nkStmtListExpr, result, processCall(n[last])) - result.typ = t - - case n.kind - of nkStmtList, nkStmtListExpr: - if n.len == 0: return n - result = shallowCopy(n) - let last = n.len - 1 - for i in 0.. 0: # unpacked tuple needs reset at every loop iteration res.add newTree(nkFastAsgn, v, genDefaultCall(v.typ, c, v.info)) elif sfThread notin v.sym.flags: # do not destroy thread vars for now at all for consistency. - if sfGlobal in v.sym.flags: + if sfGlobal in v.sym.flags and s.parent == nil: c.graph.globalDestructors.add genDestroy(c, v) else: - c.destroys.add genDestroy(c, v) + s.final.add genDestroy(c, v) if ri.kind == nkEmpty and c.inLoop > 0: - res.add moveOrCopy(v, genDefaultCall(v.typ, c, v.info), c, isDecl = true) + res.add moveOrCopy(v, genDefaultCall(v.typ, c, v.info), c, s, isDecl = true) elif ri.kind != nkEmpty: - res.add moveOrCopy(v, ri, c, isDecl = true) - -proc pVarScoped(v: PNode; c: var Con; ri, res: PNode) = - if not containsOrIncl(c.declaredVars, v.sym.id): - c.addTopVar(v) - if isUnpackedTuple(v): - if c.inLoop > 0: - # unpacked tuple needs reset at every loop iteration - res.add newTree(nkFastAsgn, v, genDefaultCall(v.typ, c, v.info)) - elif {sfGlobal, sfThread} * v.sym.flags == {sfGlobal}: - c.graph.globalDestructors.add genDestroy(c, v) - else: - # We always translate 'var v = f()' into bitcopies. If 'v' is in a loop, - # the destruction at the loop end will free the resources. Other assignments - # will destroy the old value inside 'v'. If we have 'var v' without an initial - # default value we translate it into 'var v = default()'. We translate - # 'var x = someGlobal' into 'var v = default(); `=`(v, someGlobal). The - # lack of copy constructors is really beginning to hurt us. :-( - #if c.inDangerousBranch == 0: v.sym.flags.incl sfNoInit - c.scopeDestroys.add genDestroy(c, v) - if ri.kind == nkEmpty and c.inLoop > 0: - res.add moveOrCopy(v, genDefaultCall(v.typ, c, v.info), c, isDecl = true) - elif ri.kind != nkEmpty: - res.add moveOrCopy(v, ri, c, isDecl = true) + res.add moveOrCopy(v, ri, c, s, isDecl = true) template handleNestedTempl(n: untyped, processCall: untyped) = + template maybeVoid(child, s): untyped = + if isEmptyType(child.typ): p(child, c, s, normal) + else: processCall(child, s) + case n.kind of nkStmtList, nkStmtListExpr: + # a statement list does not open a new scope if n.len == 0: return n result = copyNode(n) for i in 0.. 0): - result[i] = p(n[i], c, sinkArg) + result[i] = p(n[i], c, s, sinkArg) else: - result[i] = p(n[i], c, normal) + result[i] = p(n[i], c, s, normal) - if isDangerous: - dec c.inDangerousBranch + when false: + if isDangerous: + dec c.inDangerousBranch if n[0].kind == nkSym and n[0].sym.magic in {mNew, mNewFinalize}: result[0] = copyTree(n[0]) @@ -873,15 +805,14 @@ proc p(n: PNode; c: var Con; mode: ProcessMode): PNode = let destroyOld = genDestroy(c, result[1]) result = newTree(nkStmtList, destroyOld, result) else: - result[0] = p(n[0], c, normal) - when scopeBasedDestruction: - if canRaise(n[0]): inc c.hasUnstructuredCf + result[0] = p(n[0], c, s, normal) + if canRaise(n[0]): s.needsTry = true if mode == normal: - result = ensureDestruction(result, c) + result = ensureDestruction(result, c, s) of nkDiscardStmt: # Small optimization result = shallowCopy(n) if n[0].kind != nkEmpty: - result[0] = p(n[0], c, normal) + result[0] = p(n[0], c, s, normal) else: result[0] = copyNode(n[0]) of nkVarSection, nkLetSection: @@ -891,27 +822,24 @@ proc p(n: PNode; c: var Con; mode: ProcessMode): PNode = var ri = it[^1] if it.kind == nkVarTuple and hasDestructor(ri.typ): let x = lowerTupleUnpacking(c.graph, it, c.owner) - result.add p(x, c, consumed) + result.add p(x, c, s, consumed) elif it.kind == nkIdentDefs and hasDestructor(it[0].typ) and not isCursor(it[0]): for j in 0.. 0: ri = genDefaultCall(v.typ, c, v.info) if ri.kind != nkEmpty: - result.add moveOrCopy(v, ri, c, isDecl = true) + result.add moveOrCopy(v, ri, c, s, isDecl = true) 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) - result.add p(ri, c, consumed) + result.add p(ri, c, s, consumed) else: - result = genSink(c, dest, p(ri, c, consumed), isDecl) + result = genSink(c, s, dest, p(ri, c, s, consumed), isDecl) of nkObjConstr, nkTupleConstr, nkClosure, nkCharLit..nkNilLit: - result = genSink(c, dest, p(ri, c, consumed), isDecl) + result = genSink(c, s, dest, p(ri, c, s, consumed), isDecl) of nkSym: if isSinkParam(ri.sym) and isLastRead(ri, c): # Rule 3: `=sink`(x, z); wasMoved(z) - #sinkParamIsLastReadCheck(c, ri) - let snk = genSink(c, dest, ri, isDecl) + let snk = genSink(c, s, dest, ri, isDecl) result = newTree(nkStmtList, snk, genWasMoved(ri, c)) elif ri.sym.kind != skParam and ri.sym.owner == c.owner and isLastRead(ri, c) and canBeMoved(c, dest.typ): # Rule 3: `=sink`(x, z); wasMoved(z) - let snk = genSink(c, dest, ri, isDecl) + let snk = genSink(c, s, dest, ri, isDecl) result = newTree(nkStmtList, snk, genWasMoved(ri, c)) else: result = genCopy(c, dest, ri) - result.add p(ri, c, consumed) - of nkHiddenSubConv, nkHiddenStdConv, nkConv: - when false: - result = moveOrCopy(dest, ri[1], c, isDecl) - if not sameType(ri.typ, ri[1].typ): - let copyRi = copyTree(ri) - copyRi[1] = result[^1] - result[^1] = copyRi - else: - result = genSink(c, dest, p(ri, c, sinkArg), isDecl) - of nkObjDownConv, nkObjUpConv: - when false: - result = moveOrCopy(dest, ri[0], c, isDecl) - let copyRi = copyTree(ri) - copyRi[0] = result[^1] - result[^1] = copyRi - else: - result = genSink(c, dest, p(ri, c, sinkArg), isDecl) + result.add p(ri, c, s, consumed) + of nkHiddenSubConv, nkHiddenStdConv, nkConv, nkObjDownConv, nkObjUpConv: + result = genSink(c, s, dest, p(ri, c, s, sinkArg), isDecl) of nkStmtListExpr, nkBlockExpr, nkIfExpr, nkCaseStmt: - when scopeBasedDestruction: - result = handleNested(ri, dest, c, normal) - else: - handleNestedTempl(ri): moveOrCopy(dest, node, c, isDecl) + template process(child, s): untyped = moveOrCopy(dest, child, c, s, isDecl) + handleNestedTempl(ri, process) + of nkRaiseStmt: + result = pRaiseStmt(ri, c, s) else: if isAnalysableFieldAccess(ri, c.owner) and isLastRead(ri, c) and canBeMoved(c, dest.typ): # Rule 3: `=sink`(x, z); wasMoved(z) - let snk = genSink(c, dest, ri, isDecl) + let snk = genSink(c, s, dest, ri, isDecl) result = newTree(nkStmtList, snk, genWasMoved(ri, c)) else: result = genCopy(c, dest, ri) - result.add p(ri, c, consumed) + result.add p(ri, c, s, consumed) proc computeUninit(c: var Con) = if not c.uninitComputed: @@ -1102,30 +998,19 @@ proc injectDestructorCalls*(g: ModuleGraph; owner: PSym; n: PNode): PNode = echo "\n### ", owner.name.s, ":\nCFG:" echoCfg(c.g) echo n + + var scope: Scope + let body = p(n, c, scope, normal) + if owner.kind in {skProc, skFunc, skMethod, skIterator, skConverter}: let params = owner.typ.n for i in 1.. 0: - result.add c.topLevelVars - if c.destroys.len > 0 or c.scopeDestroys.len > 0: - reverse c.destroys.sons - var fin: PNode - if owner.kind == skModule: - fin = newTryFinally(body, extractDestroysForTemporaries(c, c.destroys)) - g.globalDestructors.add c.destroys - else: - fin = newTryFinally(body, c.destroys) - for i in countdown(c.scopeDestroys.high, 0): fin[1][0].add c.scopeDestroys[i] - result.add fin - else: - result.add body + result = toTree(scope, body) dbg: echo ">---------transformed-to--------->" echo renderTree(result, {renderIds}) diff --git a/compiler/semstmts.nim b/compiler/semstmts.nim index a2515357b2..c88ff36095 100644 --- a/compiler/semstmts.nim +++ b/compiler/semstmts.nim @@ -229,8 +229,8 @@ proc semTry(c: PContext, n: PNode; flags: TExprFlags): PNode = a[0][2] = newSymNode(symbol, a[0][2].info) elif a.len == 1: - # count number of ``except: body`` blocks - inc catchAllExcepts + # count number of ``except: body`` blocks + inc catchAllExcepts else: # support ``except KeyError, ValueError, ... : body`` diff --git a/compiler/spawn.nim b/compiler/spawn.nim index 9d837b993e..15bed77b0b 100644 --- a/compiler/spawn.nim +++ b/compiler/spawn.nim @@ -10,7 +10,7 @@ ## This module implements threadpool's ``spawn``. import ast, types, idents, magicsys, msgs, options, modulegraphs, - lowerings, liftdestructors + lowerings, liftdestructors, renderer from trees import getMagic, getRoot proc callProc(a: PNode): PNode = @@ -321,7 +321,7 @@ proc wrapProcForSpawn*(g: ModuleGraph; owner: PSym; spawnExpr: PNode; retType: P result = newNodeI(nkStmtList, n.info) if n.kind notin nkCallKinds: - localError(g.config, n.info, "'spawn' takes a call expression") + localError(g.config, n.info, "'spawn' takes a call expression; got " & $n) return if optThreadAnalysis in g.config.globalOptions: if {tfThread, tfNoSideEffect} * n[0].typ.flags == {}: diff --git a/doc/destructors.rst b/doc/destructors.rst index b06d78bbe0..4285bad8b1 100644 --- a/doc/destructors.rst +++ b/doc/destructors.rst @@ -283,8 +283,8 @@ Rewrite rules around the complete routine body. 2. The produced ``finally`` section is wrapped around the enclosing scope. -The current implementation follows strategy (1). This means that resources are -not destroyed at the scope exit, but at the proc exit. +The current implementation follows strategy (2). This means that resources are +destroyed at the scope exit. :: diff --git a/tests/arc/tarcmisc.nim b/tests/arc/tarcmisc.nim index 898efd138c..e66d9a75f0 100644 --- a/tests/arc/tarcmisc.nim +++ b/tests/arc/tarcmisc.nim @@ -173,3 +173,39 @@ proc bug14495 = echo o[] bug14495() + +when false: + # bug #14396 + type + Spinny = ref object + t: ref int + text: string + + proc newSpinny*(): Spinny = + Spinny(t: new(int), text: "hello") + + proc spinnyLoop(x: ref int, spinny: sink Spinny) = + echo x[] + + proc start*(spinny: sink Spinny) = + spinnyLoop(spinny.t, spinny) + + var spinner1 = newSpinny() + spinner1.start() + + # bug #14345 + + type + SimpleLoopB = ref object + children: seq[SimpleLoopB] + parent: SimpleLoopB + + proc addChildLoop(self: SimpleLoopB, loop: SimpleLoopB) = + self.children.add loop + + proc setParent(self: SimpleLoopB, parent: SimpleLoopB) = + self.parent = parent + self.parent.addChildLoop(self) + + var l = SimpleLoopB() + l.setParent(l) diff --git a/tests/arc/tcontrolflow.nim b/tests/arc/tcontrolflow.nim index efe51cc32f..478806567b 100644 --- a/tests/arc/tcontrolflow.nim +++ b/tests/arc/tcontrolflow.nim @@ -11,15 +11,13 @@ begin true if end true 7 +##index 2 not in 0 .. 1## ''' cmd: "nim c --gc:arc -d:danger $file" - disabled: "true" """ # we use the -d:danger switch to detect uninitialized stack # slots more reliably (there shouldn't be any, of course). -# XXX Enable once scope based destruction works! - type Foo = object id: int @@ -68,12 +66,25 @@ proc run(data: Control) = evt.control = data if evt.button == 1: discard - else: + else: return - + echo data.x var c = Control(x: 7) run(c) +proc sysFatal(exceptn: typedesc, message: string) {.inline, noreturn.} = + var buf = newStringOfCap(200) + add(buf, "##") + add(buf, message) + add(buf, "##") + echo buf + +proc ifexpr(i, a, b: int) {.compilerproc, noinline.} = + sysFatal(IndexDefect, + if b < a: "index out of bounds, the container is empty" + else: "index " & $i & " not in " & $a & " .. " & $b) + +ifexpr(2, 0, 1) diff --git a/tests/arc/tmovebug.nim b/tests/arc/tmovebug.nim index 951979a462..ec0fce9a82 100644 --- a/tests/arc/tmovebug.nim +++ b/tests/arc/tmovebug.nim @@ -247,7 +247,7 @@ iterator combinations[T](s: openarray[T], k: int): seq[T] = break type - UndefEx = object of Exception + UndefEx = object of ValueError proc main2 = var delayedSyms = @[1, 2, 3] @@ -318,10 +318,10 @@ proc `=sink`(dest: var O2, src: O2) = var testSeq: O2 -proc Update(): void = +proc update() = # testSeq.add(0) # uncommenting this line fixes the leak testSeq = O2(s: @[]) testSeq.s.add(0) for i in 1..3: - Update() + update() diff --git a/tests/arc/tthread.nim b/tests/arc/tthread.nim index c653c753f8..8a55a666e7 100644 --- a/tests/arc/tthread.nim +++ b/tests/arc/tthread.nim @@ -14,7 +14,7 @@ type p: int MyObjRef = ref MyObj -proc `=destroy`(x: var MyObj) = +proc `=destroy`(x: var MyObj) = if x.p != 0: echo "destroyed" @@ -48,16 +48,16 @@ proc thread5(x: sink MyObjRef): MyObjRef = os.sleep(1000) result = x -proc ref_forwarding_test = +proc ref_forwarding_test = var x = new(MyObj) x[].p = 2 var y = spawn thread4(x) - -proc ref_sink_forwarding_test = + +proc ref_sink_forwarding_test = var x = new(MyObj) x[].p = 2 var y = spawn thread5(x) ref_forwarding_test() -ref_sink_forwarding_test() +ref_sink_forwarding_test() sync() diff --git a/tests/arc/tweave.nim b/tests/arc/tweave.nim new file mode 100644 index 0000000000..220d65f97d --- /dev/null +++ b/tests/arc/tweave.nim @@ -0,0 +1,154 @@ +discard """ + outputsub: '''Success''' + cmd: '''nim c --gc:arc --threads:on $file''' + disabled: "bsd" +""" + +# bug #13936 + +import std/atomics + +const MemBlockSize = 256 + +type + ChannelSPSCSingle* = object + full{.align: 128.}: Atomic[bool] + itemSize*: uint8 + buffer*{.align: 8.}: UncheckedArray[byte] + +proc `=`( + dest: var ChannelSPSCSingle, + source: ChannelSPSCSingle + ) {.error: "A channel cannot be copied".} + +proc initialize*(chan: var ChannelSPSCSingle, itemsize: SomeInteger) {.inline.} = + ## If ChannelSPSCSingle is used intrusive another data structure + ## be aware that it should be the last part due to ending by UncheckedArray + ## Also due to 128 bytes padding, it automatically takes half + ## of the default MemBlockSize + assert itemsize.int in 0 .. int high(uint8) + assert itemSize.int + + sizeof(chan.itemsize) + + sizeof(chan.full) < MemBlockSize + + chan.itemSize = uint8 itemsize + chan.full.store(false, moRelaxed) + +func isEmpty*(chan: var ChannelSPSCSingle): bool {.inline.} = + not chan.full.load(moAcquire) + +func tryRecv*[T](chan: var ChannelSPSCSingle, dst: var T): bool {.inline.} = + ## Try receiving the item buffered in the channel + ## Returns true if successful (channel was not empty) + ## + ## ⚠ Use only in the consumer thread that reads from the channel. + assert (sizeof(T) == chan.itemsize.int) or + # Support dummy object + (sizeof(T) == 0 and chan.itemsize == 1) + + let full = chan.full.load(moAcquire) + if not full: + return false + dst = cast[ptr T](chan.buffer.addr)[] + chan.full.store(false, moRelease) + return true + +func trySend*[T](chan: var ChannelSPSCSingle, src: sink T): bool {.inline.} = + ## Try sending an item into the channel + ## Reurns true if successful (channel was empty) + ## + ## ⚠ Use only in the producer thread that writes from the channel. + assert (sizeof(T) == chan.itemsize.int) or + # Support dummy object + (sizeof(T) == 0 and chan.itemsize == 1) + + let full = chan.full.load(moAcquire) + if full: + return false + cast[ptr T](chan.buffer.addr)[] = src + chan.full.store(true, moRelease) + return true + +# Sanity checks +# ------------------------------------------------------------------------------ +when isMainModule: + + when not compileOption("threads"): + {.error: "This requires --threads:on compilation flag".} + + template sendLoop[T](chan: var ChannelSPSCSingle, + data: sink T, + body: untyped): untyped = + while not chan.trySend(data): + body + + template recvLoop[T](chan: var ChannelSPSCSingle, + data: var T, + body: untyped): untyped = + while not chan.tryRecv(data): + body + + type + ThreadArgs = object + ID: WorkerKind + chan: ptr ChannelSPSCSingle + + WorkerKind = enum + Sender + Receiver + + template Worker(id: WorkerKind, body: untyped): untyped {.dirty.} = + if args.ID == id: + body + + proc thread_func(args: ThreadArgs) = + + # Worker RECEIVER: + # --------- + # <- chan + # <- chan + # <- chan + # + # Worker SENDER: + # --------- + # chan <- 42 + # chan <- 53 + # chan <- 64 + Worker(Receiver): + var val: int + for j in 0 ..< 10: + args.chan[].recvLoop(val): + # Busy loop, in prod we might want to yield the core/thread timeslice + discard + echo " Receiver got: ", val + doAssert val == 42 + j*11 + + Worker(Sender): + doAssert args.chan.full.load(moRelaxed) == false + for j in 0 ..< 10: + let val = 42 + j*11 + args.chan[].sendLoop(val): + # Busy loop, in prod we might want to yield the core/thread timeslice + discard + echo "Sender sent: ", val + + proc main() = + echo "Testing if 2 threads can send data" + echo "-----------------------------------" + var threads: array[2, Thread[ThreadArgs]] + + var chan = cast[ptr ChannelSPSCSingle](allocShared(MemBlockSize)) + chan[].initialize(itemSize = sizeof(int)) + + createThread(threads[0], thread_func, ThreadArgs(ID: Receiver, chan: chan)) + createThread(threads[1], thread_func, ThreadArgs(ID: Sender, chan: chan)) + + joinThread(threads[0]) + joinThread(threads[1]) + + freeShared(chan) + + echo "-----------------------------------" + echo "Success" + + main() diff --git a/tests/async/testmanyasyncevents.nim b/tests/async/testmanyasyncevents.nim index 5a103736fd..9fdd01b4fb 100644 --- a/tests/async/testmanyasyncevents.nim +++ b/tests/async/testmanyasyncevents.nim @@ -1,8 +1,9 @@ discard """ output: ''' hasPendingOperations: false -triggerCount: 100 +triggerCount: 100 ''' +disabled: "windows" """ import asyncDispatch diff --git a/tests/casestmt/t12785.nim b/tests/casestmt/t12785.nim index bf0f30d425..7177fb9c27 100644 --- a/tests/casestmt/t12785.nim +++ b/tests/casestmt/t12785.nim @@ -3,6 +3,8 @@ discard """ output: '''copied copied 2 +destroyed +destroyed copied copied 2 @@ -37,9 +39,9 @@ proc test(a: range[0..1], arg: ObjWithDestructor) = inc iteration case a - of 0: - assert false - of 1: + of 0: + assert false + of 1: echo b if iteration == 2: break diff --git a/tests/destructor/tdestructor.nim b/tests/destructor/tdestructor.nim index 9fd47fe001..e081eb251d 100644 --- a/tests/destructor/tdestructor.nim +++ b/tests/destructor/tdestructor.nim @@ -1,27 +1,27 @@ discard """ - output: '''---- + output: '''----1 myobj constructed myobj destroyed ----- +----2 mygeneric1 constructed mygeneric1 destroyed ----- +----3 mygeneric2 constructed mygeneric2 destroyed myobj destroyed ----- +----4 mygeneric3 constructed mygeneric1 destroyed ----- +----5 mydistinctObj constructed myobj destroyed mygeneric2 destroyed ------------------- ----- ----- -myobj destroyed +------------------8 mygeneric1 destroyed ---- +----6 +myobj destroyed +----7 +---9 myobj destroyed myobj destroyed ''' @@ -114,19 +114,19 @@ proc mydistinctObj = echo "mydistinctObj constructed" -echo "----" +echo "----1" myobj() -echo "----" +echo "----2" mygeneric1() -echo "----" +echo "----3" mygeneric2[int](10) -echo "----" +echo "----4" mygeneric3() -echo "----" +echo "----5" mydistinctObj() proc caseobj = @@ -134,16 +134,16 @@ proc caseobj = var o1 = TCaseObj(kind: A, x: TMyGeneric1[int](x: 10)) block: - echo "----" + echo "----6" var o2 = TCaseObj(kind: B, y: open()) block: - echo "----" + echo "----7" var o3 = TCaseObj(kind: D, innerKind: B, r: "test", p: TMyGeneric3[int, float, string](x: 10, y: 1.0, z: "test")) -echo "------------------" +echo "------------------8" caseobj() proc caseobj_test_sink: TCaseObj = @@ -153,7 +153,7 @@ proc caseobj_test_sink: TCaseObj = result = TCaseObj(kind: B, y: open()) -echo "---" +echo "---9" discard caseobj_test_sink() # issue #14315