Skip to content

Commit

Permalink
source2il: fix missing seq copies (#111)
Browse files Browse the repository at this point in the history
## 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.
  • Loading branch information
zerbina authored Jan 28, 2025
1 parent 9883b33 commit 822b02f
Show file tree
Hide file tree
Showing 5 changed files with 35 additions and 6 deletions.
12 changes: 6 additions & 6 deletions passes/source2il.nim
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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:
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down
File renamed without changes.
8 changes: 8 additions & 0 deletions tests/expr/t17_seq_copy_2.test
Original file line number Diff line number Diff line change
@@ -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)
13 changes: 13 additions & 0 deletions tests/expr/t17_seq_copy_3.test
Original file line number Diff line number Diff line change
@@ -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))))
8 changes: 8 additions & 0 deletions tests/expr/t17_seq_copy_4.test
Original file line number Diff line number Diff line change
@@ -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)

0 comments on commit 822b02f

Please sign in to comment.