Skip to content
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

#[instrument] on a fn returning Pin<Box<dyn Future>> leads to bogus unused_braces lint #1831

Closed
Swatinem opened this issue Jan 13, 2022 · 2 comments · Fixed by #2090
Closed
Labels
crate/attributes Related to the `tracing-attributes` crate kind/bug Something isn't working

Comments

@Swatinem
Copy link
Contributor

Bug Report

Version

0.1.29

Description

When using a fn that returns a Pin<Box<dyn Future>> and uses an async move block, I get a bogus unused_braces lint.

use std::future::Future;
use std::pin::Pin;

#[tracing::instrument]
pub fn instumented_fn() -> Pin<Box<dyn Future<Output = ()>>> {
    Box::pin(async move { () })
}

The lint is bogus and the suggestion would yield invalid code:


warning: unnecessary braces around block return value
  --> src/main.rs:50:25
   |
50 |     Box::pin(async move { () })
   |                         ^^  ^^
   |
   = note: `#[warn(unused_braces)]` on by default
help: remove these braces
   |
50 -     Box::pin(async move { () })
50 +     Box::pin(async move ())
   | 

In our original code we had something along the lines of Box::pin(async move { foo.await.map_err(…)? }).

@hawkw
Copy link
Member

hawkw commented Jan 13, 2022

Huh, that's weird! I wonder what it is about the #[instrument] codegen that's incorrectly triggering that lint --- it should still always be generating an async move block...

@hawkw hawkw added crate/attributes Related to the `tracing-attributes` crate kind/bug Something isn't working labels Jan 13, 2022
@Swatinem
Copy link
Contributor Author

Swatinem commented Jan 14, 2022

Looking at the expanded source, I highlighted the redundant braces that I think rustc is pointing at:

Mind you, the expanded output might be super hard to read.

pub fn instumented_fn() -> Pin<Box<dyn Future<Output = ()>>> {
    Box::pin(async move {
        let __tracing_attr_span = {
            use ::tracing::__macro_support::Callsite as _;
            static CALLSITE: ::tracing::__macro_support::MacroCallsite = {
                use ::tracing::__macro_support::MacroCallsite;
                static META: ::tracing::Metadata<'static> = {
                    ::tracing_core::metadata::Metadata::new(
                        "instumented_fn",
                        "instrument_repro",
                        tracing::Level::INFO,
                        Some("src/main.rs"),
                        Some(4u32),
                        Some("instrument_repro"),
                        ::tracing_core::field::FieldSet::new(
                            &[],
                            ::tracing_core::callsite::Identifier(&CALLSITE),
                        ),
                        ::tracing::metadata::Kind::SPAN,
                    )
                };
                MacroCallsite::new(&META)
            };
            let mut interest = ::tracing::subscriber::Interest::never();
            if tracing::Level::INFO <= ::tracing::level_filters::STATIC_MAX_LEVEL
                && tracing::Level::INFO <= ::tracing::level_filters::LevelFilter::current()
                && {
                    interest = CALLSITE.interest();
                    !interest.is_never()
                }
                && CALLSITE.is_enabled(interest)
            {
                let meta = CALLSITE.metadata();
                ::tracing::Span::new(meta, &{ meta.fields().value_set(&[]) })
            } else {
                let span = CALLSITE.disabled_span();
                {};
                span
            }
        };
        let __tracing_instrument_future = async move {
            { // <--
                ()
            } // <--
        };
        if !__tracing_attr_span.is_disabled() {
            tracing::Instrument::instrument(__tracing_instrument_future, __tracing_attr_span).await
        } else {
            __tracing_instrument_future.await
        }
    })
}

hawkw pushed a commit that referenced this issue Apr 25, 2022
## Motivation

When using an `async` block (as an alternative to `async fn`, e.g. when
implementing a trait), `#[instrument]` adds extra braces around the
wrapped `async` block. This causes `rustc` to emit an `unused_braces`
lint in some cases (usually for single-line `async` blocks, as far as I
can tell). See #1831 for an example.

## Solution

Since the `async` block extracted by `AsyncInfo::from_fn` already has
braces around its statements, there's no need to wrap it with additional
braces. This updates `gen_block` to remove those extra braces when
generating the code providing the value of
`__tracing_instrument_future`.

- [x] add repros for `unused_braces` issue
- [x] remove extra braces from async blocks

Fixes #1831
hawkw pushed a commit that referenced this issue Apr 26, 2022
## Motivation

When using an `async` block (as an alternative to `async fn`, e.g. when
implementing a trait), `#[instrument]` adds extra braces around the
wrapped `async` block. This causes `rustc` to emit an `unused_braces`
lint in some cases (usually for single-line `async` blocks, as far as I
can tell). See #1831 for an example.

## Solution

Since the `async` block extracted by `AsyncInfo::from_fn` already has
braces around its statements, there's no need to wrap it with additional
braces. This updates `gen_block` to remove those extra braces when
generating the code providing the value of
`__tracing_instrument_future`.

- [x] add repros for `unused_braces` issue
- [x] remove extra braces from async blocks

Fixes #1831
hawkw pushed a commit that referenced this issue Apr 26, 2022
## Motivation

When using an `async` block (as an alternative to `async fn`, e.g. when
implementing a trait), `#[instrument]` adds extra braces around the
wrapped `async` block. This causes `rustc` to emit an `unused_braces`
lint in some cases (usually for single-line `async` blocks, as far as I
can tell). See #1831 for an example.

## Solution

Since the `async` block extracted by `AsyncInfo::from_fn` already has
braces around its statements, there's no need to wrap it with additional
braces. This updates `gen_block` to remove those extra braces when
generating the code providing the value of
`__tracing_instrument_future`.

- [x] add repros for `unused_braces` issue
- [x] remove extra braces from async blocks

Fixes #1831
hawkw pushed a commit that referenced this issue Apr 26, 2022
## Motivation

When using an `async` block (as an alternative to `async fn`, e.g. when
implementing a trait), `#[instrument]` adds extra braces around the
wrapped `async` block. This causes `rustc` to emit an `unused_braces`
lint in some cases (usually for single-line `async` blocks, as far as I
can tell). See #1831 for an example.

## Solution

Since the `async` block extracted by `AsyncInfo::from_fn` already has
braces around its statements, there's no need to wrap it with additional
braces. This updates `gen_block` to remove those extra braces when
generating the code providing the value of
`__tracing_instrument_future`.

- [x] add repros for `unused_braces` issue
- [x] remove extra braces from async blocks

Fixes #1831
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
crate/attributes Related to the `tracing-attributes` crate kind/bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants