Skip to content

Regression: bitcast initialized empty packed struct should be zero but is non-zero #19692

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

Closed
mitchellh opened this issue Apr 18, 2024 · 3 comments

Comments

@mitchellh
Copy link
Contributor

Zig Version

0.12.0-dev.3674+a0de07760

Steps to Reproduce and Observed Behavior

const Cell = packed struct(u64) {
    content: packed union {
        a: u21,
        b: u22,
    } = .{ .a = 0 },
    _padding: u42 = 0,
};

test {
    const cell: Cell = .{};
    const cell_int: u64 = @bitCast(cell);
    try @import("std").testing.expectEqual(@as(u64, 0), cell_int);
}

Expected Behavior

I expect the test to pass but it fails. It used to pass with 0.12.0-dev.3405+31791ae15

I can understand why it's failing: I'm setting the smaller field of the packed union so the extra bits are probably undefined? Should I be setting the largest field always?

I'm willing to accept this is my problem and not yours but wanted to raise it in case you believe this is a regression.

@mitchellh mitchellh added the bug Observed behavior contradicts documented or intended behavior label Apr 18, 2024
@mlugg
Copy link
Member

mlugg commented Apr 18, 2024

Yes, I think it's accurate to say that initializing a packed union with a small field leaves the remaining bits undefined, so that code snippet just has a bug. That said, I'll wait for @andrewrk to confirm the intended behavior before closing.

@andrewrk
Copy link
Member

Thanks for the report.

This is indeed working as designed given how unions work in zig: only the active field has a defined value, and the bitcast therefore accesses undefined memory, making cell_int have undefined bits, or be fully undefined, depending on the resolution of #19634.

Based on the recent guidance that I wrote on struct field initialization, I would recommend this instead:

const Cell = packed struct(u64) {
    content: packed union {
        a: u21,
        b: u22,
    },
    _padding: u42,

    const init: Cell = @bitCast(@as(u64, 0));
};
test {
    const cell = Cell.init;
    const cell_int: u64 = @bitCast(cell);
    try @import("std").testing.expectEqual(@as(u64, 0), cell_int);
}

This code snippet should be considered when evaluating #19634, but I don't think it needs to remain open as a bug or a proposal.

@andrewrk andrewrk closed this as not planned Won't fix, can't repro, duplicate, stale Apr 18, 2024
@andrewrk andrewrk removed the bug Observed behavior contradicts documented or intended behavior label Apr 18, 2024
@mitchellh
Copy link
Contributor Author

Thanks, that's exactly what I ended up doing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants