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

Wrong diagnostic when incorrectly using const generics in impl block #79598

Closed
rylev opened this issue Dec 1, 2020 · 2 comments · Fixed by #79697
Closed

Wrong diagnostic when incorrectly using const generics in impl block #79598

rylev opened this issue Dec 1, 2020 · 2 comments · Fixed by #79697
Labels
A-diagnostics Area: Messages for errors, warnings, and lints A-parser Area: The parsing of Rust source code to an AST C-bug Category: This is a bug. const-generics-bad-diagnostics An error is correctly emitted, but is confusing, for `min_const_generics`. F-const_generics `#![feature(const_generics)]` T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@rylev
Copy link
Member

rylev commented Dec 1, 2020

The following incorrect code produces an incorrect diagnostic

#![feature(min_const_generics)]

struct Foo<const SET: bool>;

impl Foo<const SET: bool> {
    fn set_bar() -> Foo<true> {
        Foo
    }
}

The diagnostic produced is:

error: expected one of `>`, const, lifetime, or type, found keyword `const`
 --> src/lib.rs:5:10
  |
5 | impl Foo<const SET: bool> {
  |          ^^^^^ expected one of `>`, const, lifetime, or type

Confusingly const is shown as one of the expected items to replace const.

Of course, the user most likely meant to type:

impl <const SET: bool> Foo<SET> {

Here's a playground reproducing the issue for convenience: https://play.rust-lang.org/?version=nightly&mode=debug&edition=2018&gist=27ae1b8286d97da1df8240f84062a339

@rylev rylev added the C-bug Category: This is a bug. label Dec 1, 2020
@jonas-schievink jonas-schievink added A-diagnostics Area: Messages for errors, warnings, and lints A-parser Area: The parsing of Rust source code to an AST T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Dec 1, 2020
@oli-obk oli-obk added the const-generics-bad-diagnostics An error is correctly emitted, but is confusing, for `min_const_generics`. label Dec 1, 2020
@lcnr lcnr added the F-const_generics `#![feature(const_generics)]` label Dec 1, 2020
@fmease
Copy link
Member

fmease commented Dec 2, 2020

error: expected one of `>`, const, lifetime, or type, found keyword `const`

In the error message, the first instance of const is not quoted (surrounded by backticks) compared to the second one. It does not refer to the keyword named const but to const arguments/parameters. In that position, const arguments like N, 0 or {0 + 1} would be legal, consider impl Foo<0> { … }.

So technically, the diagnostic is not wrong, but confusing for the uninitiated.
One could modify the listing to "const, lifetime, type argument". However, that might not make it more comprehensible.

@rylev
Copy link
Member Author

rylev commented Dec 4, 2020

tl;dr @fmease's (ultimately more concise) analysis is correct. I'll push a fix that makes the wording at least a little clearer.

Since I've never touched the parser before, I did some investigation, and here is what I found:

(please forgive the long prose, but I thought having this explanation might be useful instead of me just deleting it)

The error starts in parse_path_segment when we first parse angle args and then expect a >. Because const does not make sense in this context, parse_angle_args_with_leading_angle_bracket_recovery returns an empty set of angle args. expect_gt then fails because it's looking for > but will find const.

The error case in expect_gt will call Parser::unexpected which calls Parser::expect_one_of(&[], &[]). Since edible and inedible are both empty, expected_one_of_not_found gets called which will build the error message ultimately only using self.expected_tokens as the tokens to fill the "expected" string.

Ultimately, these expected tokens are converted to a string inside of tokens_to_string which calls to_string on TokenType. Since one of the expected tokens is TokenType::Const, const shows up in the list of expected tokens.

TokenType::Const ended up in Parser.expected_tokens because of the call to check_const_arg in parse_generic_arg.

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 C-bug Category: This is a bug. const-generics-bad-diagnostics An error is correctly emitted, but is confusing, for `min_const_generics`. F-const_generics `#![feature(const_generics)]` 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.

5 participants