Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make {Arc,Rc,Weak}::ptr_eq ignore pointer metadata #106450

Merged
merged 1 commit into from
Jun 21, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions library/alloc/src/rc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1169,7 +1169,7 @@ impl<T: ?Sized> Rc<T> {
#[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`]. See [that function][`ptr::eq`] for caveats when comparing `dyn Trait` pointers.
/// [`ptr::eq`]. This function ignores the metadata of `dyn Trait` pointers.
///
/// # Examples
///
Expand All @@ -1184,7 +1184,7 @@ impl<T: ?Sized> Rc<T> {
/// assert!(!Rc::ptr_eq(&five, &other_five));
/// ```
pub fn ptr_eq(this: &Self, other: &Self) -> bool {
this.ptr.as_ptr() == other.ptr.as_ptr()
this.ptr.as_ptr() as *const () == other.ptr.as_ptr() as *const ()
}
}

Expand Down Expand Up @@ -2468,8 +2468,8 @@ impl<T: ?Sized> Weak<T> {
}

/// 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.
/// both don't point to any allocation (because they were created with `Weak::new()`). However,
/// this function ignores the metadata of `dyn Trait` pointers.
///
/// # Notes
///
Expand Down Expand Up @@ -2510,7 +2510,7 @@ impl<T: ?Sized> Weak<T> {
#[must_use]
#[stable(feature = "weak_ptr_eq", since = "1.39.0")]
pub fn ptr_eq(&self, other: &Self) -> bool {
self.ptr.as_ptr() == other.ptr.as_ptr()
ptr::eq(self.ptr.as_ptr() as *const (), other.ptr.as_ptr() as *const ())
}
}

Expand Down
10 changes: 5 additions & 5 deletions library/alloc/src/sync.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1263,7 +1263,7 @@ impl<T: ?Sized> Arc<T> {
}

/// 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.
/// [`ptr::eq`]. This function ignores the metadata of `dyn Trait` pointers.
///
/// # Examples
///
Expand All @@ -1283,7 +1283,7 @@ impl<T: ?Sized> Arc<T> {
#[must_use]
#[stable(feature = "ptr_eq", since = "1.17.0")]
pub fn ptr_eq(this: &Self, other: &Self) -> bool {
this.ptr.as_ptr() == other.ptr.as_ptr()
this.ptr.as_ptr() as *const () == other.ptr.as_ptr() as *const ()
}
}

Expand Down Expand Up @@ -2243,8 +2243,8 @@ impl<T: ?Sized> Weak<T> {
}

/// 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.
/// both don't point to any allocation (because they were created with `Weak::new()`). However,
/// this function ignores the metadata of `dyn Trait` pointers.
///
/// # Notes
///
Expand Down Expand Up @@ -2287,7 +2287,7 @@ impl<T: ?Sized> Weak<T> {
#[must_use]
#[stable(feature = "weak_ptr_eq", since = "1.39.0")]
pub fn ptr_eq(&self, other: &Self) -> bool {
self.ptr.as_ptr() == other.ptr.as_ptr()
ptr::eq(self.ptr.as_ptr() as *const (), other.ptr.as_ptr() as *const ())
}
}

Expand Down
4 changes: 1 addition & 3 deletions src/tools/clippy/clippy_lints/src/unnamed_address.rs
Original file line number Diff line number Diff line change
Expand Up @@ -96,9 +96,7 @@ impl LateLintPass<'_> for UnnamedAddress {
if let ExprKind::Call(func, [ref _left, ref _right]) = expr.kind;
if let ExprKind::Path(ref func_qpath) = func.kind;
if let Some(def_id) = cx.qpath_res(func_qpath, func.hir_id).opt_def_id();
if match_def_path(cx, def_id, &paths::PTR_EQ) ||
match_def_path(cx, def_id, &paths::RC_PTR_EQ) ||
match_def_path(cx, def_id, &paths::ARC_PTR_EQ);
albertlarsan68 marked this conversation as resolved.
Show resolved Hide resolved
if match_def_path(cx, def_id, &paths::PTR_EQ);
let ty_param = cx.typeck_results().node_substs(func.hir_id).type_at(0);
if ty_param.is_trait();
then {
Expand Down
2 changes: 0 additions & 2 deletions src/tools/clippy/clippy_utils/src/paths.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ pub const APPLICABILITY_VALUES: [[&str; 3]; 4] = [
];
#[cfg(feature = "internal")]
pub const DIAGNOSTIC_BUILDER: [&str; 3] = ["rustc_errors", "diagnostic_builder", "DiagnosticBuilder"];
pub const ARC_PTR_EQ: [&str; 4] = ["alloc", "sync", "Arc", "ptr_eq"];
pub const BTREEMAP_CONTAINS_KEY: [&str; 6] = ["alloc", "collections", "btree", "map", "BTreeMap", "contains_key"];
pub const BTREEMAP_INSERT: [&str; 6] = ["alloc", "collections", "btree", "map", "BTreeMap", "insert"];
pub const BTREESET_ITER: [&str; 6] = ["alloc", "collections", "btree", "set", "BTreeSet", "iter"];
Expand Down Expand Up @@ -93,7 +92,6 @@ pub const PTR_WRITE_UNALIGNED: [&str; 3] = ["core", "ptr", "write_unaligned"];
pub const PTR_WRITE_VOLATILE: [&str; 3] = ["core", "ptr", "write_volatile"];
pub const PUSH_STR: [&str; 4] = ["alloc", "string", "String", "push_str"];
pub const RANGE_ARGUMENT_TRAIT: [&str; 3] = ["core", "ops", "RangeBounds"];
pub const RC_PTR_EQ: [&str; 4] = ["alloc", "rc", "Rc", "ptr_eq"];
pub const REFCELL_REF: [&str; 3] = ["core", "cell", "Ref"];
pub const REFCELL_REFMUT: [&str; 3] = ["core", "cell", "RefMut"];
pub const REGEX_BUILDER_NEW: [&str; 5] = ["regex", "re_builder", "unicode", "RegexBuilder", "new"];
Expand Down
12 changes: 6 additions & 6 deletions src/tools/clippy/tests/ui/vtable_address_comparisons.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,12 +23,6 @@ fn main() {
let b = &1 as &dyn Debug;
ptr::eq(a, b);

let a: Rc<dyn Debug> = Rc::new(1);
Rc::ptr_eq(&a, &a);

let a: Arc<dyn Debug> = Arc::new(1);
Arc::ptr_eq(&a, &a);

// These should be fine:
let a = &1;
ptr::eq(a, a);
Expand All @@ -39,6 +33,12 @@ fn main() {
let a = Arc::new(1);
Arc::ptr_eq(&a, &a);

let a: Rc<dyn Debug> = Rc::new(1);
Rc::ptr_eq(&a, &a);

let a: Arc<dyn Debug> = Arc::new(1);
Arc::ptr_eq(&a, &a);

let a: &[u8] = b"";
ptr::eq(a, a);
}
18 changes: 1 addition & 17 deletions src/tools/clippy/tests/ui/vtable_address_comparisons.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -63,21 +63,5 @@ LL | ptr::eq(a, b);
|
= help: consider extracting and comparing data pointers only

error: comparing trait object pointers compares a non-unique vtable address
--> $DIR/vtable_address_comparisons.rs:27:5
|
LL | Rc::ptr_eq(&a, &a);
| ^^^^^^^^^^^^^^^^^^
|
= help: consider extracting and comparing data pointers only

error: comparing trait object pointers compares a non-unique vtable address
--> $DIR/vtable_address_comparisons.rs:30:5
|
LL | Arc::ptr_eq(&a, &a);
| ^^^^^^^^^^^^^^^^^^^
|
= help: consider extracting and comparing data pointers only

error: aborting due to 10 previous errors
error: aborting due to 8 previous errors