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

Heisenwarning unused_braces with tracing #116347

Open
Cerber-Ursi opened this issue Oct 2, 2023 · 6 comments
Open

Heisenwarning unused_braces with tracing #116347

Cerber-Ursi opened this issue Oct 2, 2023 · 6 comments
Labels
A-diagnostics Area: Messages for errors, warnings, and lints A-lints Area: Lints (warnings about flaws in source code) such as unused_mut. C-bug Category: This is a bug. D-incorrect Diagnostics: A diagnostic that is giving misleading or incorrect information. D-invalid-suggestion Diagnostics: A structured suggestion resulting in incorrect code. L-unused_braces Lint: unused_braces T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@Cerber-Ursi
Copy link
Contributor

Cerber-Ursi commented Oct 2, 2023

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.

@Cerber-Ursi Cerber-Ursi added A-diagnostics Area: Messages for errors, warnings, and lints T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Oct 2, 2023
@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Oct 2, 2023
@workingjubilee
Copy link
Member

The lint does not fire in this case:

fn foo() -> u8 { 42 }

fn bar() -> u8 { 
    42
}

@workingjubilee workingjubilee added the A-lints Area: Lints (warnings about flaws in source code) such as unused_mut. label Oct 2, 2023
@p-kraszewski
Copy link

p-kraszewski commented Oct 2, 2023

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
Copy link
Member

workingjubilee commented Oct 2, 2023

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.

@Noratrieb Noratrieb added C-bug Category: This is a bug. and removed needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels Oct 2, 2023
@workingjubilee workingjubilee added D-invalid-suggestion Diagnostics: A structured suggestion resulting in incorrect code. D-incorrect Diagnostics: A diagnostic that is giving misleading or incorrect information. labels Oct 2, 2023
@Cerber-Ursi
Copy link
Contributor Author

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) - dtolnay/cargo-expand#201

@shepmaster
Copy link
Member

I opened tokio-rs/tracing#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
Copy link
Member

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

@jieyouxu jieyouxu added the L-unused_braces Lint: unused_braces label May 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-diagnostics Area: Messages for errors, warnings, and lints A-lints Area: Lints (warnings about flaws in source code) such as unused_mut. C-bug Category: This is a bug. D-incorrect Diagnostics: A diagnostic that is giving misleading or incorrect information. D-invalid-suggestion Diagnostics: A structured suggestion resulting in incorrect code. L-unused_braces Lint: unused_braces T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

7 participants