Skip to content

Commit

Permalink
attributes: fix #[instrument(err)] with impl Trait return types (#…
Browse files Browse the repository at this point in the history
…1236)

This backports PR #1233 to v0.1.x. This isn't *just* a simple cherry-pick
because the new tests in that branch use the v0.2.x module names, so
that had to be fixed. Otherwise, though, it's the same change, and I'll go
ahead and merge it when CI passes, since it was approved on `master`.

## Motivation

Currently, using `#[instrument(err)]` on a function returning a `Result`
with an `impl Trait` in it results in a compiler error. This is because
we generate a type annotation on the closure in the function body that
contains the user function's actual body, and `impl Trait` isn't allowed
on types in local bindings, only on function parameters and return
types.

## Solution

This branch fixes the issue by simply removing the return type
annotation from the closure. I've also added tests that break on master
for functions returning `impl Trait`, both with and without the `err`
argument.

Fixes #1227

Signed-off-by: Eliza Weisman <eliza@buoyant.io>
  • Loading branch information
hawkw authored Feb 11, 2021
1 parent 4609f22 commit e1e3431
Show file tree
Hide file tree
Showing 3 changed files with 62 additions and 1 deletion.
3 changes: 2 additions & 1 deletion tracing-attributes/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -654,7 +654,8 @@ fn gen_body(
quote_spanned!(block.span()=>
let __tracing_attr_span = #span;
let __tracing_attr_guard = __tracing_attr_span.enter();
match move || #return_type #block () {
#[allow(clippy::redundant_closure_call)]
match (move || #block)() {
#[allow(clippy::unit_arg)]
Ok(x) => Ok(x),
Err(e) => {
Expand Down
31 changes: 31 additions & 0 deletions tracing-attributes/tests/err.rs
Original file line number Diff line number Diff line change
Expand Up @@ -116,3 +116,34 @@ fn test_mut_async() {
});
handle.assert_finished();
}

#[test]
fn impl_trait_return_type() {
// Reproduces https://github.com/tokio-rs/tracing/issues/1227

#[instrument(err)]
fn returns_impl_trait(x: usize) -> Result<impl Iterator<Item = usize>, String> {
Ok(0..x)
}

let span = span::mock().named("returns_impl_trait");

let (subscriber, handle) = subscriber::mock()
.new_span(
span.clone()
.with_field(field::mock("x").with_value(&format_args!("10")).only()),
)
.enter(span.clone())
.exit(span.clone())
.drop_span(span)
.done()
.run_with_handle();

with_default(subscriber, || {
for _ in returns_impl_trait(10).unwrap() {
// nop
}
});

handle.assert_finished();
}
29 changes: 29 additions & 0 deletions tracing-attributes/tests/instrument.rs
Original file line number Diff line number Diff line change
Expand Up @@ -200,3 +200,32 @@ fn methods() {

handle.assert_finished();
}

#[test]
fn impl_trait_return_type() {
#[instrument]
fn returns_impl_trait(x: usize) -> impl Iterator<Item = usize> {
0..x
}

let span = span::mock().named("returns_impl_trait");

let (subscriber, handle) = subscriber::mock()
.new_span(
span.clone()
.with_field(field::mock("x").with_value(&format_args!("10")).only()),
)
.enter(span.clone())
.exit(span.clone())
.drop_span(span)
.done()
.run_with_handle();

with_default(subscriber, || {
for _ in returns_impl_trait(10) {
// nop
}
});

handle.assert_finished();
}

0 comments on commit e1e3431

Please sign in to comment.