Tagged union panic when reading and writing same union in one assignment

Help
1

Hi,

I hit a runtime panic with a tagged union and want to confirm whether this is expected based on Zig’s evaluation rules.

Given:

const State = union(enum) {
    alive: Alive,
    exploding: Exploding,
};

And:

if (bullet.state != .alive) return;

bullet.state = .{
    .exploding = .{
        .origin = bullet.state.alive.body.pos(),
    },
};

This panics at runtime:

panic: access of union field 'alive' while field 'exploding' is active

Logging shows bullet.state is alive at function entry. There is no threading and nothing else mutates bullet.state in between.

However, this works:

const exploding = Exploding{
    .origin = bullet.state.alive.body.pos(),
};
bullet.state = .{ .exploding = exploding };

It seems that Zig does not guarantee evaluation order here, and that during the first form the union tag may be updated as part of the assignment before the nested bullet.state.alive expression is evaluated.

Is that the correct interpretation?

More generally:

  • Does Zig guarantee that the RHS of an assignment is fully evaluated before the LHS is mutated?
  • Is it considered undefined / unspecified to read from x in the same expression where x is assigned a different union tag?
  • Are there any other related gotchas to be aware of?

Thanks.

2

This is expected. See the Result Location section of the language reference. E.g.

When compiling the simple assignment expression x = e, many languages would create the temporary value e on the stack, and then assign it to x, potentially performing a type coercion in the process. Zig approaches this differently. The expression e is given a result type matching the type of x, and a result location of &x. For many syntactic forms of e, this has no practical impact. However, it can have important semantic effects when working with more complex syntax forms.

And the swap example in that section.

It’s a subtle, but important difference to C.

Edit: ok. I’ll defer to vupesx here. Ignore the above.

1 Like
3

.{} writes directly to the destination, but T{} creates a new instance then copies to the destination.

For some reason, the payload is being written before the tag. Seems like a regression/unhandled edge case. Even if the payload is written first, the compiler should probably still allow it since the tag will be updated anyway.

1 Like
4

Thanks. So, should I raise it with ziglang on codeberg?

5

The bug is in your code, but it’s subtle.

Let’s start with your questions:

No. Zig uses a strict LHS-to-RHS evaluation order. Assignments happen as they are evaluated in that order.

It’s not undefined, but you can’t do what you’re trying to do.

On this line:

bullet.state = .{
    .exploding = .{ // <- now .exploding

You’ve assigned the .exploding tag to bullet.state.

Here:

        .origin = bullet.state.alive.body.pos(),

You try to read from .alive, which is no longer the tag, so Zig won’t let you. Ironically, in .ReleaseFast mode this would work: please don’t count on that.

When you use the Exploding{ form, Zig creates an entirely new struct before copying it to the destination. Less efficient (presumably) but it avoids the bad interaction.

Best is probably to cache the value you need into a temp variable, then use .{ form to assign it.

8 Likes
6

oops, missed that they were accessing it while updating it :sweat_smile:

1 Like
8

I misread the first line of the OP code. @mnemnion’s answer sums up the issue perfectly.

1 Like
9

Got it - thank you for the detailed explanation.

1 Like
10

Hm, so this is also not a miscompilation after all?

11

That issue does not at all explain the problem! So I can’t say if it’s a miscompilation when I don’t know what does happen or is expected to happen. Not without checking out and compiling your project, which I don’t want to do.

You can imagine why no one has investigated it after 3 weeks. Not that there aren’t other reasons, such as more important work being done.

I can only guess that the issue is the br branch in the switch is always taken, and that you get a corrupted old value?

If so then, no, it might not be a miscompilation.
The semantics of .{} are not super clear so its possible, but unlikely, that it is a miscompilation and your code is supposed to work.

13

It’s hard to explain when I don’t know what’s wrong :slight_smile: the tests were passing on macos and they were failing on linux. The linked commit fixed the issue - it really seems like it might be left-to-right issue and in that case I’d consider it a serious foot-gun, as it’s easy to miss it in every day code (just like I did)

(reposting again, sorry for the accidental delete, I am on slow internet right now and I’ve accidentally sent click twice to the previous three-dots/delete button position…)

14

Result location semantics is a foot gun, as these two examples indicate.

Couldn’t the compiler detect and forbid assignments where this happens?

15

At this point it’s worth saying that Zig’s semantics are not, at this time, rigorously specified. They’re intended to be, they will be, but they’re not.

Indeed, and there is something… emotionally unsatisfying, about this behavior. The tag is on the LHS side within the .{}, so it ‘feels’ like it’s not an assignment, but it is.

This isn’t really a consequence of RLS, though, but rather LTR. I don’t want Zig to grow a complex evaluation semantics in an attempt to provide DWIM, and I can’t come up with an interpretation of left-to-right in which the tag is not assigned as soon as it’s stated.

It’s a little weird that the Struct{ ... } form and the .{ ... } work differently, but it isn’t that weird, constructors are always a bit hairy and as long as it gets documented someday, seems fine.

There is the fact that, in this specific case, the payload itself is untouched, and that makes it a bit dissatisfying that trying to read it triggers the safety check. But:

some_union, const val = .{ .{ .tag = "str" }, some_union.old_tag + 7};

As weird as that is, it’s legal code, and that one has to trigger the safety check or it will exhibit illegal behavior.

Generalizing this, what the compiler would have to do in order to avoid the bad case but allow the good one, it’s closer to mind-reading than it is to sound engineering.

1 Like
16

That’s my point: Something like that should not be valid code in Zig - it’s ok in languages where the assignment first evaluates the full RHS and only then perform the assignment, but not in Zig.