Skip to content

Commit

Permalink
Merge pull request #29 from rib/re-add-use-of-arc-ptr-eq
Browse files Browse the repository at this point in the history
Revert recent change to avoid using Arc:ptr_eq
  • Loading branch information
rib authored Dec 20, 2022
2 parents 77a5d23 + 86dd933 commit 76128d3
Showing 1 changed file with 8 additions and 24 deletions.
32 changes: 8 additions & 24 deletions src/verifier.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,31 +57,15 @@ struct VerifierClosure {
impl Eq for VerifierClosure {}
impl PartialEq for VerifierClosure {
fn eq(&self, other: &Self) -> bool {
// WARNING:
// The libs team has resolved to update Arc::ptr_eq so that it doesn't compare
// meta data for the wrapped value (such as a dyn trait vtable):
// https://github.com/rust-lang/rust/issues/103763
//
// `Arc::ptr_eq(&self.func, &other.func)` is effectively implemented as:
//
// `Arc::as_ptr(&arc_a) == Arc::as_ptr(&arc_b)`
//
// which will notably result in comparing the vtable pointer for (wide) trait object pointers
// and does _not_ match the API documentation that states:
//
// > Returns true if the two Arcs point to the same allocation (in a vein similar to ptr::eq).
//
// (which is what we need here)
//
// See: https://github.com/rust-lang/rust/pull/80505
//
// To ensure we are comparing the data pointer component of any wide pointer then we
// additionally cast `Arc::as_ptr(&arc)` to a thin pointer to discard the vtable
//
// This avoid getting a `clippy::vtable_address_comparisons` error
//
// Ref: https://github.com/rust-lang/rust-clippy/issues/6524

let a = Arc::as_ptr(&self.func) as *const u8;
let b = Arc::as_ptr(&other.func) as *const u8;
std::ptr::eq(a, b)
// In the meantime we can silence this clippy suggestion since the vtable is
// benign in this case anyway:
// https://github.com/rust-lang/rust-clippy/issues/6524
#[allow(clippy::vtable_address_comparisons)]
Arc::ptr_eq(&self.func, &other.func)
}
}
impl std::fmt::Debug for VerifierClosure {
Expand Down

0 comments on commit 76128d3

Please sign in to comment.