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

Why does destructuring not use temporaries? #17283

Closed
temhelk opened this issue Sep 25, 2023 · 5 comments
Closed

Why does destructuring not use temporaries? #17283

temhelk opened this issue Sep 25, 2023 · 5 comments
Labels
question No questions on the issue tracker, please.

Comments

@temhelk
Copy link

temhelk commented Sep 25, 2023

Zig Version

0.12.0-dev.575+fb6fff256

Steps to Reproduce and Observed Behavior

This is more of a question about whether this behavior is correct or not.
It is related to the new feature in this pull request: #17156

I'm using this feature in this case to get a swap-like behavior.

const std = @import("std");

pub fn main() void {
    var a: i32, var b: i32 = .{1, 2};
    std.debug.print("{} {}\n", .{ a, b });

    a, b = .{ b, a };
    std.debug.print("{} {}\n", .{ a, b });
}

This chunk of code prints out the following output:

1 2
2 2

It looks to me like the first assignment 'a = b' happens, and then 'b = a' after that, which would give the same output.

But if we assign '.{ b, a }' to a variable before assignment like here:

const std = @import("std");

pub fn main() void {
    var a: i32, var b: i32 = .{1, 2};
    std.debug.print("{} {}\n", .{ a, b });

    const temp = .{ b, a };
    a, b = temp;
    std.debug.print("{} {}\n", .{ a, b });
}

We would get a different output:

1 2
2 1

And that is what I would normally expect.

Now my question is - should we expect a change in behavior if we temporarily save '.{ b, a }' to a variable?
Should these two outputs be different?

Expected Behavior

Possibly(?) we should expect output to be the same like this in two cases:

1 2
2 1
@temhelk temhelk added the bug Observed behavior contradicts documented or intended behavior label Sep 25, 2023
@rohlem
Copy link
Contributor

rohlem commented Sep 25, 2023

Pretty much a duplicate of #12064, although the new syntax certainly makes it even easier to come across.

@Vexu Vexu closed this as not planned Won't fix, can't repro, duplicate, stale Sep 25, 2023
@andrewrk andrewrk added question No questions on the issue tracker, please. and removed bug Observed behavior contradicts documented or intended behavior labels Sep 25, 2023
@andrewrk
Copy link
Member

This is working as designed. Everybody was aware this behavior would be a consequence of destructuring and there is no plan to change it.

@andrewrk andrewrk changed the title Behavior of the new destructuring feature Why does destructuring not use temporaries? Sep 25, 2023
@Ahmed-0-0
Copy link
Contributor

I noticed that it works as expected when destructuring an array

a, b = [_]i32{b, a}

will eventually output
1 2
2 1

@rohlem
Copy link
Contributor

rohlem commented Dec 27, 2023

@0Ahmed-0 I think this is current a remnant of the fact that explicitly specifying the type blocks the result location mechanism, as decided in #17194.
From what I can tell the code still contains the same undefined (illegal?) behavior, meaning the behavior may change at any point.

Explanation of what I think is happening:
Zig introduces a temporary copy early on in its pipeline (done internally to simplify the compiler's implementation).
From Zig's perspective, assignments like this are undefined behavior if you swap values, therefore
it would be allowed to remove the temporary copy as optimization,
meaning it would behave the same as in the original post / when writing .{b, a}.
However, Zig doesn't have any internal optimizations yet, so it never does this.

LLVM (used for optimizations) currently also doesn't optimize it away.
I assume because this is because its optimizations see that doing so would change the behavior,
and it doesn't know that Zig's semantics would allow this optimization (since the code is undefined (illegal?) behavior).

@nektro
Copy link
Contributor

nektro commented Dec 27, 2023

I noticed that it works as expected when destructuring an array

this is great to see but just to clarify where the discrepancy may arise, tuples are structs and so swaps there are assigning to struct fields just as in the issue this is marked as a duplicate of.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question No questions on the issue tracker, please.
Projects
None yet
Development

No branches or pull requests

6 participants