From 4459124ecae1e207eb9a4d3b0dfaaeeccc7bff50 Mon Sep 17 00:00:00 2001 From: Eliza Weisman Date: Fri, 12 Jun 2020 10:21:42 -0700 Subject: [PATCH] core: compare `callsite::Identifier` data pointers only (#749) ## 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 --- tracing-core/src/callsite.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tracing-core/src/callsite.rs b/tracing-core/src/callsite.rs index 8b672b6a1e..1ee3254fb2 100644 --- a/tracing-core/src/callsite.rs +++ b/tracing-core/src/callsite.rs @@ -3,7 +3,6 @@ use crate::stdlib::{ fmt, hash::{Hash, Hasher}, - ptr, sync::Mutex, vec::Vec, }; @@ -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 () } }