fixes #20993 [backport:1.6] (#21574)

* fixes #20993 [backport:1.6]

* proper line endings for the test file

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)

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()
+
+