From c5358b0d4b1d27db04b974a0183c9b7312bc7bdc Mon Sep 17 00:00:00 2001 From: Andreas Rumpf Date: Wed, 15 Jul 2020 23:00:06 +0200 Subject: [PATCH] An optimizer for ARC (#14962) * WIP: an optimizer for ARC * do not optimize away destructors in 'finally' if unstructured control flow is involved * optimized the optimizer * minor code cleanup * first steps to .cursor inference * cursor inference: big steps to a working solution * baby steps * better .cursor inference * new feature: expandArc for easy inspection of the AST after ARC transformations * added topt_cursor test * adapt tests * cleanups, make tests green * optimize common traversal patterns * moved test case * fixes .cursor inference so that npeg compiles once again * cursor inference: more bugfixes Co-authored-by: Clyybber --- compiler/commands.nim | 3 + compiler/cursor_inference.nim | 295 ++++++++++++++++++ compiler/injectdestructors.nim | 54 ++-- compiler/optimizer.nim | 285 +++++++++++++++++ compiler/options.nim | 2 + compiler/renderer.nim | 21 +- compiler/transf.nim | 2 +- doc/advopt.txt | 2 + doc/destructors.rst | 28 +- lib/impure/nre.nim | 10 +- .../t12785.nim => arc/tcomputedgoto.nim} | 12 +- tests/arc/topt_cursor.nim | 35 +++ tests/arc/topt_no_cursor.nim | 37 +++ tests/arc/topt_refcursors.nim | 39 +++ tests/arc/topt_wasmoved_destroy_pairs.nim | 92 ++++++ tests/{generics => arc}/trtree.nim | 18 +- tests/destructor/tdestructor3.nim | 11 +- tests/destructor/tmisc_destructors.nim | 6 +- .../destructor/tuse_result_prevents_sinks.nim | 5 + 19 files changed, 895 insertions(+), 62 deletions(-) create mode 100644 compiler/cursor_inference.nim create mode 100644 compiler/optimizer.nim rename tests/{casestmt/t12785.nim => arc/tcomputedgoto.nim} (87%) create mode 100644 tests/arc/topt_cursor.nim create mode 100644 tests/arc/topt_no_cursor.nim create mode 100644 tests/arc/topt_refcursors.nim create mode 100644 tests/arc/topt_wasmoved_destroy_pairs.nim rename tests/{generics => arc}/trtree.nim (98%) diff --git a/compiler/commands.nim b/compiler/commands.nim index 1984c894a..d0936a0c6 100644 --- a/compiler/commands.nim +++ b/compiler/commands.nim @@ -866,6 +866,9 @@ proc processSwitch*(switch, arg: string, pass: TCmdLinePass, info: TLineInfo; of "expandmacro": expectArg(conf, switch, arg, pass, info) conf.macrosToExpand[arg] = "T" + of "expandarc": + expectArg(conf, switch, arg, pass, info) + conf.arcToExpand[arg] = "T" of "oldgensym": processOnOffSwitchG(conf, {optNimV019}, arg, pass, info) of "useversion": diff --git a/compiler/cursor_inference.nim b/compiler/cursor_inference.nim new file mode 100644 index 000000000..7806a535f --- /dev/null +++ b/compiler/cursor_inference.nim @@ -0,0 +1,295 @@ +# +# +# The Nim Compiler +# (c) Copyright 2020 Andreas Rumpf +# +# See the file "copying.txt", included in this +# distribution, for details about the copyright. +# + +## Cursor inference: +## The basic idea was like this: Elide 'destroy(x)' calls if only +## special literals are assigned to 'x' and 'x' is not mutated or +## passed by 'var T' to something else. Special literals are string literals or +## arrays / tuples of string literals etc. +## +## However, there is a much more general rule here: Compute which variables +## can be annotated with `.cursor`. To see how and when we can do that, +## think about this question: In `dest = src` when do we really have to +## *materialize* the full copy? - Only if `dest` or `src` are mutated +## afterwards. `dest` is the potential cursor variable, so that is +## simple to analyse. And if `src` is a location derived from a +## formal parameter, we also know it is not mutated! In other words, we +## do a compile-time copy-on-write analysis. + +import + ast, types, renderer, idents, intsets, options, msgs + +type + Cursor = object + s: PSym + deps: IntSet + Con = object + cursors: seq[Cursor] + mayOwnData: IntSet + mutations: IntSet + reassigns: IntSet + config: ConfigRef + +proc locationRoot(e: PNode; followDotExpr = true): PSym = + var n = e + while true: + case n.kind + of nkSym: + if n.sym.kind in {skVar, skResult, skTemp, skLet, skForVar, skParam}: + return n.sym + else: + return nil + of nkDotExpr, nkDerefExpr, nkBracketExpr, nkHiddenDeref, + nkCheckedFieldExpr, nkAddr, nkHiddenAddr: + if followDotExpr: + n = n[0] + else: + return nil + of nkObjUpConv, nkObjDownConv: + n = n[0] + of nkHiddenStdConv, nkHiddenSubConv, nkConv, nkCast: + n = n[1] + of nkStmtList, nkStmtListExpr: + if n.len > 0: + n = n[^1] + else: + return nil + else: + return nil + +proc addDep(c: var Con; dest: var Cursor; dependsOn: PSym) = + if dest.s != dependsOn: + dest.deps.incl dependsOn.id + +proc cursorId(c: Con; x: PSym): int = + for i in 0.. 0: + analyseAsgn(c, dest, n[^1]) + + of nkClosure: + for i in 1.. treat it like a sink parameter + c.mayOwnData.incl r.id + c.mutations.incl r.id + +proc analyse(c: var Con; n: PNode) = + case n.kind + of nkCallKinds: + let parameters = n[0].typ + let L = if parameters != nil: parameters.len else: 0 + + analyse(c, n[0]) + for i in 1..= 0: + analyseAsgn(c, c.cursors[idx], n[1]) + + c.reassigns.incl n[0].sym.id + else: + # assignments like 'x.field = value' mean that 'x' itself cannot + # be a cursor: + let r = locationRoot(n[0]) + if r != nil and r.typ.skipTypes(abstractInst).kind notin {tyPtr, tyRef}: + # however, an assignment like 'it.field = x' does not influence r's + # cursorness property: + c.mayOwnData.incl r.id + c.mutations.incl r.id + + if hasDestructor(n[1].typ): + rhsIsSink(c, n[1]) + + of nkAddr, nkHiddenAddr: + analyse(c, n[0]) + let r = locationRoot(n[0]) + if r != nil: + c.mayOwnData.incl r.id + c.mutations.incl r.id + + of nkTupleConstr, nkBracket, nkObjConstr: + for i in ord(n.kind == nkObjConstr)..---------transformed-to--------->" echo renderTree(result, {renderIds}) + + if g.config.arcToExpand.hasKey(owner.name.s): + echo "--expandArc: ", owner.name.s + echo renderTree(result, {renderIr}) + echo "-- end of expandArc ------------------------" diff --git a/compiler/optimizer.nim b/compiler/optimizer.nim new file mode 100644 index 000000000..5d1139bfd --- /dev/null +++ b/compiler/optimizer.nim @@ -0,0 +1,285 @@ +# +# +# The Nim Compiler +# (c) Copyright 2020 Andreas Rumpf +# +# See the file "copying.txt", included in this +# distribution, for details about the copyright. +# + +## Optimizer: +## - elide 'wasMoved(x); destroy(x)' pairs +## - recognize "all paths lead to 'wasMoved(x)'" + +import + ast, renderer, idents, intsets + +from trees import exprStructuralEquivalent + +const + nfMarkForDeletion = nfNone # faster than a lookup table + +type + BasicBlock = object + wasMovedLocs: seq[PNode] + kind: TNodeKind + hasReturn, hasBreak: bool + label: PSym # can be nil + parent: ptr BasicBlock + + Con = object + somethingTodo: bool + inFinally: int + +proc nestedBlock(parent: var BasicBlock; kind: TNodeKind): BasicBlock = + BasicBlock(wasMovedLocs: @[], kind: kind, hasReturn: false, hasBreak: false, + label: nil, parent: addr(parent)) + +proc breakStmt(b: var BasicBlock; n: PNode) = + var it = addr(b) + while it != nil: + it.wasMovedLocs.setLen 0 + it.hasBreak = true + + if n.kind == nkSym: + if it.label == n.sym: break + else: + # unnamed break leaves the block is nkWhileStmt or the like: + if it.kind in {nkWhileStmt, nkBlockStmt, nkBlockExpr}: break + + it = it.parent + +proc returnStmt(b: var BasicBlock) = + b.hasReturn = true + var it = addr(b) + while it != nil: + it.wasMovedLocs.setLen 0 + it = it.parent + +proc mergeBasicBlockInfo(parent: var BasicBlock; this: BasicBlock) {.inline.} = + if this.hasReturn: + parent.wasMovedLocs.setLen 0 + parent.hasReturn = true + +proc wasMovedTarget(matches: var IntSet; branch: seq[PNode]; moveTarget: PNode): bool = + result = false + for i in 0.. 0 and (b.hasReturn or b.hasBreak): + discard "cannot optimize away the destructor" + else: + c.wasMovedDestroyPair b, n + special = true + elif s.name.s == "=sink": + reverse = true + + if not special: + if not reverse: + for i in 0 ..< n.len: + analyse(c, b, n[i]) + else: + #[ Test tmatrix.test3: + Prevent this from being elided. We should probably + find a better solution... + + `=sink`(b, - ( + let blitTmp = b; + wasMoved(b); + blitTmp + a) + `=destroy`(b) + + ]# + for i in countdown(n.len-1, 0): + analyse(c, b, n[i]) + if canRaise(n[0]): returnStmt(b) + + of nkSym: + # any usage of the location before destruction implies we + # cannot elide the 'wasMoved(x)': + b.invalidateWasMoved n + + of nkNone..pred(nkSym), succ(nkSym)..nkNilLit, nkTypeSection, nkProcDef, nkConverterDef, + nkMethodDef, nkIteratorDef, nkMacroDef, nkTemplateDef, nkLambda, nkDo, + nkFuncDef, nkConstSection, nkConstDef, nkIncludeStmt, nkImportStmt, + nkExportStmt, nkPragma, nkCommentStmt, nkBreakState, nkTypeOfExpr: + discard "do not follow the construct" + + of nkAsgn, nkFastAsgn: + # reverse order, see remark for `=sink`: + analyse(c, b, n[1]) + analyse(c, b, n[0]) + + of nkIfStmt, nkIfExpr: + let isExhaustive = n[^1].kind in {nkElse, nkElseExpr} + var wasMovedSet: seq[PNode] = @[] + + for i in 0 ..< n.len: + var branch = nestedBlock(b, n[i].kind) + + analyse(c, branch, n[i]) + mergeBasicBlockInfo(b, branch) + if isExhaustive: + if i == 0: + wasMovedSet = move(branch.wasMovedLocs) + else: + wasMovedSet.intersect(branch.wasMovedLocs) + for i in 0..= ord(tokKeywordLow) - ord(tkSymbol)) and (i.id <= ord(tokKeywordHigh) - ord(tkSymbol)): @@ -850,7 +860,12 @@ proc gident(g: var TSrcGen, n: PNode) = t = tkSymbol else: t = tkOpr - if n.kind == nkSym and (renderIds in g.flags or sfGenSym in n.sym.flags or n.sym.kind == skTemp): + if renderIr in g.flags and n.kind == nkSym: + let localId = disamb(g, n.sym) + if localId != 0 and n.sym.magic == mNone: + s.add '_' + s.addInt localId + elif n.kind == nkSym and (renderIds in g.flags or sfGenSym in n.sym.flags or n.sym.kind == skTemp): s.add '_' s.addInt n.sym.id when defined(debugMagics): @@ -1022,7 +1037,7 @@ proc gsub(g: var TSrcGen, n: PNode, c: TContext) = else: put(g, tkSymbol, "(wrong conv)") of nkHiddenCallConv: - if renderIds in g.flags: + if {renderIds, renderIr} * g.flags != {}: accentedName(g, n[0]) put(g, tkParLe, "(") gcomma(g, n, 1) diff --git a/compiler/transf.nim b/compiler/transf.nim index 10a2680ae..be559abb8 100644 --- a/compiler/transf.nim +++ b/compiler/transf.nim @@ -233,7 +233,7 @@ proc hasContinue(n: PNode): bool = proc newLabel(c: PTransf, n: PNode): PSym = result = newSym(skLabel, nil, getCurrOwner(c), n.info) - result.name = getIdent(c.graph.cache, genPrefix & $result.id) + result.name = getIdent(c.graph.cache, genPrefix) proc transformBlock(c: PTransf, n: PNode): PNode = var labl: PSym diff --git a/doc/advopt.txt b/doc/advopt.txt index 135f297c4..6665ce537 100644 --- a/doc/advopt.txt +++ b/doc/advopt.txt @@ -122,6 +122,8 @@ Advanced options: use the provided namespace for the generated C++ code, if no namespace is provided "Nim" will be used --expandMacro:MACRO dump every generated AST from MACRO + --expandArc:PROCNAME show how PROCNAME looks like after diverse optimizations + before the final backend phase (mostly ARC/ORC specific) --excludePath:PATH exclude a path from the list of search paths --dynlibOverride:SYMBOL marks SYMBOL so that dynlib:SYMBOL has no effect and can be statically linked instead; diff --git a/doc/destructors.rst b/doc/destructors.rst index fefc646f5..6cab6e1d2 100644 --- a/doc/destructors.rst +++ b/doc/destructors.rst @@ -354,7 +354,8 @@ Destructor removal ``wasMoved(x);`` followed by a `=destroy(x)` operation cancel each other out. An implementation is encouraged to exploit this in order to improve -efficiency and code sizes. +efficiency and code sizes. The current implementation does perform this +optimization. Self assignments @@ -509,6 +510,31 @@ to be safe, but for ``ptr`` the compiler has to remain silent about possible problems. +Cursor inference / copy elision +=============================== + +The current implementation also performs `.cursor` inference. Cursor inference is +a form of copy elision. + +To see how and when we can do that, think about this question: In `dest = src` when +do we really have to *materialize* the full copy? - Only if `dest` or `src` are mutated +afterwards. If `dest` is a local variable that is simple to analyse. And if `src` is a +location derived from a formal parameter, we also know it is not mutated! In other +words, we do a compile-time copy-on-write analysis. + +This means that "borrowed" views can be written naturally and without explicit pointer +indirections: + +.. code-block:: nim + + proc main(tab: Table[string, string]) = + let v = tab["key"] # inferred as .cursor because 'tab' is not mutated. + # no copy into 'v', no destruction of 'v'. + use(v) + useItAgain(v) + + + Owned refs ========== diff --git a/lib/impure/nre.nim b/lib/impure/nre.nim index 653d4d1c5..0817ca4ec 100644 --- a/lib/impure/nre.nim +++ b/lib/impure/nre.nim @@ -498,7 +498,7 @@ proc re*(pattern: string): Regex = initRegex(pattern, flags, study) proc matchImpl(str: string, pattern: Regex, start, endpos: int, flags: int): Option[RegexMatch] = - var myResult = RegexMatch(pattern : pattern, str : str) + var myResult = RegexMatch(pattern: pattern, str: str) # See PCRE man pages. # 2x capture count to make room for start-end pairs # 1x capture count as slack space for PCRE @@ -528,13 +528,13 @@ proc matchImpl(str: string, pattern: Regex, start, endpos: int, flags: int): Opt of pcre.ERROR_NULL: raise newException(AccessViolationDefect, "Expected non-null parameters") of pcre.ERROR_BADOPTION: - raise RegexInternalError(msg : "Unknown pattern flag. Either a bug or " & + raise RegexInternalError(msg: "Unknown pattern flag. Either a bug or " & "outdated PCRE.") of pcre.ERROR_BADUTF8, pcre.ERROR_SHORTUTF8, pcre.ERROR_BADUTF8_OFFSET: - raise InvalidUnicodeError(msg : "Invalid unicode byte sequence", - pos : myResult.pcreMatchBounds[0].a) + raise InvalidUnicodeError(msg: "Invalid unicode byte sequence", + pos: myResult.pcreMatchBounds[0].a) else: - raise RegexInternalError(msg : "Unknown internal error: " & $execRet) + raise RegexInternalError(msg: "Unknown internal error: " & $execRet) proc match*(str: string, pattern: Regex, start = 0, endpos = int.high): Option[RegexMatch] = ## Like ` ``find(...)`` <#proc-find>`_, but anchored to the start of the diff --git a/tests/casestmt/t12785.nim b/tests/arc/tcomputedgoto.nim similarity index 87% rename from tests/casestmt/t12785.nim rename to tests/arc/tcomputedgoto.nim index 7177fb9c2..8af17b56e 100644 --- a/tests/casestmt/t12785.nim +++ b/tests/arc/tcomputedgoto.nim @@ -1,15 +1,7 @@ discard """ cmd: '''nim c --newruntime $file''' - output: '''copied -copied -2 -destroyed -destroyed -copied -copied -2 -destroyed -destroyed''' + output: '''2 +2''' """ type diff --git a/tests/arc/topt_cursor.nim b/tests/arc/topt_cursor.nim new file mode 100644 index 000000000..6b923cf76 --- /dev/null +++ b/tests/arc/topt_cursor.nim @@ -0,0 +1,35 @@ +discard """ + output: '''("string here", 80)''' + cmd: '''nim c --gc:arc --expandArc:main --hint:Performance:off $file''' + nimout: '''--expandArc: main + +var + :tmpD + :tmpD_1 + :tmpD_2 +try: + var x = ("hi", 5) + x = if cond: + :tmpD = ("different", 54) + :tmpD else: + :tmpD_1 = ("string here", 80) + :tmpD_1 + echo [ + :tmpD_2 = `$`(x) + :tmpD_2] +finally: + `=destroy`(:tmpD_2) +-- end of expandArc ------------------------''' +""" + +proc main(cond: bool) = + var x = ("hi", 5) # goal: computed as cursor + + x = if cond: + ("different", 54) + else: + ("string here", 80) + + echo x + +main(false) diff --git a/tests/arc/topt_no_cursor.nim b/tests/arc/topt_no_cursor.nim new file mode 100644 index 000000000..3d3262491 --- /dev/null +++ b/tests/arc/topt_no_cursor.nim @@ -0,0 +1,37 @@ +discard """ + output: '''(repo: "", package: "meo", ext: "")''' + cmd: '''nim c --gc:arc --expandArc:newTarget --hint:Performance:off $file''' + nimout: '''--expandArc: newTarget + +var + splat + :tmp + :tmp_1 + :tmp_2 +splat = splitFile(path) +:tmp = splat.dir +wasMoved(splat.dir) +:tmp_1 = splat.name +wasMoved(splat.name) +:tmp_2 = splat.ext +wasMoved(splat.ext) +result = ( + let blitTmp = :tmp + blitTmp, + let blitTmp_1 = :tmp_1 + blitTmp_1, + let blitTmp_2 = :tmp_2 + blitTmp_2) +`=destroy`(splat) +-- end of expandArc ------------------------''' +""" + +import os + +type Target = tuple[repo, package, ext: string] + +proc newTarget*(path: string): Target = + let splat = path.splitFile + result = (repo: splat.dir, package: splat.name, ext: splat.ext) + +echo newTarget("meo") diff --git a/tests/arc/topt_refcursors.nim b/tests/arc/topt_refcursors.nim new file mode 100644 index 000000000..7316e2beb --- /dev/null +++ b/tests/arc/topt_refcursors.nim @@ -0,0 +1,39 @@ +discard """ + output: '''''' + cmd: '''nim c --gc:arc --expandArc:traverse --hint:Performance:off $file''' + nimout: '''--expandArc: traverse + +var it = root +block :tmp: + while ( + not (it == nil)): + echo [it.s] + it = it.ri +var jt = root +block :tmp_1: + while ( + not (jt == nil)): + let ri_1 = jt.ri + echo [jt.s] + jt = ri_1 +-- end of expandArc ------------------------''' +""" + +type + Node = ref object + le, ri: Node + s: string + +proc traverse(root: Node) = + var it = root + while it != nil: + echo it.s + it = it.ri + + var jt = root + while jt != nil: + let ri = jt.ri + echo jt.s + jt = ri + +traverse(nil) diff --git a/tests/arc/topt_wasmoved_destroy_pairs.nim b/tests/arc/topt_wasmoved_destroy_pairs.nim new file mode 100644 index 000000000..4d248492b --- /dev/null +++ b/tests/arc/topt_wasmoved_destroy_pairs.nim @@ -0,0 +1,92 @@ +discard """ + output: '''''' + cmd: '''nim c --gc:arc --expandArc:main --expandArc:tfor --hint:Performance:off $file''' + nimout: '''--expandArc: main + +var + a + b + x +x = f() +if cond: + add(a): + let blitTmp = x + blitTmp +else: + add(b): + let blitTmp_1 = x + blitTmp_1 +`=destroy`(b) +`=destroy`(a) +-- end of expandArc ------------------------ +--expandArc: tfor + +var + a + b + x +try: + x = f() + block :tmp: + var i + var i_1 = 0 + block :tmp_1: + while i_1 < 4: + var :tmpD + i = i_1 + if i == 2: + return + add(a): + wasMoved(:tmpD) + `=`(:tmpD, x) + :tmpD + inc i_1, 1 + if cond: + add(a): + let blitTmp = x + wasMoved(x) + blitTmp + else: + add(b): + let blitTmp_1 = x + wasMoved(x) + blitTmp_1 +finally: + `=destroy`(x) + `=destroy_1`(b) + `=destroy_1`(a) +-- end of expandArc ------------------------''' +""" + +proc f(): seq[int] = + @[1, 2, 3] + +proc main(cond: bool) = + var a, b: seq[seq[int]] + var x = f() + if cond: + a.add x + else: + b.add x + +# all paths move 'x' so no wasMoved(x); destroy(x) pair should be left in the +# AST. + +main(false) + + +proc tfor(cond: bool) = + var a, b: seq[seq[int]] + + var x = f() + + for i in 0 ..< 4: + if i == 2: return + a.add x + + if cond: + a.add x + else: + b.add x + +tfor(false) diff --git a/tests/generics/trtree.nim b/tests/arc/trtree.nim similarity index 98% rename from tests/generics/trtree.nim rename to tests/arc/trtree.nim index b45ac8c83..986268f51 100644 --- a/tests/generics/trtree.nim +++ b/tests/arc/trtree.nim @@ -138,16 +138,16 @@ proc search*[M, D: Dim; RT, LT](t: RTree[M, D, RT, LT]; b: Box[D, RT]): seq[LT] # a R*TREE proc proc chooseSubtree[M, D: Dim; RT, LT](t: RTree[M, D, RT, LT]; b: Box[D, RT]; level: int): H[M, D, RT, LT] = assert level >= 0 - var n = t.root - while n.level > level: - let nn = Node[M, D, RT, LT](n) + var it = t.root + while it.level > level: + let nn = Node[M, D, RT, LT](it) var i0 = 0 # selected index var minLoss = type(b[0].a).high - if n.level == 1: # childreen are leaves -- determine the minimum overlap costs - for i in 0 ..< n.numEntries: + if it.level == 1: # childreen are leaves -- determine the minimum overlap costs + for i in 0 ..< it.numEntries: let nx = union(nn.a[i].b, b) var loss = 0 - for j in 0 ..< n.numEntries: + for j in 0 ..< it.numEntries: if i == j: continue loss += (overlap(nx, nn.a[j].b) - overlap(nn.a[i].b, nn.a[j].b)) # overlap (i, j) == (j, i), so maybe cache that? var rep = loss < minLoss @@ -163,7 +163,7 @@ proc chooseSubtree[M, D: Dim; RT, LT](t: RTree[M, D, RT, LT]; b: Box[D, RT]; lev i0 = i minLoss = loss else: - for i in 0 ..< n.numEntries: + for i in 0 ..< it.numEntries: let loss = enlargement(nn.a[i].b, b) var rep = loss < minLoss if loss == minLoss: @@ -174,8 +174,8 @@ proc chooseSubtree[M, D: Dim; RT, LT](t: RTree[M, D, RT, LT]; b: Box[D, RT]; lev if rep: i0 = i minLoss = loss - n = nn.a[i0].n - return n + it = nn.a[i0].n + return it proc pickSeeds[M, D: Dim; RT, LT](t: RTree[M, D, RT, LT]; n: Node[M, D, RT, LT] | Leaf[M, D, RT, LT]; bx: Box[D, RT]): (int, int) = var i0, j0: int diff --git a/tests/destructor/tdestructor3.nim b/tests/destructor/tdestructor3.nim index b68aedce9..f967bbf95 100644 --- a/tests/destructor/tdestructor3.nim +++ b/tests/destructor/tdestructor3.nim @@ -1,5 +1,6 @@ discard """ - output: '''assign + output: ''' +assign destroy destroy 5 @@ -104,12 +105,12 @@ test() #------------------------------------------------------------ # Issue #12883 -type +type TopObject = object internal: UniquePtr[int] proc deleteTop(p: ptr TopObject) = - if p != nil: + if p != nil: `=destroy`(p[]) # !!! this operation used to leak the integer deallocshared(p) @@ -117,12 +118,12 @@ proc createTop(): ptr TopObject = result = cast[ptr TopObject](allocShared0(sizeof(TopObject))) result.internal = newUniquePtr(1) -proc test2() = +proc test2() = let x = createTop() echo $x.internal deleteTop(x) -echo "---------------" +echo "---------------" echo "app begin" test2() echo "app end" \ No newline at end of file diff --git a/tests/destructor/tmisc_destructors.nim b/tests/destructor/tmisc_destructors.nim index 73c54eab3..082cb0f78 100644 --- a/tests/destructor/tmisc_destructors.nim +++ b/tests/destructor/tmisc_destructors.nim @@ -21,11 +21,13 @@ proc `=sink`(dest: var Foo, src: Foo) = proc `=`(dest: var Foo, src: Foo) = assign_counter.inc +proc createFoo(): Foo = Foo(boo: 0) + proc test(): auto = - var a, b: Foo + var a, b = createFoo() return (a, b, Foo(boo: 5)) -var (a, b, _) = test() +var (ag, bg, _) = test() doAssert assign_counter == 0 doAssert sink_counter == 0 diff --git a/tests/destructor/tuse_result_prevents_sinks.nim b/tests/destructor/tuse_result_prevents_sinks.nim index 37b5af9b2..6eac5c902 100644 --- a/tests/destructor/tuse_result_prevents_sinks.nim +++ b/tests/destructor/tuse_result_prevents_sinks.nim @@ -17,15 +17,20 @@ proc `=sink`(self: var Foo; other: Foo) = proc `=destroy`(self: var Foo) = discard +template preventCursorInference(x) = + let p = unsafeAddr(x) + proc test(): Foo = result = Foo() let temp = result + preventCursorInference temp doAssert temp.i > 0 return result proc testB(): Foo = result = Foo() let temp = result + preventCursorInference temp doAssert temp.i > 0 discard test()