Skip to content

std: std.MultiArrayList(TaggedUnion) will store the tag twice in Debug and ReleaseSafe #22785

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

Open
Techatrix opened this issue Feb 6, 2025 · 2 comments

Comments

@Techatrix
Copy link
Contributor

std.MultiArrayList will convert a tagged union into two fields. One for the tag and one for the untagged data. Problem is that the untagged data will also have a safety tag which counteracts the point of storing the tag separately. This will compile in ReleaseFast and ReleaseSmall but not in Debug and ReleaseSafe:

const std = @import("std");

const Tag = enum(u32) { foo, bar };
pub const TaggedUnion = union(Tag) {
    foo: u4,
    bar: u32,
};

comptime {
    const list: std.MultiArrayList(TaggedUnion) = undefined;
    std.debug.assert(@sizeOf(@typeInfo(@TypeOf(list.items(.tags))).pointer.child) == 4);
    std.debug.assert(@sizeOf(@typeInfo(@TypeOf(list.items(.data))).pointer.child) == 4);
}

The untagged union type is being constructed here. I don't think that internally using an extern union and packed union would resolve this since they have stricter requirements (well-defined in-memory layout, C ABI compatibility).

@rohlem
Copy link
Contributor

rohlem commented Feb 6, 2025

It's possible to explicitly remove the safety tag of an untagged union in status-quo by constructing it in a scope with @setRuntimeSafety(false); - see #20232 for previous discussion.
The std.MultiArrayList implementation could use this trick to reduce its memory usage - for the price of allowing misuse.
We could also expose this as a comptime parameter, or make it globally configurable via std.options if we wanted.

@BratishkaErik
Copy link
Contributor

BratishkaErik commented Feb 6, 2025

It's possible to explicitly remove the safety tag of an untagged union in status-quo by constructing it in a scope with @setRuntimeSafety(false); - see #20232 for previous discussion. The std.MultiArrayList implementation could use this trick to reduce its memory usage - for the price of allowing misuse. We could also expose this as a comptime parameter, or make it globally configurable via std.options if we wanted.

Here's test for anyone wondering:

// multi_array_list.zig

pub const Bare = Bare: {
    @setRuntimeSafety(false);
    break :Bare @Type(.{ .@"union" = .{
        .layout = u.layout,
        .tag_type = null,
        .fields = u.fields,
        .decls = &.{},
    } });
};

// main.zig

const std = @import("std");
const MultiArrayList = @import("multi_array_list.zig").MultiArrayList;

const Tag = enum(u32) { foo, bar };
pub const TaggedUnion = union(Tag) {
    foo: u4,
    bar: u32,
};

test {
    const list: MultiArrayList(TaggedUnion) = undefined;
    std.debug.assert(@sizeOf(@typeInfo(@TypeOf(list.items(.tags))).pointer.child) == 4);
    std.debug.assert(@sizeOf(@typeInfo(@TypeOf(list.items(.data))).pointer.child) == 4);
}
$ zig test -ODebug main.zig
All 9 tests passed.

travisstaloch added a commit to travisstaloch/multi-bounded-array that referenced this issue Feb 6, 2025
* add insertAssumeCapacity(), addOne(), addOneAssumeCapacity()
* only store union tags once in Debug and ReleaseSafe as discussed in
  ziglang/zig#22785
* add all tests from std.MultiArrayList
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