-
Notifications
You must be signed in to change notification settings - Fork 13.3k
Add pointing const identifier when emitting E0435 #80012
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Add pointing const identifier when emitting E0435 #80012
Conversation
(rust-highfive has picked a reviewer for you, use r? to override) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
r? @petrochenkov maybe?
I don't think this is necessary. |
The same item, yes, but not the same part of the code.
People just learning rust might not know that 'constant' means 'a compile-time value' instead of 'immutable'.
Hmm, I think the difference is that most errors aren't specific to it being a function, it would occur in any expression. The exception is things like capturing environment in a function (https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=283552032a0f0dccad62310b4b811e42) and I would argue we should point to cc @estebank |
I agree with @jyn514. It's not necessary but it's helpful to the new rust users from other language has constant keyword like JavaScript.
Improving E0435 seems another solution to fix it. To add descriptions like |
@sasurau4 sure, that sounds good :) there's a diagnostic like that for Box IIRC. |
The code changes themselves look reasonable. The ui change I'm ambivalent about. I see in the original report why this might be a problem for people coming from JS, but I don't know that this change will be a solution. I can see other possibilities, like detecting that the @sasurau4 would you be interested in potentially implementing the above? |
@estebank I'm interested in that and try it out! |
2c3ca68
to
00cfca8
Compare
The job Click to see the possible cause of the failure (guessed by this bot)
|
00cfca8
to
050c517
Compare
@rustbot modify labels to +S-waiting-on-review, -S-waiting-on-author |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks amazing ❤️ ❤️
I think someone else should review the implementation, but I love the error messages :)
This comment has been minimized.
This comment has been minimized.
a3fadec
to
c71348a
Compare
@rustbot modify labels to +S-waiting-on-review, -S-waiting-on-author |
Thanks! |
📌 Commit c71348a has been approved by |
…ntifier-E0435, r=petrochenkov Add pointing const identifier when emitting E0435 Fix rust-lang#79919
Rollup of 10 pull requests Successful merges: - rust-lang#80012 (Add pointing const identifier when emitting E0435) - rust-lang#80521 (MIR Inline is incompatible with coverage) - rust-lang#80659 (Edit rustc_ast::tokenstream docs) - rust-lang#80660 (Properly handle primitive disambiguators in rustdoc) - rust-lang#80738 (Remove bottom margin from crate version when the docs sidebar is collapsed) - rust-lang#80744 (rustdoc: Turn `next_def_id` comments into docs) - rust-lang#80750 (Don't use to_string on Symbol in rustc_passes/check_attr.rs) - rust-lang#80769 (Improve wording of parse doc) - rust-lang#80780 (Return EOF_CHAR constant instead of magic char.) - rust-lang#80784 (rustc_parse: Better spans for synthesized token streams) Failed merges: - rust-lang#80785 (rustc_ast_pretty: Remove `PrintState::insert_extra_parens`) r? `@ghost` `@rustbot` modify labels: rollup
On structured suggestion for `let` -> `const` and `const` -> `let`, use a proper `Span` and update tests to check the correct application. Follow up to rust-lang#80012.
… r=petrochenkov Use correct span for structured suggestion On structured suggestion for `let` -> `const` and `const` -> `let`, use a proper `Span` and update tests to check the correct application. Follow up to rust-lang#80012.
… r=petrochenkov Use correct span for structured suggestion On structured suggestion for `let` -> `const` and `const` -> `let`, use a proper `Span` and update tests to check the correct application. Follow up to rust-lang#80012.
Fix #79919