Tracking issue for <*mut T, *const T>::{as_ref, as_mut_ref}

[alexcrichton]

This is a tracking issue for the unstable ptr_as_ref feature of the standard library. These functions allow unsafely converting raw pointers to optional safe references, returning None if they're null.

I would personally vote for removing these, but there's certainly some usage throughout the standard distribution.

cc @gankro

[Gankra]

People really really really _really_ like using Options for anything that "might" be. null is a standard sentinel value, and this promotes it to an Option (as a no-op, no less!). I'd rather people abuse this than Option<NonZero<*const T>>.

Historically I recall you arguing against it because raw pointers can be invalid in so many other ways. But that seems immaterial here. When you're calling this you're asserting that the only way it can be invalid is if it's null -- which is a really common pattern.

Surely this is something FFI encounters a lot?

[alexcrichton]

I have personally never felt the need to use these functions, I've always just done if ptr.is_null() { ... } else { ... }, converting it to an option and then using something like map isn't something I've done that often.

[ghost]

converting it to an option and then using something like map isn't something I've done that often.

This is actually really useful in concurrent lock free hashtables where the hash points to an atomic pointer. This means you end up iteratoring across a list of atomics [1], which is a nice clean recursive operation with the old Option syntax.

I guess it is just syntax, but here mapping the Option vaule is pretty nice.

[1] Data Structure described: https://www.youtube.com/watch?v=HJ-719EGIts

[pythonesque]

I have personally never felt the need to use these functions, I've always just done if ptr.is_null() { ... } else { ... }, converting it to an option and then using something like map isn't something I've done that often.

I find them quite frequently useful, and it appears that others do as well. To be honest, I'm not sure what the justification for removing it is anyway. If anything we should be adding more sugar to raw pointers, not removing what already exists.

[bholley]

I came across this issue while working on Gecko/Servo layout bindings, which involves heavy FFI usage.

My first implementation looked like this:

fn root_node(&self) -> Option<GeckoLayoutNode<'ld>> {
    unsafe {
        let root = Gecko_RootNode(self.document);
        if root.is_null() {
            None         
        } else {         
            Some(GeckoLayoutNode::from_raw(root))
        }                
    }                    
}

With as_ref, I was able to transform it to:

fn root_node(&self) -> Option<GeckoLayoutNode<'ld>> {
    unsafe {
        Gecko_RootNode(self.document).as_ref().map(|n| GeckoLayoutNode::from_ref(n))
    }                                                                         
}

Which is a heck of a lot nicer.

[alexcrichton]

🔔 This issue is now entering its cycle final comment period to be handled in 1.8 🔔

The libs team is somewhat on the fence about this method, and we figured that the FCP can serve as a good opportunity to foster discussion around these methods!

[bholley]

Most of the discussion in this thread so far is about why they are useful and convenient. Can you elaborate on the rationale for removing them?

[alexcrichton]

I think that I'm probably one of the bigger proponents for their removal, so certainly!

I certainly agree that these methods can be nice to use from time to time, but I have personally never actually found a case where I ended up using them in the long run. I'm a little unsettled by the only invalid pointer is the null pointer (when there's a whole bunch of other invalid values), and to me this kinda sticks out as an initial attempt at "nicer unsafe programming" but doesn't seem quite fleshed out.

I would be more comfortable if we had a comprehensive story about ergonomically working with raw pointers. @gankro's initial stab with raw-rs I think is a great start here. Starting with just as_ref, however, seems a little odd to me.

Now I definitely don't feel super strongly either way on this, however. If others find these methods quite useful, then I'd be fine stabilizing.

[SimonSapin]

I'm a little unsettled by the only invalid pointer is the null pointer (when there's a whole bunch of other invalid values)

I think you’re looking at this differently than the people who use these methods. The point is that in some cases, null is not invalid, it’s used to represent the absence of something. (Some other invariant is used to make sure other invalid pointers don’t occur, for example "this field is private and only ever set to ptr::null() or Box::into_raw(something)". Business as usual when using raw pointers at all.)

Now, you could argue that people should not use null as an expected raw pointer value and should use something like Option<NonZero<*const T>> instead, but NonZero is not stable and it’s unclear if it’s ever going to be.

I would be more comfortable if we had a comprehensive story about ergonomically working with raw pointers. @gankro's initial stab with raw-rs I think is a great start here.

raw-rs looks nice but largely unrelated. I don’t see anything related to null-as-an-expected-value in it.

[ghost]

The main use case is handling return Null pointers. But I understand the concern with other forms of pointer invalidation.

If raw-rs pans out could it be integrated into the Option return value? Or is returning invalid pointers something that stabilizing on could potentially bite the project? (I can't think of a use case for this but I also haven't had coffee yet this morning).

[bholley]

Yeah, as @SimonSapin noted, it's not really an issue of pointer validity - when working over an FFI, you kinda need to assume that returned struct pointers (which are generally opaque) are always valid.

