From 83b11580789e237cd61e03bb1db68bf5d98f4514 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Wed, 26 Oct 2022 11:15:14 +0200 Subject: [PATCH 1/2] ptr::eq: clarify that comparing dyn Trait is fragile --- alloc/src/rc.rs | 10 +++++----- alloc/src/sync.rs | 10 +++++----- core/src/ptr/mod.rs | 40 +++++----------------------------------- 3 files changed, 15 insertions(+), 45 deletions(-) diff --git a/alloc/src/rc.rs b/alloc/src/rc.rs index 9c229665c..006d813e5 100644 --- a/alloc/src/rc.rs +++ b/alloc/src/rc.rs @@ -1110,8 +1110,8 @@ impl Rc { #[inline] #[stable(feature = "ptr_eq", since = "1.17.0")] - /// Returns `true` if the two `Rc`s point to the same allocation - /// (in a vein similar to [`ptr::eq`]). + /// Returns `true` if the two `Rc`s point to the same allocation in a vein similar to + /// [`ptr::eq`]. See [that function][`ptr::eq`] for caveats when comparing `dyn Trait` pointers. /// /// # Examples /// @@ -2419,9 +2419,9 @@ impl Weak { } } - /// Returns `true` if the two `Weak`s point to the same allocation (similar to - /// [`ptr::eq`]), or if both don't point to any allocation - /// (because they were created with `Weak::new()`). + /// Returns `true` if the two `Weak`s point to the same allocation similar to [`ptr::eq`], or if + /// both don't point to any allocation (because they were created with `Weak::new()`). See [that + /// function][`ptr::eq`] for caveats when comparing `dyn Trait` pointers. /// /// # Notes /// diff --git a/alloc/src/sync.rs b/alloc/src/sync.rs index e8d9de4fb..81cd77074 100644 --- a/alloc/src/sync.rs +++ b/alloc/src/sync.rs @@ -1117,8 +1117,8 @@ impl Arc { drop(Weak { ptr: self.ptr }); } - /// Returns `true` if the two `Arc`s point to the same allocation - /// (in a vein similar to [`ptr::eq`]). + /// Returns `true` if the two `Arc`s point to the same allocation in a vein similar to + /// [`ptr::eq`]. See [that function][`ptr::eq`] for caveats when comparing `dyn Trait` pointers. /// /// # Examples /// @@ -2069,9 +2069,9 @@ impl Weak { } } - /// Returns `true` if the two `Weak`s point to the same allocation (similar to - /// [`ptr::eq`]), or if both don't point to any allocation - /// (because they were created with `Weak::new()`). + /// Returns `true` if the two `Weak`s point to the same allocation similar to [`ptr::eq`], or if + /// both don't point to any allocation (because they were created with `Weak::new()`). See [that + /// function][`ptr::eq`] for caveats when comparing `dyn Trait` pointers. /// /// # Notes /// diff --git a/core/src/ptr/mod.rs b/core/src/ptr/mod.rs index cfffe351a..a4b89fc62 100644 --- a/core/src/ptr/mod.rs +++ b/core/src/ptr/mod.rs @@ -1733,6 +1733,11 @@ pub(crate) unsafe fn align_offset(p: *const T, a: usize) -> usize { /// by their address rather than comparing the values they point to /// (which is what the `PartialEq for &T` implementation does). /// +/// However, note that comparing trait object pointers (`*const dyn Trait`) is unrealiable: pointers +/// to values of the same underlying type can compare inequal (because vtables are duplicated in +/// multiple codegen units), and pointers to values of *different* underlying type can compare equal +/// (since identical vtables can be deduplicated within a codegen unit). +/// /// # Examples /// /// ``` @@ -1759,41 +1764,6 @@ pub(crate) unsafe fn align_offset(p: *const T, a: usize) -> usize { /// assert!(!std::ptr::eq(&a[..2], &a[..3])); /// assert!(!std::ptr::eq(&a[0..2], &a[1..3])); /// ``` -/// -/// Traits are also compared by their implementation: -/// -/// ``` -/// #[repr(transparent)] -/// struct Wrapper { member: i32 } -/// -/// trait Trait {} -/// impl Trait for Wrapper {} -/// impl Trait for i32 {} -/// -/// let wrapper = Wrapper { member: 10 }; -/// -/// // Pointers have equal addresses. -/// assert!(std::ptr::eq( -/// &wrapper as *const Wrapper as *const u8, -/// &wrapper.member as *const i32 as *const u8 -/// )); -/// -/// // Objects have equal addresses, but `Trait` has different implementations. -/// assert!(!std::ptr::eq( -/// &wrapper as &dyn Trait, -/// &wrapper.member as &dyn Trait, -/// )); -/// assert!(!std::ptr::eq( -/// &wrapper as &dyn Trait as *const dyn Trait, -/// &wrapper.member as &dyn Trait as *const dyn Trait, -/// )); -/// -/// // Converting the reference to a `*const u8` compares by address. -/// assert!(std::ptr::eq( -/// &wrapper as &dyn Trait as *const dyn Trait as *const u8, -/// &wrapper.member as &dyn Trait as *const dyn Trait as *const u8, -/// )); -/// ``` #[stable(feature = "ptr_eq", since = "1.17.0")] #[inline] pub fn eq(a: *const T, b: *const T) -> bool { From 2f0c08f8f41a0b0bc99ab061109fd9712a86afad Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Wed, 26 Oct 2022 14:20:31 +0200 Subject: [PATCH 2/2] explicitly mention that both components of wide prts are compared --- core/src/ptr/mod.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/core/src/ptr/mod.rs b/core/src/ptr/mod.rs index a4b89fc62..3a70981d2 100644 --- a/core/src/ptr/mod.rs +++ b/core/src/ptr/mod.rs @@ -1733,6 +1733,7 @@ pub(crate) unsafe fn align_offset(p: *const T, a: usize) -> usize { /// by their address rather than comparing the values they point to /// (which is what the `PartialEq for &T` implementation does). /// +/// When comparing wide pointers, both the address and the metadata are tested for equality. /// However, note that comparing trait object pointers (`*const dyn Trait`) is unrealiable: pointers /// to values of the same underlying type can compare inequal (because vtables are duplicated in /// multiple codegen units), and pointers to values of *different* underlying type can compare equal