diff --git a/compiler/injectdestructors.nim b/compiler/injectdestructors.nim index 5ddd49621f..96102d54d2 100644 --- a/compiler/injectdestructors.nim +++ b/compiler/injectdestructors.nim @@ -249,7 +249,17 @@ 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; flags: set[MoveOrCopyFlag] = {}): PNode = +proc deepAliases(dest, ri: PNode): bool = + case ri.kind + of nkCallKinds, nkStmtListExpr, nkBracket, nkTupleConstr, nkObjConstr, + nkCast, nkConv, nkObjUpConv, nkObjDownConv: + for r in ri: + if deepAliases(dest, r): return true + return false + else: + return aliases(dest, ri) != no + +proc genSink(c: var Con; s: var Scope; dest, ri: PNode; flags: set[MoveOrCopyFlag] = {}): PNode = if (c.inLoopCond == 0 and (isUnpackedTuple(dest) or IsDecl in flags or (isAnalysableFieldAccess(dest, c.owner) and isFirstWrite(dest, c)))) or isNoInit(dest): @@ -263,7 +273,14 @@ proc genSink(c: var Con; dest, ri: PNode; flags: set[MoveOrCopyFlag] = {}): PNod else: # the default is to use combination of `=destroy(dest)` and # and copyMem(dest, source). This is efficient. - result = newTree(nkStmtList, c.genDestroy(dest), newTree(nkFastAsgn, dest, ri)) + if deepAliases(dest, ri): + # consider: x = x + y, it is wrong to destroy the destination first! + # tmp to support self assignments + let tmp = c.getTemp(s, dest.typ, dest.info) + result = newTree(nkStmtList, newTree(nkFastAsgn, tmp, dest), newTree(nkFastAsgn, dest, ri), + c.genDestroy(tmp)) + else: + result = newTree(nkStmtList, c.genDestroy(dest), newTree(nkFastAsgn, dest, ri)) proc isCriticalLink(dest: PNode): bool {.inline.} = #[ @@ -454,7 +471,7 @@ proc ensureDestruction(arg, orig: PNode; c: var Con; s: var Scope): PNode = # This was already done in the sink parameter handling logic. result = newNodeIT(nkStmtListExpr, arg.info, arg.typ) let tmp = c.getTemp(s, arg.typ, arg.info) - result.add c.genSink(tmp, arg, {IsDecl}) + result.add c.genSink(s, tmp, arg, {IsDecl}) result.add tmp s.final.add c.genDestroy(tmp) else: @@ -1004,7 +1021,7 @@ proc sameLocation*(a, b: PNode): bool = of nkHiddenStdConv, nkHiddenSubConv: sameLocation(a[1], b) else: false -proc genFieldAccessSideEffects(c: var Con; dest, ri: PNode; flags: set[MoveOrCopyFlag] = {}): PNode = +proc genFieldAccessSideEffects(c: var Con; s: var Scope; dest, ri: PNode; flags: set[MoveOrCopyFlag] = {}): PNode = # with side effects var temp = newSym(skLet, getIdent(c.graph.cache, "bracketTmp"), nextSymId c.idgen, c.owner, ri[1].info) temp.typ = ri[1].typ @@ -1021,7 +1038,7 @@ proc genFieldAccessSideEffects(c: var Con; dest, ri: PNode; flags: set[MoveOrCop newAccess.add ri[0] newAccess.add tempAsNode - var snk = c.genSink(dest, newAccess, flags) + var snk = c.genSink(s, dest, newAccess, flags) result = newTree(nkStmtList, v, snk, c.genWasMoved(newAccess)) proc moveOrCopy(dest, ri: PNode; c: var Con; s: var Scope, flags: set[MoveOrCopyFlag] = {}): PNode = @@ -1039,21 +1056,21 @@ proc moveOrCopy(dest, ri: PNode; c: var Con; s: var Scope, flags: set[MoveOrCopy else: case ri.kind of nkCallKinds: - result = c.genSink(dest, p(ri, c, s, consumed), flags) + result = c.genSink(s, dest, p(ri, c, s, consumed), flags) of nkBracketExpr: if isUnpackedTuple(ri[0]): # unpacking of tuple: take over the elements - result = c.genSink(dest, p(ri, c, s, consumed), flags) + result = c.genSink(s, dest, p(ri, c, s, consumed), flags) elif isAnalysableFieldAccess(ri, c.owner) and isLastRead(ri, c, s): if aliases(dest, ri) == no: # Rule 3: `=sink`(x, z); wasMoved(z) if isAtom(ri[1]): - var snk = c.genSink(dest, ri, flags) + var snk = c.genSink(s, dest, ri, flags) result = newTree(nkStmtList, snk, c.genWasMoved(ri)) else: - result = genFieldAccessSideEffects(c, dest, ri, flags) + result = genFieldAccessSideEffects(c, s, dest, ri, flags) else: - result = c.genSink(dest, destructiveMoveVar(ri, c, s), flags) + result = c.genSink(s, dest, destructiveMoveVar(ri, c, s), flags) else: result = c.genCopy(dest, ri, flags) result.add p(ri, c, s, consumed) @@ -1065,25 +1082,25 @@ proc moveOrCopy(dest, ri: PNode; c: var Con; s: var Scope, flags: set[MoveOrCopy result.add p(ri, c, s, consumed) c.finishCopy(result, dest, isFromSink = false) else: - result = c.genSink(dest, p(ri, c, s, consumed), flags) + result = c.genSink(s, dest, p(ri, c, s, consumed), flags) of nkObjConstr, nkTupleConstr, nkClosure, nkCharLit..nkNilLit: - result = c.genSink(dest, p(ri, c, s, consumed), flags) + result = c.genSink(s, dest, p(ri, c, s, consumed), flags) of nkSym: if isSinkParam(ri.sym) and isLastRead(ri, c, s): # Rule 3: `=sink`(x, z); wasMoved(z) - let snk = c.genSink(dest, ri, flags) + let snk = c.genSink(s, dest, ri, flags) result = newTree(nkStmtList, snk, c.genWasMoved(ri)) elif ri.sym.kind != skParam and ri.sym.owner == c.owner and isLastRead(ri, c, s) and canBeMoved(c, dest.typ) and not isCursor(ri): # Rule 3: `=sink`(x, z); wasMoved(z) - let snk = c.genSink(dest, ri, flags) + let snk = c.genSink(s, dest, ri, flags) result = newTree(nkStmtList, snk, c.genWasMoved(ri)) else: result = c.genCopy(dest, ri, flags) result.add p(ri, c, s, consumed) c.finishCopy(result, dest, isFromSink = false) of nkHiddenSubConv, nkHiddenStdConv, nkConv, nkObjDownConv, nkObjUpConv, nkCast: - result = c.genSink(dest, p(ri, c, s, sinkArg), flags) + result = c.genSink(s, dest, p(ri, c, s, sinkArg), flags) of nkStmtListExpr, nkBlockExpr, nkIfExpr, nkCaseStmt, nkTryStmt: template process(child, s): untyped = moveOrCopy(dest, child, c, s, flags) # We know the result will be a stmt so we use that fact to optimize @@ -1094,7 +1111,7 @@ proc moveOrCopy(dest, ri: PNode; c: var Con; s: var Scope, flags: set[MoveOrCopy if isAnalysableFieldAccess(ri, c.owner) and isLastRead(ri, c, s) and canBeMoved(c, dest.typ): # Rule 3: `=sink`(x, z); wasMoved(z) - let snk = c.genSink(dest, ri, flags) + let snk = c.genSink(s, dest, ri, flags) result = newTree(nkStmtList, snk, c.genWasMoved(ri)) else: result = c.genCopy(dest, ri, flags) diff --git a/tests/arc/taliased_reassign.nim b/tests/arc/taliased_reassign.nim new file mode 100644 index 0000000000..5563fae8c4 --- /dev/null +++ b/tests/arc/taliased_reassign.nim @@ -0,0 +1,41 @@ +discard """ + matrix: "--mm:orc" +""" + +# bug #20993 + +type + Dual[int] = object # must be generic (even if fully specified) + p: int +proc D(p: int): Dual[int] = Dual[int](p: p) +proc `+`(x: Dual[int], y: Dual[int]): Dual[int] = D(x.p + y.p) + +type + Tensor[T] = object + buf: seq[T] +proc newTensor*[T](s: int): Tensor[T] = Tensor[T](buf: newSeq[T](s)) +proc `[]`*[T](t: Tensor[T], idx: int): T = t.buf[idx] +proc `[]=`*[T](t: var Tensor[T], idx: int, val: T) = t.buf[idx] = val + +proc `+.`[T](t1, t2: Tensor[T]): Tensor[T] = + let n = t1.buf.len + result = newTensor[T](n) + for i in 0 ..< n: + result[i] = t1[i] + t2[i] + +proc toTensor*[T](a: sink seq[T]): Tensor[T] = + ## This breaks it: Using `T` instead makes it work + type U = typeof(a[0]) + var t: Tensor[U] # Tensor[T] works + t.buf = a + result = t + +proc loss() = + var B = toTensor(@[D(123)]) + let a = toTensor(@[D(-10)]) + B = B +. a + doAssert B[0].p == 113, "I want to be 113, but I am " & $B[0].p + +loss() + +