Skip to content

Proposal: @offsetOf instead of @byteOffsetOf #7794

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
hvenev opened this issue Jan 16, 2021 · 5 comments · Fixed by #9073
Closed

Proposal: @offsetOf instead of @byteOffsetOf #7794

hvenev opened this issue Jan 16, 2021 · 5 comments · Fixed by #9073
Labels
accepted This proposal is planned. breaking Implementing this issue could cause existing code to no longer compile or have different behavior. 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.
Milestone

Comments

@hvenev
Copy link

hvenev commented Jan 16, 2021

I believe that the builtin used for getting the byte offset of a struct field should be called @offsetOf rather than @byteOffsetOf. Here are a few reasons why:

  • It is consistent with @sizeOf / @bitSizeOf. @alignOf is also not called @byteAlignOf.
  • offsetOf is common in other programming languages. The C macro is called offsetof. There are also a few Rust libraries that provide macros named offset_of.
  • Bit-packed structs are rare, especially dealing with pointers to individual fields.

I also believe @offsetOf should fail when the struct field is not aligned to a byte boundary. The most obvious reason is to prevent bugs where @byteOffsetOf is used without checking if the offset is actually a whole number of bytes. Having this check as a part of @offsetOf will make subtly incorrect code invalid and will make correct code cleaner.

Using @byteOffsetOf on non-byte-aligned bitfields is rarely useful, and I'd imagine that all such uses will be made more clear by doing @bitOffetOf(T, field) / 8 instead. Even if we keep the current @byteOffsetOf for backwards compatibility, I believe it should be phased out.

A quick grep through the standard library revealed that the only uses of @byteOffsetOf are assertions about the layout of extern structs. This can also be done with the proposed @offsetOf with the added advantage that if we were to make the structs packed, we'd automatically make sure that the fields are byte-aligned.

@hvenev hvenev changed the title inconsistency between @sizeOf/@bitSizeOf and @byteOffsetOf/@bitOffsetOf Naming inconsistency between @sizeOf/@bitSizeOf and @byteOffsetOf/@bitOffsetOf Jan 16, 2021
@faraazahmad
Copy link

I'm not too familiar with Zig yet so forgive my ignorance, but what was the reason @byteOffsetOf and @offsetOf were named so?

@hvenev
Copy link
Author

hvenev commented Jan 18, 2021

@offsetOf does not exist.

@jayschwa jayschwa added breaking Implementing this issue could cause existing code to no longer compile or have different behavior. proposal This issue suggests modifications. If it also has the "accepted" label then it is planned. labels Jan 24, 2021
@Vexu Vexu added this to the 0.8.0 milestone Jan 26, 2021
@hvenev
Copy link
Author

hvenev commented Jan 28, 2021

I've implemented the non-breaking version of this proposal at #7903.

@hvenev hvenev changed the title Naming inconsistency between @sizeOf/@bitSizeOf and @byteOffsetOf/@bitOffsetOf Proposal: @offsetOf instead of @byteOffsetOf Feb 6, 2021
@hvenev
Copy link
Author

hvenev commented Feb 7, 2021

I've updated the proposal.

@andrewrk andrewrk added the accepted This proposal is planned. label Apr 29, 2021
@andrewrk
Copy link
Member

See also #8642

@andrewrk andrewrk added the contributor friendly This issue is limited in scope and/or knowledge of Zig internals. label Apr 29, 2021
@andrewrk andrewrk modified the milestones: 0.8.0, 0.9.0 May 19, 2021
@Vexu Vexu linked a pull request Jun 12, 2021 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accepted This proposal is planned. breaking Implementing this issue could cause existing code to no longer compile or have different behavior. 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.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants