From 822b02f69a71a007e7a079e10f7c3ad37fb8cba0 Mon Sep 17 00:00:00 2001
From: zerbina <100542850+zerbina@users.noreply.github.com>
Date: Tue, 28 Jan 2025 07:34:10 +0100
Subject: [PATCH] source2il: fix missing seq copies (#111)

## Summary

Fix `SeqTy` values not being copied upon being passed to union and
tuple constructors, resulting in modifications of the constructed value
reflecting on the original sequence (and vice versa).

## Details

The `TupleCons` (tuple constructor) translation and union construction
(in `fitExpr`) emitted a direct use of their operands, ignoring whether
the type is trivially copyable or not.

`use` - which emits a call to the type's copy procedure where needed -
is now used in both cases, fixing the bug. A raw usage of `Expr.expr`
part of `While` translation is - while not causing any issues - also
replaced with a call to `use`.

The tests for sequence copies are expanded to also cover tuple,
union, and seq constructors.
---
 passes/source2il.nim                                | 12 ++++++------
 .../expr/{t17_seq_copy.test => t17_seq_copy_1.test} |  0
 tests/expr/t17_seq_copy_2.test                      |  8 ++++++++
 tests/expr/t17_seq_copy_3.test                      | 13 +++++++++++++
 tests/expr/t17_seq_copy_4.test                      |  8 ++++++++
 5 files changed, 35 insertions(+), 6 deletions(-)
 rename tests/expr/{t17_seq_copy.test => t17_seq_copy_1.test} (100%)
 create mode 100644 tests/expr/t17_seq_copy_2.test
 create mode 100644 tests/expr/t17_seq_copy_3.test
 create mode 100644 tests/expr/t17_seq_copy_4.test

diff --git a/passes/source2il.nim b/passes/source2il.nim
index 7f7cbfd..227d269 100644
--- a/passes/source2il.nim
+++ b/passes/source2il.nim
@@ -654,7 +654,8 @@ proc fitExpr(c; e: sink Expr, target: SemType): Expr =
     if e.typ.kind == tkVoid:
       result = e
     else:
-      result = Expr(stmts: e.stmts, typ: target)
+      result = Expr(stmts: move e.stmts, typ: target)
+      # the destructive move of `e.stmts` is deliberate
       case target.kind
       of tkUnion:
         # construct a union
@@ -670,7 +671,8 @@ proc fitExpr(c; e: sink Expr, target: SemType): Expr =
           newAsgn(newFieldExpr(tmp, 0), newIntVal(idx))
         # emit the value assignment:
         result.stmts.add:
-          newAsgn(newFieldExpr(newFieldExpr(tmp, 1), idx), e.expr)
+          newAsgn(newFieldExpr(newFieldExpr(tmp, 1), idx),
+                  use(c, e, result.stmts))
 
         result.expr = tmp
       of tkError:
@@ -1090,8 +1092,7 @@ proc exprToIL(c; t: InTree, n: NodeIndex, expr, stmts): ExprType =
       c.error("`While` body must be a unit or void expression")
 
     var sub = IrNode(kind: Stmts)
-    sub.add cond.stmts
-    sub.add newIf(newNot(cond.expr), newBreak(1))
+    sub.add newIf(newNot(use(c, cond, sub.children)), newBreak(1))
     sub.add body.stmts
     if body.typ.kind != tkVoid:
       sub.add newDrop(body.expr)
@@ -1123,9 +1124,8 @@ proc exprToIL(c; t: InTree, n: NodeIndex, expr, stmts): ExprType =
           c.error("tuple element cannot be 'void'")
           return errorType() + {}
 
-        stmts.add e.stmts
         # add an assignment for the field:
-        stmts.add newAsgn(newFieldExpr(tmp, i), e.expr)
+        stmts.add newAsgn(newFieldExpr(tmp, i), use(c, e, stmts))
 
       # now that we know the type, correct it:
       c.locals[tmp.id] = SemType(kind: tkTuple, elems: elems)
diff --git a/tests/expr/t17_seq_copy.test b/tests/expr/t17_seq_copy_1.test
similarity index 100%
rename from tests/expr/t17_seq_copy.test
rename to tests/expr/t17_seq_copy_1.test
diff --git a/tests/expr/t17_seq_copy_2.test b/tests/expr/t17_seq_copy_2.test
new file mode 100644
index 0000000..f492311
--- /dev/null
+++ b/tests/expr/t17_seq_copy_2.test
@@ -0,0 +1,8 @@
+discard """
+  output: "([1, 2, 3],): (seq(int),)"
+"""
+(Exprs
+  (Decl x (Seq (IntTy) 1 2 3))
+  (Decl y (TupleCons x))
+  (Asgn (At x 0) 4)
+  y)
diff --git a/tests/expr/t17_seq_copy_3.test b/tests/expr/t17_seq_copy_3.test
new file mode 100644
index 0000000..f8c13a3
--- /dev/null
+++ b/tests/expr/t17_seq_copy_3.test
@@ -0,0 +1,13 @@
+discard """
+  output: "[1, 2, 3]: union(int, seq(int))"
+"""
+(Module
+  (ProcDecl union (UnionTy (IntTy) (SeqTy (IntTy))) (Params)
+    (Return 1))
+  (ProcDecl main (UnionTy (IntTy) (SeqTy (IntTy))) (Params)
+    (Exprs
+      (Decl x (Seq (IntTy) 1 2 3))
+      (Decl y (Call union))
+      (Asgn y x)
+      (Asgn (At x 0) 4)
+      (Return y))))
diff --git a/tests/expr/t17_seq_copy_4.test b/tests/expr/t17_seq_copy_4.test
new file mode 100644
index 0000000..c6f41db
--- /dev/null
+++ b/tests/expr/t17_seq_copy_4.test
@@ -0,0 +1,8 @@
+discard """
+  output: "[[1, 2, 3]]: seq(seq(int))"
+"""
+(Exprs
+  (Decl x (Seq (IntTy) 1 2 3))
+  (Decl y (Seq (SeqTy (IntTy)) x))
+  (Asgn (At x 0) 4)
+  y)