Implement AsRef, Borrow for std::cell::Ref, RefMut

[dhardy]

Proposal

Problem statement

Implement AsRef, Borrow for std::cell::Ref, RefMut.

Motivation, use-cases

Why? Because we can. This is what AsRef and Borrow are for? (More generally: any impl of Deref should impl AsRef too?)

Further motivation is that it appears we should be able to do the following.

trait Foo<T: ?Sized> {
    type Ref<'b>: std::borrow::Borrow<T>
    where
        Self: 'b;
        
    fn borrow(&self) -> Self::Ref<'_>;
}

struct MyCell(std::cell::RefCell<String>);
impl Foo<str> for MyCell {
    // error[E0277]: the trait bound `Ref<'b, str>: Borrow<str>` is not satisfied
    type Ref<'b> = std::cell::Ref<'b, str>;
    
    fn borrow(&self) -> Self::Ref<'_> {
        std::cell::Ref::map(self.0.borrow(), String::as_str)
    }
}

However, since std::cell::Ref does not implement Borrow<str>, we need <MyCell as Foo>::Ref to be a wrapper around std::cell::Ref like this:

struct MyStrRef<'b>(std::cell::Ref<'b, str>);
impl<'b> std::borrow::Borrow<str> for MyStrRef<'b> {
    fn borrow(&self) -> &str {
        &self.0
    }
}

impl Foo<str> for MyCell {
    // error[E0277]: the trait bound `Ref<'b, str>: Borrow<str>` is not satisfied
    type Ref<'b> = MyStrRef<'b>;
    
    fn borrow(&self) -> Self::Ref<'_> {
        MyStrRef(std::cell::Ref::map(self.0.borrow(), String::as_str))
    }
}

Here, MyStrRef is boilerplate which should not be necessary.

Solution

rust-lang/rust#101981

[dtolnay]

We discussed this proposal in today's standard library API meeting. Those present were ambivalent about whether adding these impls would really be an improvement or not. Having a better motivation written up would have been helpful, ideally with a link to real-world code showing that not having these impls causes friction, and what the code today is doing instead to work around it.

The current ACP's motivation probably applies equally to quite a few other types — for example MutexGuard came up as a type that implements Deref but not AsRef nor Borrow.

Would it be possible to get a list of all other Deref-implementing standard library types which are equally well motivated for getting a new AsRef and Borrow impl? We'd be interested to get a PR adding them all, and put it through Crater all at once. If that goes smoothly, we're probably fine going forward with this.

AsRef in particular is the most likely to cause widespread breakage. For r of type std::cell::Ref, today the code r.as_ref() first does a Deref then an AsRef. With a new AsRef impl added, the Deref would no longer happen and the type of the expression would change.

It's possible that something like impl<T: ?Sized + AsRef<U>, U: ?Sized> AsRef<U> for Ref<T> ends up being more viable to add than impl<T: ?Sized> AsRef<T> for Ref<T>.

[dhardy]

Having a better motivation written up would have been helpful

I expanded the motivation above (with fixed code).

Note: this motivation applies only to the Borrow impl.

[dhardy]

AsRef in particular is the most likely to cause widespread breakage

We could restrict this proposal to Borrow, which suffices for my motivation.

Would it be possible to get a list of all other Deref-implementing standard library types which are equally well motivated for getting a new AsRef and Borrow impl?

Existing impls are:

• std::ffi::CString impls Deref<Target = CStr>, AsRef<CStr> and Borrow<CStr>
• std::ffi::OsString impls Deref<Target = OsStr>, AsRef<OsStr> and Borrow<OsStr>
• std::path::PathBuf impls Deref<Target = Path>, AsRef<Path> and Borrow<Path>
• std::string::String impls Deref<Target = str>, AsRef<str> and Borrow<str>
• std::borrow::Cow<'_, B> impls Deref<Target = B>, AsRef<B> and Borrow<B>
• std::boxed::Box<T, A> impls Deref<Target = T>, AsRef<T> and Borrow<T>
• std::rc::Rc<T, A> impls Deref<Target = T>, AsRef<T> and Borrow<T>
• std::sync::Arc<T, A> impls Deref<Target = T>, AsRef<T> and Borrow<T>
• std::vec::Vec<T, A> impls Deref<Target = [T]>, AsRef<[T]> and Borrow<[T]>

Possibly missing impls are:

• std::io::IoSlice impls Deref<Target = [u8]> but not AsRef or Borrow
• std::io::IoSliceMut impls Deref<Target = [u8]> but not AsRef or Borrow
• std::ffi::VaList<'a, 'f> impls Deref<Target = VaListImpl<'f>> but not AsRef or Borrow
• std::pin::Pin<Ptr> impls Deref<Target = <Ptr as Deref>::Target> but not AsRef or Borrow
• std::boxed::ThinBox<T> impls Deref<Target = T> but not AsRef or Borrow
• std::cell::Ref<'_, T> impls Deref<Target = T> but not AsRef or Borrow
• std::cell::RefMut<'_, T> impls Deref<Target = T> but not AsRef or Borrow
• std::mem::ManuallyDrop<T> impls Deref<Target = T>, but not AsRef or Borrow
• std::panic::AssertUnwindSafe<T> impls Deref<Target = T>, but not AsRef or Borrow
• std::rc::UniqueRc<T> impls Deref<Target = T>, but not AsRef or Borrow
• std::collections::binary_heap::PeekMut<'a, T, A> impls Deref<Target = T>, but not AsRef or Borrow
• std::cell::LazyCell<T, F> impls Deref<Target = T> but not AsRef or Borrow
• std::sync::LazyLock<T, F> impls Deref<Target = T> but not AsRef or Borrow
• std::sync::MappedRwLockReadGuard<'a, T> impls Deref<Target = T> but not AsRef or Borrow
• std::sync::MappedRwLockWriteGuard<'a, T> impls Deref<Target = T> but not AsRef or Borrow
• std::sync::MutexGuard<'a, T> impls Deref<Target = T> but not AsRef or Borrow
• std::sync::ReentrantLockGuard<'a, T> impls Deref<Target = T> but not AsRef or Borrow
• std::sync::RwLockReadGuard<'a, T> impls Deref<Target = T> but not AsRef or Borrow
• std::sync::RwLockWriteGuard<'a, T> impls Deref<Target = T> but not AsRef or Borrow

I'm not sure about VaList, Pin, LazyCell, LazyLock. Other cases at first glance look safe to add Borrow and AsRef impls, aside from the above point about an implied .deref() possibly changing the resultant type.

I skipped AsMut and BorrowMut, but they should likely follow the above for types implementing DerefMut.

[the8472]

Thank you for the effort put into the summary, that provided context for our discussion in the last libs-API meeting.

We came to the conclusion that it would be possible to implement AsRef and Borrow traits for some of those types, but not consistently so due to varying breakage potential, especially for AsRef. While such breakage is permitted by our stability rules, we avoid it by default unless there's stronger motivation than avoiding some boilerplate.

Additionally the main use of the Borrow trait is for collection lookups (such as btreemap), it's less appropriate for general borrowing due to its requirements around Hash and Eq.

Based on this we decided to reject this proposal.