-
Notifications
You must be signed in to change notification settings - Fork 13k
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
libsyntax: forbid visibility modifiers for enum variants #28440
Conversation
r? @huonw (rust_highfive has picked a reviewer for you, use r? to override) |
goose | ||
} | ||
enum bird { | ||
pub duck, //~ ERROR: expected identifier, found keyword `pub` |
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 test will not pass because of #28439 I think.
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.
How can I run this this test fater than make -j8 check
?
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.
I’d suggest waiting for travis build to finish for now.
Otherwise the fastest way without editing makefiles is to do make -j8 check-stage2-cfail TESTNAME=issue-28433
I think.
cc @alexcrichton not sure if we'd want to warn for a bit before adjusting the parser. |
I've started a crater build to help evaluate the impact here. I personally be fine just turning this into a hard error immediately and sending PRs to any affected crates to update the syntax. cc @rust-lang/lang, a semi-language-change but mostly just a bugfix |
Followup on #28440 Do not merge before the referenced PR is merged. I will fix the PR once that is merged (or close if it is not)
fixes #28433
This just removes visibility parsing from the parser. No custom error message is provided: I guess default "expected identifier, found keyword
pub
" is already good.So the
ast::Variant_
vis
is alwaysInherited
. I guess it would be possible to get rid of this field now, but I'm not sure that it is in scope of this PR.The tests are changed as follows: