The macros in this crate may call `DerefMut` on uninitialized values.

[eddyb]

If I have a field that implements DerefMut (e.g. a Box), what stops .a.b from going through the (uninitialized) DerefMut?

A quick test suggests this is just allowed:

fn main() {
    let mut x = std::mem::MaybeUninit::<((), Box<(u32, u32)>)>::uninit();
    let p = x.as_mut_ptr();
    let _ = unsafe { std::ptr::addr_of_mut!((*p).1.0); };
}

I suspect this crate would need to either change its API to require a "struct name" to use in a pattern (in the "check if accesses are safe" part of the code, since there's sadly no ref mut pattern equivalent for raw pointers).

Or maybe try to find something that is only possible on direct fields, and DerefMut blocks it (and it can't be something that would be bypassed by a Copy field or if we got more features like DerefMove or DerefGet).
I can't come up with anything. Partial initialization could theoretically work, but it's an error in Rust today.

[youngspe]

That's an excellent point, thank you for identifying that! Looks like these macros are unsound as-is. I'll have to see if there's a way to prevent them from following Deref/DerefMut, or somehow forbid projecting into a struct that implements Deref. If all else fails I'd think the best solution would be to deprecate this library altogether or require the macros be used in an unsafe context, with a warning in the documentation about this possibility.

[youngspe]

I might have a hacky solution: define two traits that both have a method like __assert_type_does_not_impl_deref(&self), implement one for all MaybeUninit<T>, and the other for MaybeUninit<T> where T: Deref, bring them both within scope in the macro, and attempt to call the method. I think the name collision would prevent it from compiling only when T: Deref?

[eddyb]

That's a bit too subtle to rely on, and also keep in mind Index overloading can also cause issues of its own.
Also, I think I just realized that even a &mut T field can cause this (it ofc implements DerefMut, but it's an issue even if it didn't).

I am not aware of a way to reliably do this without taking advantage of pattern-matching, but even then it's tricky because you have to somehow make sure that you are matching on a Foo<...> with a Foo {...} pattern, and not just something that Derefs to Foo.

This is the solution I proposed for a similar problem (offset_of - funnily enough, I believe that can be easily implemented in terms of this crate, entirely in safe code, if/once this crate can be used in const fns): https://internals.rust-lang.org/t/discussion-on-offset-of/7440/2

Now the thing I'm not certain about is whether &mut $Struct { $field: ref foo, .. } = rhs requires rhs: &mut Struct (or if there are silly things one can do with e.g. &mut &mut Struct).
Oh, it doesn't have to: it does MaybeUninit::<$Struct> so it simply requires the full type anyway.

Also, in the case of offset_of, forcing the computation to run at compile-time would also have the advantage that certain forms of misuse would be detected by miri (though perhaps not all). At the very least, miri wouldn't allow UB to stop the assert that the offset is in-bounds of the unitialized variable, which I don't think you can do at runtime (LLVM will happily optimize out the assert).

[eddyb]

Actually, I forgot one thing you can do with a struct name/path (that does not have all of its generic parameters specified), that should allow you to get away with this.

You can generate something like $Struct { $field: mem::uninitialized(), ..mem::uninitialized() } (tho ideally you'd use a helper fn placeholder<T>() -> T { unreachable!() } instead of mem::uninitialized, to make it a bit safer than just it being inside if false).
(EDIT: what I meant to say is that you can use p.write($Struct { ..p.read() }) to guarantee that p is *mut $Struct even if the latter may be invalid with some kind of *mut $Struct<..> syntax we don't have today)

That should guarantee that $field is contained in $Struct and that (*p).$field will not do anything funny. And yes this means you can only support project!(Struct { a, b } => ...) and not nested fields (not without requiring a syntax like Struct { a: Nested { x }, b }).

Another thing about getting the name of the struct so you can use struct literal expressions/patterns is that you can use an exhaustive pattern to require the user to specify all fields (or write ..).
It should also not require a different macro to disallow duplicates.

[RalfJung]

This sound somewhat similar to the problem that memoffset solves with this macro:

https://github.com/Gilnaa/memoffset/blob/01e2e42ef0d833682c27b0f40a2cc748d86b2dc3/src/raw_field.rs#L57

[eddyb]

Ah, great, that's a nice demonstration of what I was talking about, thanks!

The other issue is that memsoffset gets to start with the right type, where here any type name specified by the user might differ from what they're passing in (and coercions could ensue between the two).