fixes #9263

compiler/destroyer.nim

@@ -282,8 +282,6 @@ template recurse(n, dest) =
 proc isSinkParam(s: PSym): bool {.inline.} =
   result = s.kind == skParam and s.typ.kind == tySink
 
-const constrExprs = nkCallKinds+{nkObjConstr}
-
 proc destructiveMoveSink(n: PNode; c: var Con): PNode =
   # generate:  (chckMove(sinkParam_AliveBit); sinkParam_AliveBit = false; sinkParam)
   result = newNodeIT(nkStmtListExpr, n.info, n.typ)
@@ -299,39 +297,6 @@ proc genMagicCall(n: PNode; c: var Con; magicname: string; m: TMagic): PNode =
   result.add(newSymNode(createMagic(c.graph, magicname, m)))
   result.add n
 
-proc moveOrCopy(dest, ri: PNode; c: var Con): PNode =
-  if ri.kind in constrExprs:
-    result = genSink(c, dest.typ, dest, ri)
-    # watch out and no not transform 'ri' twice if it's a call:
-    let ri2 = copyNode(ri)
-    recurse(ri, ri2)
-    result.add ri2
-  elif ri.kind == nkSym and ri.sym.kind != skParam and isHarmlessVar(ri.sym, c):
-    # Rule 3: `=sink`(x, z); wasMoved(z)
-    var snk = genSink(c, dest.typ, dest, ri)
-    snk.add p(ri, c)
-    result = newTree(nkStmtList, snk, genMagicCall(ri, c, "wasMoved", mWasMoved))
-  elif ri.kind == nkSym and isSinkParam(ri.sym):
-    result = genSink(c, dest.typ, dest, ri)
-    result.add destructiveMoveSink(ri, c)
-  else:
-    result = genCopy(c, dest.typ, dest, ri)
-    result.add p(ri, c)
-
-proc passCopyToSink(n: PNode; c: var Con): PNode =
-  result = newNodeIT(nkStmtListExpr, n.info, n.typ)
-  let tmp = getTemp(c, n.typ, n.info)
-  if hasDestructor(n.typ):
-    var m = genCopy(c, n.typ, tmp, n)
-    m.add p(n, c)
-    result.add m
-    message(c.graph.config, n.info, hintPerformance,
-      ("passing '$1' to a sink parameter introduces an implicit copy; " &
-      "use 'move($1)' to prevent it") % $n)
-  else:
-    result.add newTree(nkAsgn, tmp, p(n, c))
-  result.add tmp
-
 proc genWasMoved(n: PNode; c: var Con): PNode =
   # The mWasMoved builtin does not take the address.
   result = genMagicCall(n, c, "wasMoved", mWasMoved)
@@ -355,6 +320,83 @@ proc destructiveMoveVar(n: PNode; c: var Con): PNode =
   result.add genWasMoved(n, c)
   result.add tempAsNode
 
