Skip to content

Commit

Permalink
attributes: fix #[instrument(err)] with mutable parameters (#1167)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
okready authored and hawkw committed Feb 4, 2021
1 parent f5f99dd commit 72eb468
Show file tree
Hide file tree
Showing 2 changed files with 57 additions and 1 deletion.
4 changes: 3 additions & 1 deletion tracing-attributes/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -631,6 +631,7 @@ fn gen_body(
let __tracing_attr_span = #span;
tracing::Instrument::instrument(async move {
match async move { #block }.await {
#[allow(clippy::unit_arg)]
Ok(x) => Ok(x),
Err(e) => {
tracing::error!(error = %e);
Expand All @@ -653,7 +654,8 @@ fn gen_body(
quote_spanned!(block.span()=>
let __tracing_attr_span = #span;
let __tracing_attr_guard = __tracing_attr_span.enter();
match { #block } {
match move || #return_type #block () {
#[allow(clippy::unit_arg)]
Ok(x) => Ok(x),
Err(e) => {
tracing::error!(error = %e);
Expand Down
54 changes: 54 additions & 0 deletions tracing-attributes/tests/err.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,3 +62,57 @@ fn test_async() {
});
handle.assert_finished();
}

#[instrument(err)]
fn err_mut(out: &mut u8) -> Result<(), TryFromIntError> {
*out = u8::try_from(1234)?;
Ok(())
}

#[test]
fn test_mut() {
let span = span::mock().named("err_mut");
let (subscriber, handle) = subscriber::mock()
.new_span(span.clone())
.enter(span.clone())
.event(event::mock().at_level(Level::ERROR))
.exit(span.clone())
.drop_span(span)
.done()
.run_with_handle();
with_default(subscriber, || err_mut(&mut 0).ok());
handle.assert_finished();
}

#[instrument(err)]
async fn err_mut_async(polls: usize, out: &mut u8) -> Result<(), TryFromIntError> {
let future = PollN::new_ok(polls);
tracing::trace!(awaiting = true);
future.await.ok();
*out = u8::try_from(1234)?;
Ok(())
}

#[test]
fn test_mut_async() {
let span = span::mock().named("err_mut_async");
let (subscriber, handle) = subscriber::mock()
.new_span(span.clone())
.enter(span.clone())
.event(
event::mock()
.with_fields(field::mock("awaiting").with_value(&true))
.at_level(Level::TRACE),
)
.exit(span.clone())
.enter(span.clone())
.event(event::mock().at_level(Level::ERROR))
.exit(span.clone())
.drop_span(span)
.done()
.run_with_handle();
with_default(subscriber, || {
block_on_future(async { err_mut_async(2, &mut 0).await }).ok();
});
handle.assert_finished();
}

0 comments on commit 72eb468

Please sign in to comment.