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 core::mem::offset_of! RFC #3308

Merged
merged 7 commits into from
Jan 9, 2023
Merged

Conversation

thomcc
Copy link
Member

@thomcc thomcc commented Aug 30, 2022

I started this a jillion years ago, and have been meaning to clean it up and submit it for a long time. The intention is to remove most (ideally all) need for hardcoding offsets, computing offsets manually, or using {memoffset,bytemuck}::offset_of!.

CC @JakobDegen, @saethlin (since they provided some amount of input on this during RustConf)

Rendered

text/0000-offset_of.md Outdated Show resolved Hide resolved
text/0000-offset_of.md Outdated Show resolved Hide resolved
@thomcc
Copy link
Member Author

thomcc commented Aug 30, 2022

Oh right, CC @rust-lang/wg-const-eval since this RFC allow using this macro during const-eval.

@thomcc thomcc added the T-libs-api Relevant to the library API team, which will review and decide on the RFC. label Aug 30, 2022
text/0000-offset_of.md Outdated Show resolved Hide resolved
text/0000-offset_of.md Outdated Show resolved Hide resolved
text/0000-offset_of.md Outdated Show resolved Hide resolved
text/0000-offset_of.md Outdated Show resolved Hide resolved
@BurntSushi
Copy link
Member

BurntSushi commented Aug 30, 2022

I am personally very much in favor of something like this.

