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

Invalid help suggestion when vec macro incorrectly used without ! #101490

Open
Rageking8 opened this issue Sep 6, 2022 · 6 comments
Open

Invalid help suggestion when vec macro incorrectly used without ! #101490

Rageking8 opened this issue Sep 6, 2022 · 6 comments
Assignees
Labels
A-diagnostics Area: Messages for errors, warnings, and lints A-parser Area: The parsing of Rust source code to an AST A-resolve Area: Name/path resolution done by `rustc_resolve` specifically D-papercut Diagnostics: An error or lint that needs small tweaks. P-low Low priority T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@Rageking8
Copy link
Contributor

Rageking8 commented Sep 6, 2022

Given the following code: link

fn main() {
    let _x = vec[1, 2, 3];
}

The current output is:

   Compiling playground v0.0.1 (/playground)
error: expected `;`, found `[`
 --> src/main.rs:2:17
  |
2 |     let _x = vec[1, 2, 3];
  |                 ^
  |
help: consider adding `;` here
  |
2 |     let _x = vec;[1, 2, 3];
  |                 +

error: could not compile `playground` due to previous error

Just for some context this is the current error message for the stable and beta versions:

   Compiling playground v0.0.1 (/playground)
error: expected one of `.`, `?`, `]`, or an operator, found `,`
 --> src/main.rs:2:19
  |
2 |     let _x = vec[1, 2, 3];
  |                   ^ expected one of `.`, `?`, `]`, or an operator

error: could not compile `playground` due to previous error

Which is also not ideal as for the other std macros AFAIK (only checked a few), incorrect usage (i.e. without the !) will lead to a help suggestion informing the user to include a ! to invoke a macro (e.g. print, println).

Hence, the ideal output is to suggest adding the ! when invoking the vec macro incorrectly as with other macros.

@rustbot label +regression-from-stable-to-nightly +D-invalid-suggestion

May need a bisect.

@Rageking8 Rageking8 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 Sep 6, 2022
@rustbot rustbot added D-invalid-suggestion Diagnostics: A structured suggestion resulting in incorrect code. regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. I-prioritize Issue: Indicates that prioritization has been requested for this issue. labels Sep 6, 2022
@lukas-code
Copy link
Member

Did not bisect, but this is probably caused by #100334.

@apiraino
Copy link
Contributor

apiraino commented Sep 6, 2022

LOL at the new suggestion

anyway, yes confirmed the bisection points to a rollup merge with contains #100334 cc @TaKO8Ki

searched nightlies: from nightly-2022-07-13 to nightly-2022-09-06
regressed nightly: nightly-2022-08-11
searched commit range: 34a6cae...29e4a9e
regressed commit: 1603a70

bisected with cargo-bisect-rustc v0.6.3

Host triple: x86_64-unknown-linux-gnu
Reproduce with:

cargo bisect-rustc --start 2022-07-13 --preserve 

I'll try to dig further back in time to find where we lost the correct diagnostic message

EDIT: the diagnostic we had previously was introduced in Rust 1.18 so basically I think we never really suggested the right thing ("adding the ! when invoking the vec macro incorrectly as with other macros.")

@TaKO8Ki TaKO8Ki self-assigned this Sep 6, 2022
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Sep 7, 2022
…or-macro-without-exclamation-mark, r=wesleywiser

Do not suggest a semicolon for a macro without `!`

Fixes a regression in rust-lang#101490
@apiraino apiraino removed the I-prioritize Issue: Indicates that prioritization has been requested for this issue. label Sep 8, 2022
@Rageking8
Copy link
Contributor Author

The regression that causes the compiler to emit the invalid help suggestion is fixed by #101502
Hence, the current output error message is:

   Compiling playground v0.0.1 (/playground)
error: expected one of `.`, `?`, `]`, or an operator, found `,`
 --> src/main.rs:2:19
  |
2 |     let _x = vec[1, 2, 3];
  |                   ^ expected one of `.`, `?`, `]`, or an operator

error: could not compile `playground` due to previous error

However, as mentioned in the top of this issue, it is still not ideal.
Therefore, "suggest adding the ! when invoking the vec macro incorrectly as with other macros." this part still needs to get implemented.

@estebank
Copy link
Contributor

Doing the "correct" thing and emitting an always correct "add ! here" suggestion will require delaying a parse error until resolution is available, which is quite hard to do. Alternatively, we could customize the check to tentatively mention macros when encountering ident[<parse error> not dissimilar to how we treat ident{<parse error>.

@apiraino
Copy link
Contributor

WG-prioritization assigning priority (Zulip discussion).

@rustbot label -I-prioritize +P-low +regression-from-stable-to-stable -regression-from-stable-to-nightly

@rustbot rustbot added P-low Low priority regression-from-stable-to-stable Performance or correctness regression from one stable version to another. and removed regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. labels May 19, 2023
@estebank
Copy link
Contributor

The current output is back to what it originally was

error: expected one of `.`, `?`, `]`, or an operator, found `,`
 --> src/main.rs:2:19
  |
2 |     let _x = vec[1, 2, 3];
  |                   ^ expected one of `.`, `?`, `]`, or an operator

I don't think it qualifies as a regression anymore, but it would be great still to make the output be the following instead

error: missing `!` in macro invokation
 --> src/main.rs:2:19
  |
2 |     let _x = vec[1, 2, 3];
  |                 ^ expected a `!` here

@estebank estebank added A-resolve Area: Name/path resolution done by `rustc_resolve` specifically A-parser Area: The parsing of Rust source code to an AST D-papercut Diagnostics: An error or lint that needs small tweaks. and removed regression-from-stable-to-stable Performance or correctness regression from one stable version to another. D-invalid-suggestion Diagnostics: A structured suggestion resulting in incorrect code. labels May 20, 2023
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-parser Area: The parsing of Rust source code to an AST A-resolve Area: Name/path resolution done by `rustc_resolve` specifically D-papercut Diagnostics: An error or lint that needs small tweaks. P-low Low priority 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

6 participants