+proc passCopyToSink(n: PNode; c: var Con): PNode =
+  result = newNodeIT(nkStmtListExpr, n.info, n.typ)
+  let tmp = getTemp(c, n.typ, n.info)
+  if hasDestructor(n.typ):
+    var m = genCopy(c, n.typ, tmp, n)
+    m.add p(n, c)
+    result.add m
+    message(c.graph.config, n.info, hintPerformance,
+      ("passing '$1' to a sink parameter introduces an implicit copy; " &
+      "use 'move($1)' to prevent it") % $n)
+  else:
+    result.add newTree(nkAsgn, tmp, p(n, c))
+  result.add tmp
+
+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!
+      let a = copyNode(arg)
+      recurse(arg, a)
+      result = a
+    elif arg.kind in {nkObjConstr, nkCharLit..nkFloat128Lit}:
+      discard "object construction to sink parameter: nothing to do"
+      result = arg
+    elif arg.kind == nkSym and arg.sym.kind in InterestingSyms and isHarmlessVar(arg.sym, c):
+      # if x is a variable and it its last read we eliminate its
+      # destructor invokation, but don't. We need to reset its memory
+      # to disable its destructor which we have not elided:
+      result = destructiveMoveVar(arg, c)
+    elif arg.kind == nkSym and isSinkParam(arg.sym):
+      # mark the sink parameter as used:
+      result = destructiveMoveSink(arg, c)
+    else:
+      # an object that is not temporary but passed to a 'sink' parameter
+      # results in a copy.
+      result = passCopyToSink(arg, c)
+  else:
+    result = p(arg, c)
+
+proc moveOrCopy(dest, ri: PNode; c: var Con): PNode =
+  case ri.kind
+  of nkCallKinds:
+    result = genSink(c, dest.typ, dest, ri)
+    # watch out and no not transform 'ri' twice if it's a call:
+    let ri2 = copyNode(ri)
+    let parameters = ri[0].typ
+    let L = if parameters != nil: parameters.len else: 0
+    ri2.add ri[0]
+    for i in 1..<ri.len:
+      ri2.add pArg(ri[i], c, i < L and parameters[i].kind == tySink)
+    #recurse(ri, ri2)
+    result.add ri2
+  of nkObjConstr:
+    result = genSink(c, dest.typ, dest, ri)
+    let ri2 = copyTree(ri)
+    for i in 1..<ri.len:
+      # everything that is passed to an object constructor is consumed,
+      # so these all act like 'sink' parameters:
+      ri2[i].sons[1] = pArg(ri[i][1], c, isSink = true)
+    result.add ri2
+  of nkSym:
+    if ri.sym.kind != skParam and isHarmlessVar(ri.sym, c):
+      # Rule 3: `=sink`(x, z); wasMoved(z)
+      var snk = genSink(c, dest.typ, dest, ri)
+      snk.add p(ri, c)
+      result = newTree(nkStmtList, snk, genMagicCall(ri, c, "wasMoved", mWasMoved))
+    elif isSinkParam(ri.sym):
+      result = genSink(c, dest.typ, dest, ri)
+      result.add destructiveMoveSink(ri, c)
+    else:
+      result = genCopy(c, dest.typ, dest, ri)
+      result.add p(ri, c)
+  else:
+    result = genCopy(c, dest.typ, dest, ri)
+    result.add p(ri, c)
+
 proc p(n: PNode; c: var Con): PNode =
   case n.kind
   of nkVarSection, nkLetSection:
@@ -392,31 +434,7 @@ proc p(n: PNode; c: var Con): PNode =
     let parameters = n[0].typ
     let L = if parameters != nil: parameters.len else: 0
     for i in 1 ..< n.len:
-      let arg = n[i]
-      if i < L and parameters[i].kind == tySink:
-        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!
-          let a = copyNode(arg)
-          recurse(arg, a)
-          n.sons[i] = a
-        elif arg.kind in {nkObjConstr, nkCharLit..nkFloat128Lit}:
-          discard "object construction to sink parameter: nothing to do"
-        elif arg.kind == nkSym and isHarmlessVar(arg.sym, c):
-          # if x is a variable and it its last read we eliminate its
-          # destructor invokation, but don't. We need to reset its memory
-          # to disable its destructor which we have not elided:
-          n.sons[i] = destructiveMoveVar(arg, c)
-        elif arg.kind == nkSym and isSinkParam(arg.sym):
-          # mark the sink parameter as used:
-          n.sons[i] = destructiveMoveSink(arg, c)
-        else:
-          # an object that is not temporary but passed to a 'sink' parameter
-          # results in a copy.
-          n.sons[i] = passCopyToSink(arg, c)
-      else:
-        n.sons[i] = p(arg, c)
-
+      n.sons[i] = pArg(n[i], c, i < L and parameters[i].kind == tySink)
     if n.typ != nil and hasDestructor(n.typ):
       discard "produce temp creation"
       result = newNodeIT(nkStmtListExpr, n.info, n.typ)
@@ -442,7 +460,7 @@ proc p(n: PNode; c: var Con): PNode =
     recurse(n, result)
 
 proc injectDestructorCalls*(g: ModuleGraph; owner: PSym; n: PNode): PNode =
-  when defined(nimDebugDestroys):
+  when false: # defined(nimDebugDestroys):
     echo "injecting into ", n
   var c: Con
   c.owner = owner
