Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

source2il: fix missing seq copies #111

Merged
merged 1 commit into from
Jan 28, 2025

Conversation

zerbina
Copy link
Collaborator

@zerbina zerbina commented Jan 26, 2025

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.

Summary
=======

Fix `SeqTy` values not being copied upon being passed to union and
tuple constructors, resulting in modification to the constructed value
to reflect on the original sequence.

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.

The tests for sequence copies are expanded to also cover tuple,
union, and seq constructors.
@zerbina zerbina added the bug Something isn't working label Jan 26, 2025
@saem saem merged commit 822b02f into nim-works:main Jan 28, 2025
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants