from_ref, from_mut: clarify documentation

[RalfJung]

This was brought up here. The domain of quantification is generally always constrained by the type in the type signature, and I am not sure it's always worth spelling that out explicitly as that makes things exceedingly verbose. But since this was explicitly brought up, let's clarify.

[rustbot]

r? @Amanieu

rustbot has assigned @Amanieu.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

[briansmith]

In addition, I think it is worth also clarifying that for r: &mut T, from_ref(r) is equivalent to from_ref(r as &T) which is not guaranteed to be equivalent to r as *const and is not necessarily safer than r as const T, in particular if the resulting pointer will ever be cast to a *mut _ pointer. Maybe there is similar nuance for r: Deref<T>.

However, https://doc.rust-lang.org/reference/type-coercions.html#coercion-types does seem to say that when r: &mut T, r as *const T is equivalent to (r as &T) as *const T since there is no coercion rule for &mut T to *const T. So first we should decide whether we can really make the potentially-backward-incompatible change to add a &mut T to *const T coercion rule, because if we decide we can't, then this new wording isn't necessary.

[briansmith]

for r: &mut T, r as *const T is equivalent to (r as &T) as *const T since there is no coercion rule for &mut T to *const T.

Actually, r as *const T could also be (r as *mut T) as *const T, so I guess some clarification is needed.

[RalfJung]

I don't think this is the right place to discuss the semantics of coercions. They behave the same everywhere, surely we don't want to explain them in each function that takes a reference. (e.g. slice.as_ptr() has exactly the same questions).

I also don't see why this PR should be blocked on #56604. That issue is long-standing so I don't expect it to be resolved soon, this clarification however is necessary short-term according to yourself as you said it is ambiguous.

then this new wording isn't necessary.

IMO the new wording is not necessary under any ruleset, since r is declared &T and so that's the only case the docs care about. As I already pointed out, for r of arbitrary type there are counterexamples to the equivalence. So the alternative interpretation would be something like "for all r where both expressions type-check, from_ref(r) is equivalent to r as *const T", which is quite far from what the text says and it needs to conjure a fresh T out of nothing -- IMO clear enough signals that this is not the intended interpretation.

[briansmith]

It's analogous to the situation for addr_of!, which has a similar caveat:

Note that Deref/Index coercions (and their mutable counterparts) are applied inside addr_of_mut! like everywhere else, in which case a reference is created to call Deref::deref or Index::index, respectively. The statements above only apply when no such coercions are applied.

In both cases, the construct is designed to make things safer, but not understanding the interaction with coercions is especially dangerous.

[briansmith]

I suggest adding the following text, which really gets to the point:

For r: &mut T, use from_mut(r).cast_const(), instead of from_ref(r), to get a *const T that can later safely be cast to *mut T.

[RalfJung]

I don't agree with that analogy. Addr_of is a macro so one cannot tell what happens inside, but this here is just a regular function.

[briansmith]

Another issue was brought up in rust-lang/rust-clippy#12260 (comment) by @Jarcho:

This should be merged upstream as well. Changing let x = &temp as *const _ into let x = from_ref(&temp) is UB.
changelog: ref_as_ptr: Take lifetime extension into account

So from_ref(&x) is actually not equivalent to &x as *const T, in general.

[briansmith]

See also rust-lang/rust-clippy#12882, where clippy is recommending from_ref(r) for r: &mut T.

[RalfJung]

Another issue was brought up in rust-lang/rust-clippy#12260 (comment) by @Jarcho:

This should be merged upstream as well. Changing let x = &temp as *const _ into let x = from_ref(&temp) is UB.
changelog: ref_as_ptr: Take lifetime extension into account

So from_ref(&x) is actually not equivalent to &x as *const T, in general.

Ah... yeah that's an unfortunate interaction with the scoping rules caused by from_ref being a function; as is subject to tail expression rules but the arguments of functions are not.

[briansmith]

I suggest replacing "pointing to garbage" with "dangling". I also suggest adding here a concrete example from the clippy experience:

For example, as_ref(r) is not equivalent to r as *const T when lifetime extension is in effect:

let p = &foo() as *const T; // p is valid because of lifetime extension

to

let p = ptr::from_ref(&foo()); // Dangerous: p is dangling.

.

[briansmith]

Same changes here: avoid "garbage" and also add an example of what to avoid.

Also, the structure of this comment should be made symmetric with the one for from_ref; i.e. move "The caller must ..." below the next paragraph.

[briansmith]

We shouldn't be leading off with this incorrect statement. I suggest we replcae "is equivalent" is "is often but not always equivalent".

[briansmith]

Same here: scale back the "is equivalent to" claim.

[RalfJung]

r? libs-api

[RalfJung]

@rust-lang/libs-api any chance someone could look at this? :)

r? libs-api

[Amanieu]

@bors r+ rollup

[bors]

📌 Commit 05b7f28 has been approved by Amanieu

It is now in the queue for this repository.