Well, wrapping_add is already safe, so atleast with repr(C) types you can already get a pointer to member fields in entirely safe code. I don’t see how this proposal is different from that. The only reason we can’t in repr(Rust) types is beacuse layouts are not stable.
It's safe, but that's not the point. This implementation will make LLVM's alias analysis hate you.
2 Likes
I am saying it is already safe, so this proposal doesn’t intrduce anything really new.
I think I’m misunderstanding your problem.
As far as I understand, He's saying that it is safe but you should only use it sparingly it if you like your compiled code fast. Currently it is not used a lot even though it exists but adding an operator with this as its implementation could turn that on its head and thus affect performance.
2 Likes
Ah, ok that makes sense. I don’t know if it will be used a lot, but this does seem like a niche feature, so I wouldn’t expect it to have widespread use.
I agree that this is a bad idea.
When I write unsafe code (e.g., offsetting a rawptr) I don't expect Rust to insert guardrails. Even if this is not frequently used, there is no value to making operations on raw pointers safe.
If you don’t want guard rails then there is poject_inbounds which does not have guard rails and is an will be an inherent method on raw pointers. The safe version is for the Project trait which could be used generically in safe code.
This is how projection for references are implemented in my playground example.
I just don’t understand why you want this, though. Wrapping around the address space is already Really Bad in the absence of UB. It’ll only save you from very uncommon mistakes, and certainly won’t save you from accidentally wrapping around, and then offsetting past the zeroth page.
As I understand it, project((&raw 0i32) as *const (i32, i32), (i32, i32)::*1) wouldn’t immediately be UB (though project_inbounds would). However, as all the modification to the pointer is done without leaving pointers, LLVM should be able to continue to track the allocation that is being pointed to, and note that it’s one-past-the-end of the i32 allocation. (This would be the same for further past.) Any use of this pointer that isn’t reading the address as a usize or offsetting it back to the original position would be UB.
That said, I could see an argument for making project always unsafe and also allowing enum-member projection somehow, with those pointers obviously unsafe to use without knowing the discriminant (and thus data) is correct.
The reason mem::uninitialized was deprecated in favor of mem::MaybeUninit is because the former was impossible to use safely generically. Whatever projection mechanism we come up with should be possible to use generically without risking insta-UB if given normal types to work with.
There’s a difference between adding the inbounds assertion, stripping the inbounds quality, and leaving it there unchanged. We want the third, which will probably require a new Rust intrinsic to communicate properly to LLVM if wrapping_offset isn’t it. (The wrapping part of it is unrelated to why we’re using it: we’re using it because it doesn’t add an inbounds assertion where it doesn’t already exist.)
1 Like
Unfortunately, inbounds asserts both non-wrapping and “remains in the same allocation”, and there is no way in LLVM to get one without the other.
3 Likes
Please forgive me if I’m completely missing the boat on this, or if I’m hijacking the thread, but would it be possible to add an inverse projection to this trait? Basically, I’m hoping that there is a safe alternative to an offsetof macro so that it is easier and safer to implement intrusive collections. Having the ability to have a pointer to a field, and then get the object that the field is embedded within in a compiler-enforced safe method would be a really useful thing to add in.
There is already a nigjtly feature for offset from
https://doc.rust-lang.org/std/primitive.pointer.html#method.offset_from
But this can’t be safe in general because we can’t know if a value is inside another type or if it is stand alone in general.
I agree that for a bare pointer it isn’t possible, but your code got me thinking about whether or not the compiler could generate new pointer types that carry the fact that they are pointers to fields within structs within them. I’m still learning rust, so if the following is syntactically broken, please forgive me.
// Borrowing your proposal for this type.
struct Field<Parent, Type> {
offset: usize
}
struct FieldPtr<Parent, Type> {
ptr: *Type
offset: Field
}
impl FieldPtr {
pub fn parent(&self) -> *mut Parent {
unsafe {
self.ptr.sub(self.offset.offset)
}
}
// Lots of other public functions that make a FieldPtr look more
// like a raw pointer, but in a safe way
}
The basic idea is that since the compiler is generating all the offset values already, it could generate FieldPtr for each element as well. FieldPtr can only point to fields of the appropriate type, so you can never offset into some unsafe region. Or at least that’s my goal; I don’t yet know enough about rust to say that I’ve succeeded…
Well, this is something that could be a library item if this post gets turned into an RFC and accepted.
I agree. For what it’s worth, I like what you’re suggesting; it feels much cleaner and safer than using raw pointers. Adding something that lets you do you the inverse as well just closes the loop, and significantly reduces the need for using raw pointers, at least for field accesses.
Regarding Pin, maybe the original concept of having a type for every member wasn’t so bad afterall. My main critique was that it would be less powerful but that changes with the consideration of Pin.
/// Marker trait for fields over which we may pin project.
///
/// You must only implement this for a field which is never moved.
unsafe trait PinnedField { }
struct Foo {
pub member: usize,
pub gets_moved: usize,
}
// That would be cool.
//
// Problem: `Foo::member` is not from the user's crate. Alternative designs welcome.
unsafe impl PinnedField for Foo::member { }
// Unstable, disallow implementation.
trait Field {
type Parent;
type Field;
unsafe fn project_inbounds(*const Parent) -> *const Self;
fn project(*const Parent) -> *const Self;
}
impl<F, Ptr> Project<F> for Pin<&'_ mut F::Parent>
where F: Field + PinnedField
{
type Output = Pin<&'_ mut F::Field>
fn project(self, field: F) -> Self::Output {
// SAFETY: opt-in by the unsafe impl of `PinnedField`.
unsafe { self.map_unchecked(|x| x.project(field)) }
}
}
Why do you need the pinned field trait? As I understand it, Pin only guarantees that the whole value can’t move and says nothing about the fields.
If you want to pin project you need to unsafely promise you’ll never move the field after a Pin to self exists (i.e. Drop drops the field in place)
2 Likes
Ok, so we could use a wrapper type for Pin projections, lets call it PinField. Something like this, and return PinField for all Pin projections that could be turned into a mutable reference. That way the projection remains safe.
I’m curious, why can’t we move fields? What if the fields are Copy or Unpin?