Hint correct extern constant syntax #43720
Conversation
|
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @nikomatsakis (or someone else) soon. If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes. Please see the contribution instructions for more information. |
src/libsyntax/parse/parser.rs
Outdated
| return Err(self.span_fatal(self.span, "extern items cannot be `const`")); | ||
| let mut err = self.span_fatal(self.span, "extern items cannot be `const`"); | ||
| err.help("use `static` instead"); | ||
| return Err(err); |
There was a problem hiding this comment.
This can use better recovery - you can report a non-fatal error and then return the same successful result as self.check_keyword(keywords::Static) does.
You'll have to move self.expect_keyword(keywords::Static) from parse_item_foreign_static though.
There was a problem hiding this comment.
That would be better, but I just wanted to improve the error message. I don't feel the need to rework the error handling here.
There was a problem hiding this comment.
You could also use span_suggestion instead of help to replace the const with static
|
I think you should also add an ui test showing the new error message in action. Then again, ths change seems simple enough. |
arielb1
added
the
S-waiting-on-author
|
I don't know if I need to change any tests, or how. I've ran
it melted my CPU for 15 minutes and showed "running 0 tests", so I'm not sure if that worked. I've tried This is a too minor thing, and Rust compilation takes way too long for me to touch this any further. |
|
@pornel you'll have to add a new test to an appropriate directory under |
|
@estebank thank you for the instructions. The ui tests pass, even though I haven't changed them. I guess the help text is ignored in tests? |
|
@pornel none of the existing tests is hitting your new code. You should create a new test that will trigger the new output. |
|
@pornel |
|
@petrochenkov Thank you for improving this. |
src/libsyntax/parse/parser.rs
Outdated
| if self.token.is_keyword(keywords::Const) { | ||
| self.diagnostic() | ||
| .struct_span_err(self.span, "extern items cannot be `const`") | ||
| .span_label(self.span, "use `static` instead").emit(); |
There was a problem hiding this comment.
This could be .span_suggestion(self.span, "instead try using", "static".to_owned()), which would allow IDEs and tools to automatically replace it.
|
@bors r+ |
|
📌 Commit cabc9be has been approved by |
Hint correct extern constant syntax
Error message for `extern "C" { const …}` is terse, and the right syntax is hard to guess given unfortunate difference between meaning of `static` in C and Rust.
I've added a hint for the right syntax.
|
☀️ Test successful - status-appveyor, status-travis |
Error message for
extern "C" { const …}is terse, and the right syntax is hard to guess given unfortunate difference between meaning ofstaticin C and Rust.I've added a hint for the right syntax.