-
Notifications
You must be signed in to change notification settings - Fork 13.3k
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
compiletest: Make diagnostic kind mandatory on line annotations (take 2) #139720
base: master
Are you sure you want to change the base?
Conversation
MCP: rust-lang/compiler-team#862, waiting on compiler team. |
Most of the diff is due to updated |
Some changes occurred in src/tools/compiletest cc @jieyouxu |
The job Click to see the possible cause of the failure (guessed by this bot)
|
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.
const ui tets changes LGTM, except that the PR also removes some colons which caught me by surprise as it wasn't mentioned... but if we will unify the colon vs no-colon style soon anyway, it doesn't really matter.
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.
There you also changed the annotation style for no apparent reason?
#![allow(static_mut_refs)] | ||
|
||
fn invalid() { | ||
static S: i8 = 10; | ||
|
||
const C: &bool = unsafe { std::mem::transmute(&S) }; | ||
//~^ERROR: undefined behavior |
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.
More removed colons.
This is accidental, this PR was converted from #139427 which had some colon changes, and I tried to revert all of them, but apparently missed some. If some more are found during review I'll revert them too. |
☔ The latest upstream changes (presumably #139746) made this pull request unmergeable. Please resolve the merge conflicts. |
would be nice if this also updated rustc-dev-guide |
I'll update, I was waiting for #139618 to land first. |
Compiletest currently accepts line annotations without kind in UI tests.
Such annotations have two effects.
//~ NOTE my msg
in a test requires all otherNOTE
diagnostics in the same test to be annotated. Implicit//~ my msg
will just match the note and won't require other annotations.The second functionality has a replacement since recently - directive
//@ dont-require-annotations: NOTE
.This PR removes support for
//~ my message
and makes the explicit diagnostic kind mandatory.Unwanted additional annotations are suppressed using the
dont-require-annotations
directive.Previous attempt - #139427.
r? @jieyouxu