Skip to content

Commit 8a64865

Browse files
okreadyhawkw
authored andcommitted
attributes: fix #[instrument(err)] with mutable parameters (#1167)
Motivation ---------- The closure generated by `#[instrument(err)]` for non-async functions is not mutable, preventing any captured variables from being mutated. If a function has any mutable parameters that are modified within the function, adding the `#[instrument(err)]` attribute will result in a compiler error. Solution -------- This change makes it so the closure is executed directly as a temporary variable instead of storing it in a named variable prior to its use, avoiding the need to explicitly declare it as mutable altogether or to add an `#[allow(unused_mut)]` annotation on the closure declaration to silence lint warnings for closures that do not need to be mutable.
1 parent c9a58c4 commit 8a64865

File tree

2 files changed

+57
-1
lines changed

2 files changed

+57
-1
lines changed

tracing-attributes/src/lib.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -631,6 +631,7 @@ fn gen_body(
631631
let __tracing_attr_span = #span;
632632
tracing::Instrument::instrument(async move {
633633
match async move { #block }.await {
634+
#[allow(clippy::unit_arg)]
634635
Ok(x) => Ok(x),
635636
Err(e) => {
636637
tracing::error!(error = %e);
@@ -653,7 +654,8 @@ fn gen_body(
653654
quote_spanned!(block.span()=>
654655
let __tracing_attr_span = #span;
655656
let __tracing_attr_guard = __tracing_attr_span.enter();
656-
match { #block } {
657+
match move || #return_type #block () {
658+
#[allow(clippy::unit_arg)]
657659
Ok(x) => Ok(x),
658660
Err(e) => {
659661
tracing::error!(error = %e);

tracing-attributes/tests/err.rs

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,3 +62,57 @@ fn test_async() {
6262
});
6363
handle.assert_finished();
6464
}
65+
66+
#[instrument(err)]
67+
fn err_mut(out: &mut u8) -> Result<(), TryFromIntError> {
68+
*out = u8::try_from(1234)?;
69+
Ok(())
70+
}
71+
72+
#[test]
73+
fn test_mut() {
74+
let span = span::mock().named("err_mut");
75+
let (subscriber, handle) = subscriber::mock()
76+
.new_span(span.clone())
77+
.enter(span.clone())
78+
.event(event::mock().at_level(Level::ERROR))
79+
.exit(span.clone())
80+
.drop_span(span)
81+
.done()
82+
.run_with_handle();
83+
with_default(subscriber, || err_mut(&mut 0).ok());
84+
handle.assert_finished();
85+
}
86+
87+
#[instrument(err)]
88+
async fn err_mut_async(polls: usize, out: &mut u8) -> Result<(), TryFromIntError> {
89+
let future = PollN::new_ok(polls);
90+
tracing::trace!(awaiting = true);
91+
future.await.ok();
92+
*out = u8::try_from(1234)?;
93+
Ok(())
94+
}
95+
96+
#[test]
97+
fn test_mut_async() {
98+
let span = span::mock().named("err_mut_async");
99+
let (subscriber, handle) = subscriber::mock()
100+
.new_span(span.clone())
101+
.enter(span.clone())
102+
.event(
103+
event::mock()
104+
.with_fields(field::mock("awaiting").with_value(&true))
105+
.at_level(Level::TRACE),
106+
)
107+
.exit(span.clone())
108+
.enter(span.clone())
109+
.event(event::mock().at_level(Level::ERROR))
110+
.exit(span.clone())
111+
.drop_span(span)
112+
.done()
113+
.run_with_handle();
114+
with_default(subscriber, || {
115+
block_on_future(async { err_mut_async(2, &mut 0).await }).ok();
116+
});
117+
handle.assert_finished();
118+
}

0 commit comments

Comments
 (0)