I do wonder if the lang team should be roped into this? From looking at bytemuck's implementation, it seems clear to me that this is something that can be implemented in library code. But, invoking it with a type as an "argument" is also something that feels outside of libs-api and more in lang territory. That is, the way this macro is used, it looks like a new language verb that's defined as a macro in order to get around adding a new language verb. (I don't mean that as any sort of accusation of subterfuge!)

And maybe that's all okay. I don't know. I don't think I mind having this as a macro. But that it might be something the lang team wants to evaluate too. (And they might just say, "nah libs-api can deal with this." But they might not.)

@Lokathor
Copy link
Contributor

As a user, I'd prefer if the docs could say "this info isn't some goofy library calculation, this info is directly from the compiler's data structures, guaranteed accurate, guaranteed no UB, guaranteed no runtime cost."

Which is to say, yeah, "language verb" is probably a better framing.

@thomcc thomcc added the T-lang Relevant to the language team, which will review and decide on the RFC. label Aug 30, 2022
@afetisov
Copy link

afetisov commented Aug 30, 2022

One important thing is missing from the reference: the layout of #[repr(rust)] data is unspecified, but is it even true that the layout is fixed? I can totally imagine a compiler optimization that e.g. uses different layout for different local variables of the same type, provided they do not "escape" in some sense (e.g. a type T is private and no explicit pointers to T are created). Without such guarantees any attempts to manipulate individual fields via offsets are meaningless (at least unless the offset is used for accessing the same "instantiation" of T, but that's generally impossible to enforce).

For example, the reference explicitly states that the layout of #[repr(rust)] types can vary between different compilations. This much more tame than the possibility above (vary with each use of type), but may also lead to subtle errors.

I think the RFC should have some motivation for allowing offset_of! on non-#[repr(C)] types. "memoffset does it" isn't sufficient justification, since memoffset has no way to enforce the layout requirements on the supplied types.


Being a macro, offset_of! also can't distinguish between structs and enums, or between fields and arbitrary garbage. I'm afraid this will lead to some very poor post-monomorphization errors. If offset_of! calls are embedded in other macros, the errors can be super cryptic.


One possibility which isn't considered in the RFC is to use special constants instead of an expression macro. Now, defining those constants manually is mentioned, and has many problems, but what if they are defined by a compiler macro? For example, there could be some #[derive(FieldOffsets)] derive macro which would provide the constants with the values substituted by the compiler, e.g.

// before expansion
#[derive(FieldOffsets)
struct Foo {
    first: u32,
    pub second: Vec<Bar>,
    pub(crate) third: (u8, bool, usize),
}

//after expansion
impl Foo {
    const FOO_FIRST_OFFSET: usize = 0;
    pub const FOO_SECOND_OFFSET: usize = 8; // for example
    pub(crate) const FOO_THIRD_OFFSET: usize = 32; // for example
}

For better ergonomics, the constants may be accessible via some expression macro offset_of!, which would expand to the relevant constant name.

  • A derive macro trivially sidesteps the issues of const evaluation and lack of runtime overhead (since the values of constants are inlined by the compiler).
  • It easily deals with unsupported types of data (enums etc), and can more easily support them in the future (since it has full information about the type definition).
  • It automatically mirrors the privacy rules for type and fields.
  • It can transparently handle any other requirements on the type, e.g a specific #[repr] (just check for desired attributes).
  • It could even provide more granular control to the representation of the type: if the author doesn't provide the #[derive(FieldOffsets)], then the consumer code cannot rely on the specific offsets.
  • This information can also be more granularly supported for specific fields via derive macro attributes (like #[offset_of(ignore)] or #[offset_of(pub(crate))] pub field: T.

This implementation is also very hard to provide in a library crate, which provides a stronger argument for including it into the core.

@mikeleany
Copy link

I've always wanted this feature, but I'm wondering what the rational would be for not supporting ?Sized fields from the beginning.

@thomcc
Copy link
Member Author

thomcc commented Aug 30, 2022

@BurntSushi:

I do wonder if the lang team should be roped into this

Yeah, I didn't know what to tag this as. I'm certainly not opposed to roping in t-lang, and added the label.


@Lokathor:

this info isn't some goofy library calculation, this info is directly from the compiler's data structures, guaranteed accurate, guaranteed no UB, guaranteed no runtime cost

The intention is that's how this would be implemented (that's what the bit at the end of the reference is intended to mean), but "no runtime cost" is kind of difficult to define in a way that still works in implementations that don't make as firm of a distinction between compile time and runtime.

Anyway, this felt likely to bog down the implementation under bikeshedding as to what this means, and in practice "evaluates to a constant expression" should be sufficient.

I also don't want to forbid a pure-library implementation, although I believe implementing it as a builtin macro is likely to be better for several reasons (error messages, compile-time performance, reliability, ...).


@mikeleany:

I've always wanted this feature, but I'm wondering what the rational would be for not supporting ?Sized fields from the beginning.

Hm, largely because it prevents a pure library implementation. That said, I don't really care about that, and perhaps with enough cleverness it would still be possible.

@thomcc
Copy link
Member Author

thomcc commented Aug 30, 2022

@afetisov (your comments are long enough to get a separate reply)

I think the RFC should have some motivation for allowing offset_of! on non-#[repr(C)] types. "memoffset does it" isn't sufficient justification, since memoffset has no way to enforce the layout requirements on the supplied types.

The use cases for this are largely the same as the use cases for using offset_of! on repr(C) types -- {de,}serialization, debugging support code, FFI, and data structures -- in all of these it may be undesirable or impractical to force all types to use repr(C).

More generally, it is a goal of this RFC to remove the reasons people have to implement offset_of! manually (either explicitly, or often in an ad-hoc manner). If core::mem::offset_of! lacks functionality that you could implement yourself, then this adds more motivation to not use it.

One important thing is missing from the reference: the layout of #[repr(rust)] data is unspecified, but is it even true that the layout is fixed?

In several ways it is already fixed at compile time (size and alignment are, which limits the size of the overall structure). In other ways, it is fixed at runtime. Allowing use in const evaluation prevents an implementation from doing things like randomizing field offsets at startup, unless it also performs modifications to adjust offset_of! values (and calculations derived from them).

My suspicion is that many implementations which can randomize field offsets at startup can recompute constant values (because they're likely interpreted), but it's plausible that there's some middle-ground.

Either way, this is more or less what I'm getting at by the first item under "Drawbacks" -- previously this information was only available at runtime, and this allows accessing it at compile time.

I can totally imagine a compiler optimization that e.g. uses different layout for different local variables of the same type, provided they do not "escape" in some sense (e.g. a type T is private and no explicit pointers to T are created)

This kind of optimization (and more aggressive ones, like SROA) would still be allowed in exactly the same cases they are currently allowed. If a pointer to an instance of a structure does not escape, the offsets of its field do not matter. This is true regardless of the #[repr] of the type, and is not hindered or changed by offset_of! at all, as that information can already be computed using address arithmetic on pointers to the structure and field.

Being a macro, offset_of! also can't distinguish between structs and enums, or between fields and arbitrary garbage. I'm afraid this will lead to some very poor post-monomorphization errors

Are you sure? From my experience with the code in rustc_builtin_macros and from conversations with compiler contributors (which I had when preparing this RFC), none of this should be a problem.

Perhaps someone familiar with compiler internals cares to weigh in?

One possibility which isn't considered in the RFC is to use special constants instead of an expression macro

I'll add it to the RFC as an alternative, but I am not a fan of macros that expand to items not present in the source (you'll note that none of the existing builtin #[derive()]s do this); I find it results in poor tooling experience, and is generally confusing. Additionally:

<a derive macro> can more easily support them in the future (since it has full information about the type definition).

I don't think there's any reason a derive macro would have more information here than is available to a builtin macro. They're essentially the same thing, and have essentially the same limitations.

This implementation is also very hard to provide in a library crate, which provides a stronger argument for including it into the core.

I don't think this is true -- if core::mem::offset_of! is added, I think it would be quite straightforward to implement a #[derive(FieldOffsets)] equivalent to what you describe as a procedural macro library... What would be difficult about it?


I'll try to update drawbacks/alternatives/motivation with this feedback.

@JakobDegen
Copy link
Contributor

is it even true that the layout is fixed? I can totally imagine a compiler optimization that e.g. uses different layout for different local variables of the same type, provided they do not "escape" in some sense

Yes, the layouts are fixed. However, the as-if rule applies anyway. If the compiler can prove that the user can't tell the difference either way, it's always free to do whatever it wants as long as the behavior of the program is correct.

Being a macro, offset_of! also can't distinguish between structs and enums, or between fields and arbitrary garbage. I'm afraid this will lead to some very poor post-monomorphization errors. If offset_of! calls are embedded in other macros, the errors can be super cryptic.

offset_of! is less a macro and more a compiler built-in. The compiler certainly can distinguish between structs and enums perfectly well, it does this already when it allows you to write things like MyStruct { foo: 5 }. This macro would use much of the same logic. There also means that we would reject things like fn foo<T>() { offset_of!(T, field); } and so there is no possibility of post-mono errors.

@afetisov
Copy link

If a pointer to an instance of a structure does not escape, the offsets of its field do not matter.

The pointer may not be computed explicitly, but perhaps there is still some roundabout way to get it via unsafe code. Trying to bound the behaviour of unsafe code hits Rice's theorem fast, and I can't imagine what would a reasonable definition of UB for offset_of! look like. "It is UB to access fields via this offset without explicitly using *mut T at some point"? That doesn't sound sensible.

Unless the reference explicitly guarantees that a type is always represented in the same way in memory, I would feel very uneasy about exposing offset_of! for unspecified layouts. It's also a semver footgun, but that's already mentioned in RFC.

that information can already be computed using address arithmetic on pointers to the structure and field.

If the layout stability within a single artifact is not guaranteed, then all such calculations are borderline UB (their results can not be used for any unsafe operations in any nontrivial case).

Yes, the layouts are fixed.

Thank you, but that doesn't carry much weight unless it passes an RFC and is specified in the Reference. For all I know, that's just the current implementation restriction.

The use cases for this are largely the same as the use cases for using offset_of! on repr(C) types -- {de,}serialization, debugging support code, FFI, and data structures -- in all of these it may be undesirable or impractical to force all types to use repr(C).

I would like to see specific use cases. I can't imagine how field offsets would help with (de)serialization of types with unspecified layouts, you certainly can't just transmute a slice of incoming data. Nor can I imagine how layout-unstable types could possibly be used in FFI, except as opaque types with no access to inner fields.

What would be difficult about it?

Well, that depends on the implementation and power of offset_of!. For example, it may use something which make the call UB in some cases, or it may be unusable in const contexts at the moment. But I guess being a builtin macro could grant it the necessary special powers. I don't know how complex a builtin macro can be.

@Lokathor
Copy link
Contributor

I would like to just put in the standard reminder that the Reference is a best effort documentation of how the compiler/language currently works, it's not actually an authority. If the reference and the compiler conflict about a point, it's just as likely that the reference will be updated as the compiler will be.

@JakobDegen
Copy link
Contributor

JakobDegen commented Aug 30, 2022

The pointer may not be computed explicitly, but perhaps there is still some roundabout way to get it via unsafe code. Trying to bound the behaviour of unsafe code hits Rice's theorem fast

Fortunately, the language doesn't have to worry about this. People writing compiler optimizations will be responsible for proving that the results of the optimization are not observable.

Thank you, but that doesn't carry much weight unless it passes an RFC and is specified in the Reference.

I don't really know how to respond to this besides saying that this is what it means for a type to have a layout. If the layout is not fixed, then it is not a property of the type, but a property of some other thing

@Lokathor
Copy link
Contributor

Nor can I imagine how layout-unstable types could possibly be used in FFI, except as opaque types with no access to inner fields.

I got one: With the GPU you often have to send structured data and the way you do it is basically sending a bunch of bytes along with a description of what the fields of each field within each struct is. Details vary by API so I don't want to get into it too heavily, but you'd tell the API something like "the 0th field is an [f32;2] at offset 0, the 1th field is an [f32;3] at offset 8" or whatever, and then the GPU would read your buffer that way.

@afetisov
Copy link

I don't really know how to respond to this besides saying that this is what it means for a type to have a layout.

I guess my question can be rephrased as "is it guaranteed that #[repr(rust)] types have a layout?" The reference says that no guarantees on the default layout are provided, and no information besides size and alignment are explicitly exposed. "No guarantees on layout" may well be interpreted as "no specific layout at all".

@Kixiron
Copy link
Member

Kixiron commented Aug 30, 2022

For something to have an unspecified layout it by definition must have a layout

@CryZe
Copy link

CryZe commented Aug 30, 2022

For something to have an unspecified layout it by definition must have a layout

Well of course it has a layout. But the question is if there might be situations in the future where the compiler might want to optimize the layout based on the particular case where the variable is used, just like how a struct might never be on the stack or heap at all and instead be represented entirely in registers. Similar optimizations could be possible where the struct is sorted in a particular way in one function and differently in another. Stabilizing this macro for repr(Rust) would prohibit such optimizations.

@carbotaniuman
Copy link

This macro is already writable with stable Rust, with 2 library implementations of varying QoL. A compiler that wishes to reorder structs (in a way as described above) must already ensure that such reordering does not impact the current library implementations. (This RFC, as written, allows a library-only implementation, even if that might not be the preferred route).

@CryZe
Copy link

CryZe commented Aug 30, 2022

You certainly can write the macro in stable Rust, but you still need unsafe to actually use that offset. So there's still a possibility that this offset is actually unsound to use then on a repr(Rust) type. However it's possibly indeed too late to allow such an optimization as people are probably relying on it enough already to break that.

@Diggsey
Copy link
Contributor

Diggsey commented Aug 30, 2022

You certainly can write the macro in stable Rust, but you still need unsafe to actually use that offset. So there's still a possibility that this offset is actually unsound to use then on a repr(Rust) type. However it's possibly indeed too late to allow such an optimization as people are probably relying on it enough already to break that.

I've had a crate which wraps the concept of a field offset in a safe API which has existed for 6 years 😛

https://docs.rs/field-offset/latest/field_offset/index.html

@rfcbot rfcbot added finished-final-comment-period The final comment period is finished for this RFC. to-announce and removed final-comment-period Will be merged/postponed/closed in ~10 calendar days unless new substational objections are raised. labels Dec 23, 2022
@rfcbot
Copy link
Collaborator

rfcbot commented Dec 23, 2022

The final comment period, with a disposition to merge, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

This will be merged soon.

@WaffleLapkin WaffleLapkin merged commit 3fe3c65 into rust-lang:master Jan 9, 2023
@overlookmotel
Copy link

I have a different, somewhat hacky solution that does not require Default.

@llogiq Would you be able to share your hacky solution?

@llogiq
Copy link
Contributor

llogiq commented Jan 18, 2023

Sure! Look no further. Note that it involves MaybeUninit<T> and the fact that either Option<MaybeUninit<T>> has the same layout as Option<T> or the niche optimization has kicked in, so Option<T> has the same size as T (in which case the offset of the value in the Some variant is obviously zero).

@overlookmotel
Copy link

overlookmotel commented Jan 18, 2023

@llogiq Nice! It's a const fn too. Thank you.

I had assumed that either of these simpler mechanisms would yield the correct result:

  • size_of::<Option<T>>() - size_of::<T>()
  • if size_of::<Option<T>>() == size_of::<T>() { 0 } else { align_of::<T>() }

I imagine this is naive, but I'm not sure what I'm missing that complicates it.

And is there any solution for e.g. Option<str>?

@llogiq
Copy link
Contributor

llogiq commented Jan 19, 2023

This method obviously doesn't work for unsized Options.

@WaffleLapkin
Copy link
Member

WaffleLapkin commented Jan 19, 2023

Enums can't have unsized fields...

@fee1-dead

This comment was marked as resolved.

@llogiq
Copy link
Contributor

llogiq commented Jan 20, 2023

Also if enums are allowed to have unsized types at some point, a whole lot of Option's methods would have to be rewritten.

@llogiq
Copy link
Contributor

llogiq commented Jan 21, 2023

@overlookmotel Btw. the complication is layout randomization. In fact I'm not even sure my hack works in the face of that.

bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 22, 2023
Add offset_of! macro (RFC 3308)

Implements rust-lang/rfcs#3308 (tracking issue rust-lang#106655) by adding the built in macro `core::mem::offset_of`. Two of the future possibilities are also implemented:

* Nested field accesses (without array indexing)
* DST support (for `Sized` fields)

I wrote this a few months ago, before the RFC merged. Now that it's merged, I decided to rebase and finish it.

cc `@thomcc` (RFC author)
RalfJung pushed a commit to RalfJung/miri that referenced this pull request Apr 22, 2023
Add offset_of! macro (RFC 3308)

Implements rust-lang/rfcs#3308 (tracking issue #106655) by adding the built in macro `core::mem::offset_of`. Two of the future possibilities are also implemented:

* Nested field accesses (without array indexing)
* DST support (for `Sized` fields)

I wrote this a few months ago, before the RFC merged. Now that it's merged, I decided to rebase and finish it.

cc `@thomcc` (RFC author)
flip1995 pushed a commit to flip1995/rust-clippy that referenced this pull request Apr 23, 2023
Add offset_of! macro (RFC 3308)

Implements rust-lang/rfcs#3308 (tracking issue #106655) by adding the built in macro `core::mem::offset_of`. Two of the future possibilities are also implemented:

* Nested field accesses (without array indexing)
* DST support (for `Sized` fields)

I wrote this a few months ago, before the RFC merged. Now that it's merged, I decided to rebase and finish it.

cc `@thomcc` (RFC author)
bjorn3 pushed a commit to rust-lang/rustc_codegen_cranelift that referenced this pull request Apr 25, 2023
Add offset_of! macro (RFC 3308)

Implements rust-lang/rfcs#3308 (tracking issue #106655) by adding the built in macro `core::mem::offset_of`. Two of the future possibilities are also implemented:

* Nested field accesses (without array indexing)
* DST support (for `Sized` fields)

I wrote this a few months ago, before the RFC merged. Now that it's merged, I decided to rebase and finish it.

cc `@thomcc` (RFC author)
thomcc pushed a commit to tcdi/postgrestd that referenced this pull request Jul 18, 2023
Add offset_of! macro (RFC 3308)

Implements rust-lang/rfcs#3308 (tracking issue #106655) by adding the built in macro `core::mem::offset_of`. Two of the future possibilities are also implemented:

* Nested field accesses (without array indexing)
* DST support (for `Sized` fields)

I wrote this a few months ago, before the RFC merged. Now that it's merged, I decided to rebase and finish it.

cc `@thomcc` (RFC author)
RalfJung pushed a commit to RalfJung/rust-analyzer that referenced this pull request Apr 20, 2024
Add offset_of! macro (RFC 3308)

Implements rust-lang/rfcs#3308 (tracking issue #106655) by adding the built in macro `core::mem::offset_of`. Two of the future possibilities are also implemented:

* Nested field accesses (without array indexing)
* DST support (for `Sized` fields)

I wrote this a few months ago, before the RFC merged. Now that it's merged, I decided to rebase and finish it.

cc `@thomcc` (RFC author)
RalfJung pushed a commit to RalfJung/rust-analyzer that referenced this pull request Apr 27, 2024
Add offset_of! macro (RFC 3308)

Implements rust-lang/rfcs#3308 (tracking issue #106655) by adding the built in macro `core::mem::offset_of`. Two of the future possibilities are also implemented:

* Nested field accesses (without array indexing)
* DST support (for `Sized` fields)

I wrote this a few months ago, before the RFC merged. Now that it's merged, I decided to rebase and finish it.

cc `@thomcc` (RFC author)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
disposition-merge This RFC is in PFCP or FCP with a disposition to merge it. finished-final-comment-period The final comment period is finished for this RFC. T-libs-api Relevant to the library API team, which will review and decide on the RFC. to-announce
Projects
None yet
Development

Successfully merging this pull request may close these issues.