Rather, the issue here is nullability, which is represented with Option<> in Rust and nullptr in C/C++. FFIs need to return nullable values all the time (see the Gecko_GetRootNode example above), and so a more-convenient mapping between nullptr and Option reduces the impedance mismatch between the two.

[briansmith]

Does this require that Rust's alignment always matches C's alignment? A *mut u64 received from C code is only guaranteed to be 4-byte aligned on x86. With this RFC, converting that to a Option<&u64> would mean that Rust cannot assume that &u64 is 8-byte aligned on x86.

Note that the Rust alignment of a type must always be greater or equal to the C required alignment in order for as_ptr and as_mut_ptr to result in pointers that are legal (don't cause undefined behavior) when passed to C code.

So, this issue may result in locking Rust into having exactly the same alignment requirements as the C ABI.

[Gankra]

The duality of * and & is already pervasive.

The platform-specific alignment of primitives is also specified in the nomicon -- https://doc.rust-lang.org/stable/nomicon/repr-rust.html (first paragraph):

Most primitives are generally aligned to their size, although this is platform-specific behavior. In particular, on x86 u64 and f64 may be only aligned to 32 bits.

[mbrubeck]

Why do these methods take &self instead of self? The lifetime of the &self borrow is not used anywhere. (Should it be?)

[WiSaGaN]

This is very useful in FFI as it is a common pattern in C library to use null pointer to represent absence. as_ref/as_mut here produces compact FFI code. Maybe even more readily optimized to a no-op?
But I am not very sure whether this should exist as a "built-in" method for pointer, or a library trait considering the most often usage is in FFI, and whether there's difference in converting to no-op, or any alignment/ABI issue.

[huonw]

Does this require that Rust's alignment always matches C's alignment? A *mut u64 received from C code is only guaranteed to be 4-byte aligned on x86. With this RFC, converting that to a Option<&u64> would mean that Rust cannot assume that &u64 is 8-byte aligned on x86.

This isn't true: it can be incorrect/undefined behaviour to convert an unaligned pointer to a reference. (In any case, I believe u64 is 4 byte aligned on x86, but I'm actually sure.)

[alexcrichton]

The libs team discussed this during triage yesterday, and the conclusion was pretty inconclusive. Some points brought up were:

• The names here, as_ref and as_mut as quite small, and are perhaps easily confused with other methods called as_ref or as_mut. For example when scanning an expression it's difficult to see which as_ref is the relevant one if there's multiple.
• We continue to be worried that this doesn't perform any checks beyond whether the pointer is null, for example for alignment.
• This may be somewhat related to our fallible conversions story, but unfortunately that also doesn't exist today.

Overall we weren't able to reach a conclusion on this, so I'm going to nominate it for discussion to move into FCP again next cycle.

[WiSaGaN]

One minor point is that the already stabilized method is_null() shares the same "nullable" property as as_ref() and as_mut(). as_ref() and as_mut() do no more than simplify the idiom of is_null() and unsafe dereference, which looks to me the major use case of "is_null()".

[alexcrichton]

🔔 This issue is reentering its cycle-long final comment period 🔔

We still haven't quite decided on a great name for these methods just yet, but it's something we can perhaps decide during FCP!

[mbrubeck]

I haven't seen any answer or discussion about my question above:

Why do these methods take &self instead of self?

In contrast, the other standard methods on pointers all take self.

[alexcrichton]

@mbrubeck

Why do these methods take &self instead of self?

I suspect it was the old convention of trying to be "less unsafe" when returning a lifetime. Nowadays we'd just switch this back to fn name<'a>(self) -> Option<&'a T>

[diwic]

If they take self instead of &self, then into_ref would be more suitable than as_ref ?

[archshift]

To what extent does converting a pointer to a reference with these methods affect aliasing requirements on references?

[SimonSapin]

Same as dereferencing raw pointers?

[huonw]

@archshift these sort of unsafe functions don't affect requirements of safe types: it is up to the user of the function to ensure that whatever they're doing with it doesn't break the existing requirements (e.g. when you use as_mut_ref, you should ensure that the resulting &mut T is unaliased).

[alexcrichton]

The libs team discussed this during triage yesterday and the conclusion was to stabilize as-is with a minor tweak to the by-value-self method.

The new signatures will be:

unsafe fn as_ref<'a>(self) -> Option<&'a T>;
unsafe fn as_mut<'a>(self) -> Option<&'a mut T>;

We ended up concluding that the unsafe marker is enough to distinguish this from as_ref on other various types, and these names seemed to be the best as we couldn't come up with better alternatives.

Thanks again for the discussion everyone!

[bholley]

\o/

[shepmaster]

The new signatures will be:

unsafe fn as_ref<'a>(self) -> Option<&'a T>;
unsafe fn as_mut<'a>(self) -> Option<&'a T>;

The second should have a mut somewhere in the types, no?

[alexcrichton]

@shepmaster ah yes, of course!