rust-lang / rust

Empowering everyone to build reliable and efficient software.
https://www.rust-lang.org
Other
99.14k stars 12.8k forks source link

Heisenwarning `unused_braces` with `tracing` #116347

Open Cerber-Ursi opened 1 year ago

Cerber-Ursi commented 1 year ago

Code

#[tracing::instrument]
fn foo() -> u8 { 42 }

#[tracing::instrument]
fn bar() -> u8 { 
    42
}

Current output

warning: unnecessary braces around block return value
 --> src/lib.rs:2:16
  |
2 | fn foo() -> u8 { 42 }
  |                ^^  ^^
  |
  = note: `#[warn(unused_braces)]` on by default
help: remove these braces
  |
2 - fn foo() -> u8 { 42 }
2 + fn foo() -> u8 42
  |

Desired output

No response

Rationale and extra context

I'd expect both foo and bar to either trigger the warning or not, since they differ only in formatting (in fact, bar is just a rustfmt-ed foo). Looking just at the suggestion, it seems that not warning is better (since this suggestion is not applicable), but this might be a bug in tracing and not in rustc.

Other cases

No response

Anything else?

Initially found on URLO.

workingjubilee commented 1 year ago

The lint does not fire in this case:

fn foo() -> u8 { 42 }

fn bar() -> u8 { 
    42
}
p-kraszewski commented 1 year ago

Yes, but it does with #[tracing::instrument], which expands to the same code - and this expanded code somehow behaves differently in rustc for single- and multi-line function.

More discussion is here: https://users.rust-lang.org/t/probably-invalid-compiler-hint/100687/1 (I am the thread creator).

My compiler-internals-fu is weak, but @Cerber-Ursi has some interesting hypotheses.

workingjubilee commented 1 year ago

Peculiarly, the lint stops firing with this:

#[tracing::instrument]
fn foo() -> u8 { (); 42 }

But does fire on this:

#[tracing::instrument]
fn foo() -> u8 { dbg!(42) }

Thus I believe this is a bug in rustc, not in tracing.

Cerber-Ursi commented 1 year ago

Possibly-connected problem, in a sense that the code shown after expanding macros is not the same as the code used for compiling (therefore making debug problematic) - https://github.com/dtolnay/cargo-expand/issues/201/

shepmaster commented 8 months ago

I opened https://github.com/tokio-rs/tracing/issues/2830 about this and then we realized it's likely a rustc issue.

The braces warning is whitespace-sensitive:

fn warns_about_braces() -> u8 {
    { 1 }
}

fn does_not_warn_about_braces() -> u8 {
    { 
        1
    }
}
shepmaster commented 8 months ago

May be a duplicate of #88104 / #73068 / #70814