DerefPure impl for Cow may be too general

[zachs18]

One of unsafe trait DerefPure's preconditions is that Self's Deref impl is "well-behaved". std::borrow::Cow currently always implements DerefPure, but Cow<B>'s Deref impl can call arbitrary user code in <B::Owned as Borrow<B>>::borrow that may not be "well-behaved".

I tried this (safe) code:

#![feature(deref_patterns)]
use std::borrow::{Cow, Borrow};

struct Weird<T: ?Sized = [()]>(bool, T);

struct WeirdOwned {
    val: std::cell::Cell<bool>,
}

impl Borrow<Weird> for WeirdOwned {
    fn borrow(&self) -> &Weird {
        let val = self.val.get();
        self.val.set(!val);
        if val { &Weird(true, []) } else { &Weird(false, []) }
    }
}

impl ToOwned for Weird {
    type Owned = WeirdOwned;
    fn to_owned(&self) -> WeirdOwned {
        WeirdOwned {
            val: false.into(),
        }
    }
}

fn main() {
    let x: Cow<Weird> = Cow::Owned(WeirdOwned { val: true.into() });
    match x {
        deref!(Weird(false, _)) | deref!(Weird(true, _)) => println!("a"),
        _ => println!("b"),
    }
    // prints b
    
    let x: Cow<Weird> = Cow::Owned(WeirdOwned { val: true.into() });
    match x {
        deref!(Weird(false, _) | Weird(true, _)) => println!("a"),
        _ => println!("b"),
    }
    // prints a
}

I expected to see this happen: Either this shouldn't compile (because Cow<Weird>'s Deref impl is not "well-behaved"), or the first match should also print "a".

Instead, this happened: Having two separate deref! patterns joined by an or-pattern behaves differently than an or-pattern in a deref! pattern. IIUC currently there is no soundness issue because deref patterns do not interact with exhaustiveness checking, but if the first match didn't require the blanket _ arm and considered deref!(Weird(false, _)) | deref!(Weird(true, _)) exhaustive, then the exhibited behavior would be a soundness issue.

---

One possible resolution might be to restrict Cow's DerefPure impls to just Cow<T: Sized>, Cow<str>, Cow<[T]>, and other types that the stdlib fully controls the ToOwned/Borrow/Deref impls of.

-unsafe impl<B: ?Sized + ToOwned> DerefPure for Cow<'_, B> where B::Owned: Borrow<B> {}
+unsafe impl<T: Clone> DerefPure for Cow<'_, T>{}
+unsafe impl DerefPure for Cow<'_, str>{}
+unsafe impl<T: Clone> DerefPure for Cow<'_, [T]>{}

incorrect suggestion

Moved this into a hidden block because this would not work with Cow<T: Sized>, since T::Owned = T, which probably doesn't implement Deref<Target = T> + DerefPure.

One possible resolution might be to add "Borrow<Self::Target>/BorrowMut<Self::Target> are well-behaved if implemented" to DerefPure's preconditions, and then change Cow's impl as follows:

-unsafe impl<B: ?Sized + ToOwned> DerefPure for Cow<'_, B> where B::Owned: Borrow<B> {}
+unsafe impl<B: ?Sized + ToOwned> DerefPure for Cow<'_, B> where B::Owned: Borrow<B> + Deref<Target = B> + DerefPure {}

Then, Cow<Weird> would not implement DerefPure, because WeirdOwned does not implement DerefPure. This would still allow Cow<str>, and Cow<[T]> (since String and Vec<T> implement DerefPure), but this would not work with Cow<T: Sized>, since T::Owned = T, which might not implement DerefPure, and probably doesn't implement Deref<Target = T>.

Note that just adding B::Owned: DerefPure is not sufficient, since you could have Weird::Owned = Box<WeirdOwned>, and Box: DerefPure.

Meta

rustc --version --verbose:

rustc 1.86.0-nightly (649b995a9 2025-01-22)
binary: rustc
commit-hash: 649b995a9febd658b2570160703dff6fdc038ab2
commit-date: 2025-01-22
host: x86_64-unknown-linux-gnu
release: 1.86.0-nightly
LLVM version: 19.1.7

@rustbot label +F-deref-patterns +requires-nightly

[jieyouxu]

Backlink: tracking issue #87121

[Nadrieril]

You're absolutely right, I had not noticed that Cow's Deref used borrow under the hood. I'm in favor of implementing it for some known safe types.

[joseph-gio]

Might make sense to add a BorrowPure/TrustedBorrow trait to ensure that the borrow implementation is well-behaved?

[Nadrieril]

The impls suggested in the OP (for T: Clone and for Vec/String) seem sufficient for now, we can always add more later.