-
Notifications
You must be signed in to change notification settings - Fork 810
tracing: fix macros "use of moved value" with log #1823
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.
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
Changes from all commits
874a462
bc81ef5
0ad95a9
68b56d0
c20567a
50fd7f2
b4c6208
ce02eda
a691c5b
a4e8d26
0c1ab2b
794040e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -1112,6 +1112,68 @@ pub mod __macro_support { | |||||||||||||||||||||||||||||||||||
.finish() | ||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||
#[cfg(feature = "log")] | ||||||||||||||||||||||||||||||||||||
use tracing_core::field::{Field, ValueSet, Visit}; | ||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||
/// Utility to format [`ValueSet`] for logging, used by macro-generated code. | ||||||||||||||||||||||||||||||||||||
/// | ||||||||||||||||||||||||||||||||||||
/// /!\ WARNING: This is *not* a stable API! /!\ | ||||||||||||||||||||||||||||||||||||
/// This type, and all code contained in the `__macro_support` module, is | ||||||||||||||||||||||||||||||||||||
/// a *private* API of `tracing`. It is exposed publicly because it is used | ||||||||||||||||||||||||||||||||||||
/// by the `tracing` macros, but it is not part of the stable versioned API. | ||||||||||||||||||||||||||||||||||||
/// Breaking changes to this module may occur in small-numbered versions | ||||||||||||||||||||||||||||||||||||
/// without warning. | ||||||||||||||||||||||||||||||||||||
#[cfg(feature = "log")] | ||||||||||||||||||||||||||||||||||||
#[allow(missing_debug_implementations)] | ||||||||||||||||||||||||||||||||||||
pub struct LogValueSet<'a>(pub &'a ValueSet<'a>); | ||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i have a slight preference for giving this a
Skepfyr marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||
#[cfg(feature = "log")] | ||||||||||||||||||||||||||||||||||||
impl<'a> fmt::Display for LogValueSet<'a> { | ||||||||||||||||||||||||||||||||||||
Skepfyr marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||||||||||||||||||||
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { | ||||||||||||||||||||||||||||||||||||
let mut visit = LogVisitor { | ||||||||||||||||||||||||||||||||||||
f, | ||||||||||||||||||||||||||||||||||||
is_first: true, | ||||||||||||||||||||||||||||||||||||
result: Ok(()), | ||||||||||||||||||||||||||||||||||||
}; | ||||||||||||||||||||||||||||||||||||
self.0.record(&mut visit); | ||||||||||||||||||||||||||||||||||||
visit.result | ||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||
#[cfg(feature = "log")] | ||||||||||||||||||||||||||||||||||||
struct LogVisitor<'a, 'b> { | ||||||||||||||||||||||||||||||||||||
Skepfyr marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||||||||||||||||||||
f: &'a mut fmt::Formatter<'b>, | ||||||||||||||||||||||||||||||||||||
is_first: bool, | ||||||||||||||||||||||||||||||||||||
result: fmt::Result, | ||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||
#[cfg(feature = "log")] | ||||||||||||||||||||||||||||||||||||
impl Visit for LogVisitor<'_, '_> { | ||||||||||||||||||||||||||||||||||||
Skepfyr marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||||||||||||||||||||
fn record_debug(&mut self, field: &Field, value: &dyn fmt::Debug) { | ||||||||||||||||||||||||||||||||||||
let res = if self.is_first { | ||||||||||||||||||||||||||||||||||||
self.is_first = false; | ||||||||||||||||||||||||||||||||||||
if field.name() == "message" { | ||||||||||||||||||||||||||||||||||||
write!(self.f, "{:?}", value) | ||||||||||||||||||||||||||||||||||||
} else { | ||||||||||||||||||||||||||||||||||||
write!(self.f, "{}={:?}", field.name(), value) | ||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||
} else { | ||||||||||||||||||||||||||||||||||||
write!(self.f, " {}={:?}", field.name(), value) | ||||||||||||||||||||||||||||||||||||
}; | ||||||||||||||||||||||||||||||||||||
Comment on lines
+1154
to
+1163
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit, take it or leave it: this might look slightly nicer as a
Suggested change
though, up to you There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I prefer the explicit something is only weird about the first one of the if. However, your snippet does produce a different output to mine, in the case of message not being the first field. Mine would produce:
whereas yours would produce:
Is that behaviour ok, or should I change it to be in line with yours? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. in practice, the |
||||||||||||||||||||||||||||||||||||
if let Err(err) = res { | ||||||||||||||||||||||||||||||||||||
self.result = self.result.and(Err(err)); | ||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||
fn record_str(&mut self, field: &Field, value: &str) { | ||||||||||||||||||||||||||||||||||||
if field.name() == "message" { | ||||||||||||||||||||||||||||||||||||
self.record_debug(field, &format_args!("{}", value)) | ||||||||||||||||||||||||||||||||||||
} else { | ||||||||||||||||||||||||||||||||||||
self.record_debug(field, &value) | ||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||
mod sealed { | ||||||||||||||||||||||||||||||||||||
|
Uh oh!
There was an error while loading. Please reload this page.