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 committed Feb 4, 2021
1 parent f05a0f2 commit c0939c3
Show file tree
Hide file tree
Showing 2 changed files with 57 additions and 2 deletions.
5 changes: 3 additions & 2 deletions tracing-attributes/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -446,6 +446,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 @@ -468,8 +469,8 @@ fn gen_body(
quote_spanned!(block.span()=>
let __tracing_attr_span = #span;
let __tracing_attr_guard = __tracing_attr_span.enter();
let f = move || #return_type { #block };
match f() {
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 @@ -83,3 +83,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 (collector, handle) = collector::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(collector, || 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 (collector, handle) = collector::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(collector, || {
block_on_future(async { err_mut_async(2, &mut 0).await }).ok();
});
handle.assert_finished();
}

0 comments on commit c0939c3

Please sign in to comment.