-
Notifications
You must be signed in to change notification settings - Fork 14.1k
compiletest: Prepare ignore/only conditions once in advance, without a macro #149470
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: main
Are you sure you want to change the base?
Conversation
The binary crate is a tiny stub; all of the logic and tests should be in the library crate. Disabling unit tests for the binary crate makes `x test compiletest` less noisy.
|
Some changes occurred in src/tools/compiletest cc @jieyouxu
|
This commit also renames the confusing `NotADirective` outcome, and fuses it with `External` into a single `NotHandledHere` outcome.
fc681fa to
b08a5fd
Compare
|
In the future, we should probably move towards a system where all supported ignore/only conditions are listed explicitly, and only use the |
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.
Thanks, this looks good, just one comment nit.
|
@rustbot author |
b08a5fd to
b03a655
Compare
|
Added a note about condition name overlap. @rustbot ready |
|
Thanks, you can r=me once PR CI is green. |
Compiletest has historically handled
ignore-*andonly-*directives in an extremely confusing way that makes the code hard to understand and hard to modify.This PR therefore takes an important step away from that older design by instead evaluating a set of named boolean "conditions" in advance, and then using those conditions to help determine whether a particular directive should cause its test to be ignored or not.
As usual, there's more cleanup that I want to do here, but I've left most of it for future work to help keep this PR manageable.
r? jieyouxu