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.

Sign up for GitHub

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

Jump to bottom

tracing-core: implement PartialEq, Eq for Metadata, FieldSet #2229

Merged
merged 2 commits into from Jul 19, 2022
Merged

tracing-core: implement PartialEq, Eq for Metadata, FieldSet #2229

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
338dea6
tracing-core: implement `PartialEq`, `Eq` for `Metadata`, `FieldSet`
jswrenn Jul 19, 2022
df1e749
tracing-core: equal callsites imply equal `Metadata`, `FieldSet`
jswrenn Jul 19, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions tracing-core/src/field.rs
View file
Open in desktop
Expand Up @@ -70,6 +70,7 @@ pub struct Field {
pub struct Empty;

/// Describes the fields present on a span.
jswrenn marked this conversation as resolved.
Show resolved Hide resolved
#[derive(Eq, PartialEq)]
pub struct FieldSet {
/// The names of each field on the described span.
names: &'static [&'static str],
Expand Down
12 changes: 1 addition & 11 deletions tracing-core/src/metadata.rs
View file
Open in desktop
Expand Up @@ -35,17 +35,6 @@ use core::{
/// _significantly_ lower than that of creating the actual span. Therefore,
/// filtering is based on metadata, rather than on the constructed span.
///
/// <div class="example-wrap" style="display:inline-block">
/// <pre class="ignore" style="white-space:normal;font:inherit;">
///
/// **Note**: Although instances of `Metadata` cannot
/// be compared directly, they provide a method [`callsite`][Metadata::callsite],
/// returning an opaque [callsite identifier] which uniquely identifies the
/// callsite where the metadata originated. This can be used to determine if two
/// `Metadata` correspond to the same callsite.
jswrenn marked this conversation as resolved.
Show resolved Hide resolved
///
/// </pre></div>
///
/// [span]: super::span
/// [event]: super::event
/// [name]: Self::name
Expand All @@ -57,6 +46,7 @@ use core::{
/// [module path]: Self::module_path
/// [collector]: super::collect::Collect
/// [callsite identifier]: super::callsite::Identifier
#[derive(Eq, PartialEq)]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we may want a hand-written equality implementation for Metadatas so that we can compare the callsite identifier first, and short-circuit so that we don't have to do a bunch of string equality. i believe the derived PartialEq impl will compare fields in the order that they are declared.

that change could be made later as an optimization, though.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

actually, i believe the derived implementations also generate an implementation of the StructuralPartialEq trait, which allows matching a type as a match pattern (which is possible in v0.1.x of tracing-core, where Metadata's fields are pub). this means that a change from a hand-written implementation to a derived one is potentially a breaking change...

pub struct Metadata<'a> {
/// The name of the span described by this metadata.
name: &'static str,
Expand Down