@@ -477,7 +495,7 @@ proc injectDestructorCalls*(g: ModuleGraph; owner: PSym; n: PNode): PNode =
     result.add body
 
   when defined(nimDebugDestroys):
-    if owner.name.s == "main" or true:
+    if owner.name.s == "test1": # or true:
       echo "------------------------------------"
       echo owner.name.s, " transformed to: "
       echo result

tests/destructor/tmatrix.nim

@@ -0,0 +1,117 @@
+discard """
+  output: '''after 3 3
+after 3 3
+after 2 2'''
+"""
+# bug #9263
+type
+  Matrix* = object
+    # Array for internal storage of elements.
+    data: ptr UncheckedArray[float]
+    # Row and column dimensions.
+    m*, n*: int
+
+var
+  allocCount, deallocCount: int
+
+proc `=destroy`*(m: var Matrix) =
+  if m.data != nil:
+    dealloc(m.data)
+    deallocCount.inc
+    m.data = nil
+    m.m = 0
+    m.n = 0
+
+proc `=sink`*(a: var Matrix; b: Matrix) =
+  if a.data != nil and a.data != b.data:
+    dealloc(a.data)
+    deallocCount.inc
+  a.data = b.data
+  a.m = b.m
+  a.n = b.n
+
+proc `=`*(a: var Matrix; b: Matrix) =
+  if a.data != nil and a.data != b.data:
+    dealloc(a.data)
+    deallocCount.inc
+    a.data = nil
+  a.m = b.m
+  a.n = b.n
+  if b.data != nil:
+    a.data = cast[type(a.data)](alloc(a.m * a.n * sizeof(float)))
+    allocCount.inc
+    copyMem(a.data, b.data, b.m * b.n * sizeof(float))
+
+proc matrix*(m, n: int, s: float): Matrix =
+  ## Construct an m-by-n constant matrix.
+  result.m = m
+  result.n = n
+  result.data = cast[type(result.data)](alloc(m * n * sizeof(float)))
+  allocCount.inc
+  for i in 0 ..< m * n:
+    result.data[i] = s
+
+proc `[]`*(m: Matrix, i, j: int): float {.inline.} =
+  ## Get a single element.
+  m.data[i * m.n + j]
+
+proc `[]`*(m: var Matrix, i, j: int): var float {.inline.} =
+  ## Get a single element.
+  m.data[i * m.n + j]
+
+proc `[]=`*(m: var Matrix, i, j: int, s: float) =
+  ## Set a single element.
+  m.data[i * m.n + j] = s
+
+proc `-`*(m: sink Matrix): Matrix =
+  ## Unary minus
+  result = m
+  for i in 0 ..< m.m:
+    for j in 0 ..< m.n:
+      result[i, j] = -m[i, j]
+
+proc `+`*(a: sink Matrix; b: Matrix): Matrix =
+  ## ``C = A + B``
+  assert(b.m == a.m and b.n == a.n, "Matrix dimensions must agree.")
+  result = a
+  for i in 0 ..< a.m:
+    for j in 0 ..< a.n:
+      result[i, j] = a[i, j] + b[i, j]
+
+proc `-`*(a: sink Matrix; b: Matrix): Matrix =
+  ## ``C = A - B``
+  assert(b.m == a.m and b.n == a.n, "Matrix dimensions must agree.")
+  result = a
+  for i in 0 ..< a.m:
+     for j in 0 ..< a.n:
+        result[i, j] = a[i, j] - b[i, j]
+
+proc info =
+  echo "after ", allocCount, " ", deallocCount
+  allocCount = 0
+  deallocCount = 0
+
+proc test1 =
+  var a = matrix(5, 5, 1.0)
+  var b = a
+  var c = a + b
+
+proc test2 =
+  var a = matrix(5, 5, 1.0)
+  var b = a
+  var c = -a
+
+proc test3 =
+  var a = matrix(5, 5, 1.0)
+  var b = matrix(5, 5, 2.0)
+  #    a = a - b
+  b = -b + a
+
+test1()
+info()
+
+test2()
+info()
+
+test3()
+info()