Skip to content

Commit

Permalink
core: implement PartialEq, Eq for Metadata, FieldSet (#2229)
Browse files Browse the repository at this point in the history
A `FieldSet` is equal to another `FieldSet` if they share the same
callsite and fields (provided in the same order). This ensures
that a `Field` applicable to one `FieldSet` is applicable to any
equal `FieldSet`. A `Metadata` is equal to another `Metadata` if
all of their contained metadata is exactly equal.

This change manually re-implements `PartialEq` and `Eq` for
`Metadata` and `FieldSet` to define their equality strictly in
terms of callsite equality. In debug builds, the equality of
these types' other fields is also checked.

Documentation is added to both `Metadata` and `FieldSet`
explaining this behavior.

The expectation that, in a well-behaving application, `Metadata`
and `FieldSet`s with equal callsites will be otherwise equal is
documented on `Callsite::metadata`. This is not a breaking change,
as previous releases did not define equality for `Metadata` or
`FieldSet`. The `Callsite` trait remains safe, as this expectation
is not (yet) a safety-critical property.
  • Loading branch information
jswrenn authored and hawkw committed Jul 20, 2022
1 parent 67b9474 commit f4da668
Show file tree
Hide file tree
Showing 3 changed files with 124 additions and 18 deletions.
9 changes: 9 additions & 0 deletions tracing-core/src/callsite.rs
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,15 @@ pub trait Callsite: Sync {

/// Returns the [metadata] associated with the callsite.
///
/// <div class="example-wrap" style="display:inline-block">
/// <pre class="ignore" style="white-space:normal;font:inherit;">
///
/// **Note:** Implementations of this method should not produce [`Metadata`]
/// that share the same callsite [`Identifier`] but otherwise differ in any
/// way (e.g., have different `name`s).
///
/// </pre></div>
///
/// [metadata]: super::metadata::Metadata
fn metadata(&self) -> &Metadata<'_>;

Expand Down
44 changes: 44 additions & 0 deletions tracing-core/src/field.rs
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,16 @@ pub struct Field {
pub struct Empty;

/// Describes the fields present on a span.
///
/// ## Equality
///
/// In well-behaved applications, two `FieldSet`s [initialized] with equal
/// [callsite identifiers] will have identical fields. Consequently, in release
/// builds, [`FieldSet::eq`] *only* checks that its arguments have equal
/// callsites. However, the equality of field names is checked in debug builds.
///
/// [initialized]: Self::new
/// [callsite identifiers]: callsite::Identifier
pub struct FieldSet {
/// The names of each field on the described span.
names: &'static [&'static str],
Expand Down Expand Up @@ -911,6 +921,40 @@ impl fmt::Display for FieldSet {
}
}

impl Eq for FieldSet {}

impl PartialEq for FieldSet {
fn eq(&self, other: &Self) -> bool {
if core::ptr::eq(&self, &other) {
true
} else if cfg!(not(debug_assertions)) {
// In a well-behaving application, two `FieldSet`s can be assumed to
// be totally equal so long as they share the same callsite.
self.callsite == other.callsite
} else {
// However, when debug-assertions are enabled, do NOT assume that
// the application is well-behaving; check every the field names of
// each `FieldSet` for equality.

// `FieldSet` is destructured here to ensure a compile-error if the
// fields of `FieldSet` change.
let Self {
names: lhs_names,
callsite: lhs_callsite,
} = self;

let Self {
names: rhs_names,
callsite: rhs_callsite,
} = &other;

// Check callsite equality first, as it is probably cheaper to do
// than str equality.
lhs_callsite == rhs_callsite && lhs_names == rhs_names
}
}
}

// ===== impl Iter =====

impl Iterator for Iter {
Expand Down
89 changes: 71 additions & 18 deletions tracing-core/src/metadata.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,28 +35,25 @@ use crate::stdlib::{
/// _significantly_ lower than that of creating the actual span. Therefore,
/// filtering is based on metadata, rather than on the constructed span.
///
/// <pre class="ignore" style="white-space:normal;font:inherit;">
/// <strong>Note</strong>: Although instances of <code>Metadata</code>
/// cannot be compared directly, they provide a method
/// <a href="struct.Metadata.html#method.id"><code>id</code></a>, returning
/// an opaque <a href="../callsite/struct.Identifier.html">callsite
/// identifier</a>which uniquely identifies the callsite where the metadata
/// originated. This can be used to determine if two <code>Metadata</code>
/// correspond to the same callsite.
/// </pre>
/// ## Equality
///
/// In well-behaved applications, two `Metadata` with equal
/// [callsite identifiers] will be equal in all other ways (i.e., have the same
/// `name`, `target`, etc.). Consequently, in release builds, [`Metadata::eq`]
/// *only* checks that its arguments have equal callsites. However, the equality
/// of `Metadata`'s other fields is checked in debug builds.
///
/// [span]: super::span
/// [event]: super::event
/// [name]: Metadata::name()
/// [target]: Metadata::target()
/// [fields]: Metadata::fields()
/// [verbosity level]: Metadata::level()
/// [file name]: Metadata::file()
/// [line number]: Metadata::line()
/// [module path]: Metadata::module_path()
/// [name]: Self::name
/// [target]: Self::target
/// [fields]: Self::fields
/// [verbosity level]: Self::level
/// [file name]: Self::file
/// [line number]: Self::line
/// [module path]: Self::module_path
/// [`Subscriber`]: super::subscriber::Subscriber
/// [`id`]: Metadata::id
/// [callsite identifier]: super::callsite::Identifier
/// [callsite identifiers]: Self::callsite
pub struct Metadata<'a> {
/// The name of the span described by this metadata.
name: &'static str,
Expand Down Expand Up @@ -443,6 +440,62 @@ impl fmt::Debug for Kind {
}
}

impl<'a> Eq for Metadata<'a> {}

impl<'a> PartialEq for Metadata<'a> {
#[inline]
fn eq(&self, other: &Self) -> bool {
if core::ptr::eq(&self, &other) {
true
} else if cfg!(not(debug_assertions)) {
// In a well-behaving application, two `Metadata` can be assumed to
// be totally equal so long as they share the same callsite.
self.callsite() == other.callsite()
} else {
// However, when debug-assertions are enabled, do not assume that
// the application is well-behaving; check every field of `Metadata`
// for equality.

// `Metadata` is destructured here to ensure a compile-error if the
// fields of `Metadata` change.
let Metadata {
name: lhs_name,
target: lhs_target,
level: lhs_level,
module_path: lhs_module_path,
file: lhs_file,
line: lhs_line,
fields: lhs_fields,
kind: lhs_kind,
} = self;

let Metadata {
name: rhs_name,
target: rhs_target,
level: rhs_level,
module_path: rhs_module_path,
file: rhs_file,
line: rhs_line,
fields: rhs_fields,
kind: rhs_kind,
} = &other;

// The initial comparison of callsites is purely an optimization;
// it can be removed without affecting the overall semantics of the
// expression.
self.callsite() == other.callsite()
&& lhs_name == rhs_name
&& lhs_target == rhs_target
&& lhs_level == rhs_level
&& lhs_module_path == rhs_module_path
&& lhs_file == rhs_file
&& lhs_line == rhs_line
&& lhs_fields == rhs_fields
&& lhs_kind == rhs_kind
}
}
}

// ===== impl Level =====

impl Level {
Expand Down

0 comments on commit f4da668

Please sign in to comment.