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

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

Open
andrewrk opened this issue Feb 9, 2024 · 8 comments
Labels
proposal This issue suggests modifications. If it also has the "accepted" label then it is planned.
Milestone

Comments

@andrewrk
Copy link
Member

andrewrk commented Feb 9, 2024

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.

@andrewrk andrewrk added contributor friendly This issue is limited in scope and/or knowledge of Zig internals. proposal This issue suggests modifications. If it also has the "accepted" label then it is planned. accepted This proposal is planned. labels Feb 9, 2024
@andrewrk andrewrk added this to the 0.13.0 milestone Feb 9, 2024
@mlugg
Copy link
Member

mlugg commented Feb 10, 2024

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

@andrewrk
Copy link
Member Author

andrewrk commented Feb 10, 2024

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
Copy link
Member

ifreund commented Feb 10, 2024

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.

@andrewrk andrewrk removed accepted This proposal is planned. contributor friendly This issue is limited in scope and/or knowledge of Zig internals. labels Feb 10, 2024
@rohlem
Copy link
Contributor

rohlem commented Feb 11, 2024

@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
Copy link

exxjob commented Mar 12, 2024

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
Copy link

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
Copy link

kibels commented Jul 21, 2024

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
Copy link

Lking03x commented Aug 4, 2024

@ifreund
I suggest the followings:

  • @bitCast should be the default way for converting bit-to-bit. It makes clear that this is bit manipulation, it's not about integers but about the representation in memory as bits/bytes... The "backing integer" and it's type are just for code conveniences.
  • @intFromEnum/@enumFromInt should be the exception as enums are much more related to their integer value and types.
  • For converting from packed x to an integer representation (or the inverse) the programmer should take extra care as he/she knows what is being achieved with such manipulation. Zig's comptime is powerful enough to handle that and in a abstract/generic way if necessary. For common patterns there could be utility functions as part of the standard library.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
proposal This issue suggests modifications. If it also has the "accepted" label then it is planned.
Projects
None yet
Development

No branches or pull requests

8 participants