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

"consider using a semicolon here" when ; is present with dbg!() inside inspect() #81943

Closed
matthiaskrgr opened this issue Feb 9, 2021 · 1 comment · Fixed by #82090
Closed
Labels
A-diagnostics Area: Messages for errors, warnings, and lints A-suggestion-diagnostics Area: Suggestions generated by the compiler applied by `cargo fix`. D-invalid-suggestion Diagnostics: A structured suggestion resulting in incorrect code. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@matthiaskrgr
Copy link
Member

fn main() {
let sum: i32 = [1,2,3].iter().inspect(|n| dbg!(n) ).sum();
}

This will suggest adding a ;

error[E0308]: mismatched types
 --> src/main.rs:3:43
  |
3 | let sum: i32 = [1,2,3].iter().inspect(|n| dbg!(n) ).sum();
  |                                           ^^^^^^^
  |                                           |
  |                                           expected `()`, found `&&{integer}`
  |                                           expected this to be `()`
  |                                           help: consider using a semicolon here
  |
  = note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info)

But doing so will "break" the code even more and not fix the error:

error: expected one of `)`, `,`, `.`, `?`, or an operator, found `;`
--> src/main.rs:2:50
 |
2 | let sum: i32 = [1,2,3].iter().inspect(|n| dbg!(n); ).sum();
 |                                                  ^ expected one of `)`, `,`, `.`, `?`, or an operator

error[E0308]: mismatched types
--> src/main.rs:2:43
 |
2 | let sum: i32 = [1,2,3].iter().inspect(|n| dbg!(n); ).sum();
 |                                           ^^^^^^^
 |                                           |
 |                                           expected `()`, found `&&{integer}`
 |                                           expected this to be `()`
 |                                           help: consider using a semicolon here
 |
 = note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info)

Could rust suggest adding { } around the macro?

@osa1
Copy link
Contributor

osa1 commented Feb 10, 2021

The problem is we look at the macro-expanded code when generating this suggestion, which looks like this:

fn main() {
    let sum: i32 = [1, 2, 3]
        .iter()
        .inspect(|n| match n {
            tmp => {
                ... macro-generated stuff here ...
                tmp
            }
        })
        .sum();
}

Note that there are braces around tmp here. In this case adding a semicolon does work. So in that sense the suggestion is correct.

The problem is it's looking at macro-expanded code and making the suggestion based on that.

I can think of a few ways of improving this error:

  • Not making any suggestions is better than making wrong suggestions, so we could just avoid this suggestion when the problematic code is macro expansion. If I implement this the error becomes:

    error[E0308]: mismatched types
     --> src/main.rs:2:49
      |
    2 |     let sum: i32 = [1, 2, 3].iter().inspect(|n| dbg!(n)).sum();
      |                                                 ^^^^^^^
      |                                                 |
      |                                                 expected `()`, found `&&{integer}`
      |                                                 expected this to be `()`
      |
      = note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info)
    
  • Somehow check that the pre-macro-expansion code doesn't have {} but the expected type is (), and suggest adding braces. I don't know if it's possible to look at pre-macro-expansion code when type checking.

  • Try to recover by adding a semicolon in the original code and type checking the macro expansion again. I'm not sure if this is even possible (possibly requires huge amounts of work unless the code is already there for this kind of recovery). If this is possible then we only suggest adding a semicolon when type checking the recovered version (with ;) works.

@estebank estebank added A-diagnostics Area: Messages for errors, warnings, and lints A-suggestion-diagnostics Area: Suggestions generated by the compiler applied by `cargo fix`. D-invalid-suggestion Diagnostics: A structured suggestion resulting in incorrect code. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Feb 10, 2021
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue Feb 23, 2021
…n-here, r=estebank

Do not consider using a semicolon inside of a different-crate macro

Fixes rust-lang#81943
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue Feb 24, 2021
…n-here, r=estebank

Do not consider using a semicolon inside of a different-crate macro

Fixes rust-lang#81943
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue Feb 24, 2021
…n-here, r=estebank

Do not consider using a semicolon inside of a different-crate macro

Fixes rust-lang#81943
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue Feb 25, 2021
…n-here, r=estebank

Do not consider using a semicolon inside of a different-crate macro

Fixes rust-lang#81943
@bors bors closed this as completed in de6f1b8 Feb 25, 2021
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-suggestion-diagnostics Area: Suggestions generated by the compiler applied by `cargo fix`. D-invalid-suggestion Diagnostics: A structured suggestion resulting in incorrect code. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants