Skip to content

Commit

Permalink
core: compare callsite::Identifier data pointers only (tokio-rs#749)
Browse files Browse the repository at this point in the history
## Motivation

Clippy [now warns us][1] that comparing trait object pointers with
`ptr::eq` is bad, because vtable addresses are not guaranteed to be
unique due to the compiler merging vtables, and vtables for the same
trait impl may have different addresses in different compilation units.

In practice, I don't believe this actually effects `tracing-core`'s use
case for this comparison. Because callsites must be static, the data
component of the trait object wide pointer will always be unique, even
if the compiler merges the vtables for all the identical generated
`Callsite` impls (which it may very well do!).

## Solution

Although this is probably not an issue in practice, I still thought it
was good to fix the clippy warning, and to be more explicit that it's
the data pointer comparison that's load-bearing here. I've updated the
`PartialEq` impl for `callsite::Identifier` to cast to `*const ()` to
extract the data address from the wide pointer.

[1]: https://rust-lang.github.io/rust-clippy/master/index.html#vtable_address_comparisons

Signed-off-by: Eliza Weisman <eliza@buoyant.io>
  • Loading branch information
hawkw authored and theworldsbestcoder committed Jun 12, 2020
1 parent 168a521 commit 4459124
Showing 1 changed file with 1 addition and 2 deletions.
3 changes: 1 addition & 2 deletions tracing-core/src/callsite.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
use crate::stdlib::{
fmt,
hash::{Hash, Hasher},
ptr,
sync::Mutex,
vec::Vec,
};
Expand Down Expand Up @@ -121,7 +120,7 @@ pub(crate) fn register_dispatch(dispatch: &Dispatch) {

impl PartialEq for Identifier {
fn eq(&self, other: &Identifier) -> bool {
ptr::eq(self.0, other.0)
self.0 as *const _ as *const () == other.0 as *const _ as *const ()
}
}

Expand Down

0 comments on commit 4459124

Please sign in to comment.