ziglang / zig

General-purpose programming language and toolchain for maintaining robust, optimal, and reusable software.
https://ziglang.org
MIT License
33.68k stars 2.47k forks source link

add `@intFromStruct` and `@structFromInt` for converting between a packed struct and its integer representation #18882

Open andrewrk opened 7 months ago

andrewrk commented 7 months ago
const std = @import("std");
const assert = std.debug.assert;
const expect = std.testing.expect;

const S = packed struct {
    a: bool,
    b: u2,
};

test "@intFromStruct and @structFromInt" {
    var s: S = .{
        .a = true,
        .b = 2,
    };
    s.b += 1;
    const i = @intFromStruct(s);
    comptime assert(@TypeOf(i) == u3);
    try expect(i == 0b111);
    const s2: S = @structFromInt(i - 1);
    try expect(s2.a == false);
    try expect(s2.b == 3);
}

@bitCast already works to do this conversion, however it requires sometimes duplicating the destination integer type, which can lead to bugs if the packed struct integer type changes, or if the packed struct changes to an integer or enum or other @bitCast eligible type.

The exact same reasoning is why we have @enumFromInt and @intFromEnum for enums.

Generally, the more specific conversion is preferred, because it is more resilient to code churn.

mlugg commented 7 months ago

Is there any reason to limit this to structs as opposed to packed unions? And in that case, how about @intFromPacked/@packedFromInt?

andrewrk commented 7 months ago

The same question could be asked about @intFromEnum / @enumFromInt. All 3 are effectively "convert to/from the underlying integer". But it seems wrong to change the builtins corresponding to enums. And it's nice to have the symmetry regarding the keyword that declares the type.

Also, reminder that packed struct is a problematic syntax in that it has a conflicting legacy meaning with C attribute packed. We need a different syntax for packed structs.

ifreund commented 7 months ago

It seems to me that the motivating logic here only applies to one direction, namely casting from packed struct/enum/whatever to an integer. I see no advantage over bitcast for casting an integer to a packed struct/enum/whatever as the destination type is still required.

In light of this realization I would counter-propose a single @backingInt() builtin or something with a similar name that accepts packed structs, enums, and anything else with a well-defined backing integer, returning the integer value with the type determined by the packed struct/enum type passed it.

For the other direction I propose that @bitCast() be used. I also propose that the current @intFromEnum()/@enumFromInt() be removed.

rohlem commented 7 months ago

@ifreund Having separate builtins for @enumFromInt/@packedStructFromInt/@packedUnionFromInt adds a bit of readability (what the code intends to do), as well as friction (compile error when the type class changes, f.e. during refactoring, which might be in error), which gives an opportunity to verify the code is still semantically correct.

To me that seems like the fitting null hypothesis for Zig to start out with. Depending on whether it's flexible enough we can decide whether to keep allowing @bitCast in these places, or even re-merge them into fewer builtins later on (after we've tried them out).

exxjob commented 6 months ago

If you compare and contrast this, given @ifreund's solution, with #10710, the goal of both is to reduce convolution. The arguement for @backingInt sounds good to me. @rohlem:

  1. Might that exceed how much poor refactor jobs should be played into this?
  2. If compilation bureaucracy is insisted upon, should it be relevant, you could still directly leverage Zig for this anyhow.
  3. I'm not sure the alleged friction is more ergonomic or comprehensible than @backingInt, possibly overcompensating. Maybe refactor-challenging sequences as described, would be consequence of discouraged patterns to begin with.
mnemnion commented 3 months ago

Regarding @ifreund's proposal, I agree with @backingInt as sufficient for all the cases where the underlying type has a well-defined integer structure. Having a builtin for each direction of every integer-backed type seems excessive. Just @bitCast where the result location defines the type, and @backingInt where the type defines the result location, seems good and sufficient.

But not with removing @enumFromInt or @intFromEnum. Both of these have distinct properties: @enumFromInt is safety-checked if the integer isn't represented in that enum, and @intFromEnum works on the enum tag of a tagged union. The ergonomics of working with tagged unions is a rather nice feature of Zig which I'm making heavy use of, and having to use @backingInt(@as(MyEnum, tagged_union)) is pretty ponderous.

kibels commented 1 month ago

I propose adding a function to std.mem for this, along the lines of asBytes

fn BackingIntPtr(comptime T: type) type {
    const BackingT = @typeInfo(std.meta.Child(T)).Struct.backing_integer orelse @compileError("Not a pointer to packed struct");
    return CopyPtrAttrs(T, .One, BackingT);
}

pub fn asBackingInteger(packed_struct: anytype) BackingIntPtr(@TypeOf(packed_struct)) {
    return @ptrCast(packed_struct);
}

The use case as I see it is to be able to operate on the memory of the packed struct as a single value, like for setting whole registers(e.g. MicroZig mmio) or clearing/masking the value, and that would cover it without introducing unnecessary builtins.

Lking03x commented 1 month ago

@ifreund I suggest the followings: