-
Notifications
You must be signed in to change notification settings - Fork 13.9k
Emit error when using path-segment keyword as cfg pred #146978
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
base: master
Are you sure you want to change the base?
Emit error when using path-segment keyword as cfg pred #146978
Conversation
Some changes occurred in compiler/rustc_attr_parsing |
Could you
After that we should be able to run crater on this. |
Reminder, once the PR becomes ready for a review, use |
505d13f
to
b2be57d
Compare
This comment has been minimized.
This comment has been minimized.
|
@rustbot ready |
|
@petrochenkov I found rust/compiler/rustc_session/src/parse.rs Lines 318 to 329 in a2db928
and the comment of rust/compiler/rustc_errors/src/emitter.rs Lines 537 to 562 in 8155734
|
|
Ah, ok, "fatal emitter" means "fatal-only emitter". In any case, the behavior is not correct. |
This comment has been minimized.
This comment has been minimized.
956aa91
to
97cd2c7
Compare
This comment has been minimized.
This comment has been minimized.
97cd2c7
to
a7d6090
Compare
This comment has been minimized.
This comment has been minimized.
a7d6090
to
c8bc460
Compare
This comment has been minimized.
This comment has been minimized.
cc53ca4
to
9c2ed4c
Compare
@rustbot ready |
0f6589e
to
2309d40
Compare
if ident.is_reserved() { | ||
if !ident.name.can_be_raw() { | ||
if s.trim().starts_with(&format!("r#{}", ident.as_str())) { | ||
error!(format!("argument key must be an identifier, but `{}` cannot be a raw identifier", ident.name)); |
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.
So now we have to manually detect all recoverable parsing errors here, not just keywords instead of identifiers.
The set of recoverable errors is open, parser does much more recovery now than at the times of #64467, you can't catch them all manually.
If we don't want to remove the use of fatal-only emitter, then we need to set Recovery::Forbidden
in the parser, then parse_meta_item
will return Err
on any syntactically invalid stuff instead of recovering.
Then the only remaining thing to check will be is_path_segment_keyword
, like in rustc_attr_parsing
.
cc @Muscraft |
7f6d7bf
to
96e3c6b
Compare
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.
Note that this test and the change in behavior of --check-cfg
was specifically added at the request of T-lang in the stabilization of #[cfg(true)]
/#[cfg(false)]
#138632 (comment)
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 PR will need to go through lang team too after a crater run.
(Similarly to other cases in this PR, this test case was accepted because --cfg
parsing swallowed non-fatal parsing errors.)
This comment has been minimized.
This comment has been minimized.
96e3c6b
to
c41ceed
Compare
This comment has been minimized.
This comment has been minimized.
Some changes occurred in src/tools/clippy cc @rust-lang/clippy |
6c405e7
to
9171a1c
Compare
@rustbot ready |
It looks like you removed some |
9171a1c
to
c0b4b6c
Compare
Good catch! Recovered. Seems the 32bit revision is not tested on both my device and CI 🤔 |
☔ The latest upstream changes (presumably #147842) made this pull request unmergeable. Please resolve the merge conflicts. |
c0b4b6c
to
8321359
Compare
This PR was rebased onto a different master commit. Here's a range-diff highlighting what actually changed. Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers. |
The job Click to see the possible cause of the failure (guessed by this bot)
|
Fixes #146968
Emit error
CfgPredicateIdentifier
if the word is path-segment keyword.r? petrochenkov