-
Notifications
You must be signed in to change notification settings - Fork 13.6k
Verify llvm-needs-components are not empty and match the --target value #144042
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
Conversation
rustbot has assigned @albertlarsan68. Use |
r? jieyouxu |
67d476a
to
15dc6a8
Compare
This comment has been minimized.
This comment has been minimized.
15dc6a8
to
a852fa6
Compare
This comment has been minimized.
This comment has been minimized.
a852fa6
to
b3d78ec
Compare
This comment has been minimized.
This comment has been minimized.
b3d78ec
to
6a8c18b
Compare
☔ The latest upstream changes (presumably #144181) made this pull request unmergeable. Please resolve the merge conflicts. |
6a8c18b
to
598e66f
Compare
☔ The latest upstream changes (presumably #144249) made this pull request unmergeable. Please resolve the merge conflicts. |
Verify llvm-needs-components are not empty and match the --target value try-job: aarch64-msvc-1
This comment has been minimized.
This comment has been minimized.
💔 Test failed (CI). Failed jobs:
|
That directive is wrong, it's |
a5f73bf
to
9560ffd
Compare
9560ffd
to
cffde73
Compare
I'm not sure if there's an easy way in directive.rs to know if the @rustbot ready |
To be sure, |
Verify llvm-needs-components are not empty and match the --target value try-job: aarch64-msvc-1
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. The changes in this PR are mostly fine, there's a few weird cases, but I think I'll look into them in a follow-up.
@bors r+ rollup=iffy |
Rollup of 6 pull requests Successful merges: - #144042 (Verify llvm-needs-components are not empty and match the --target value) - #144268 (Add method `find_ancestor_not_from_macro` and `find_ancestor_not_from_extern_macro` to supersede `find_oldest_ancestor_in_same_ctxt`) - #144411 (Remove `hello_world` directory) - #144662 (compiletest: Move directive names back into a separate file) - #144666 (Make sure to account for the right item universal regions in borrowck) - #144668 ([test][run-make] add needs-llvm-components) r? `@ghost` `@rustbot` modify labels: rollup
Rollup merge of #144042 - dpaoliello:verifyllvmcomp, r=jieyouxu Verify llvm-needs-components are not empty and match the --target value I recently discovered a test with an empty `llvm-needs-components` entry (fixed in #143979) which meant that it didn't work correctly when building Rust with a limited set of LLVM targets. This change makes a pair of improvements to prevent this issue from creeping in again: * When parsing directives with values, `compiletest` will now raise an error if there is an empty value. * Improved the `target_specific_tests` tidy checker to map targets to LLVM components, to verify that any existing `llvm-needs-components` contains the target being used. I also fixed all the issues flagged by the improved tidy checker.
Rollup of 6 pull requests Successful merges: - rust-lang/rust#144042 (Verify llvm-needs-components are not empty and match the --target value) - rust-lang/rust#144268 (Add method `find_ancestor_not_from_macro` and `find_ancestor_not_from_extern_macro` to supersede `find_oldest_ancestor_in_same_ctxt`) - rust-lang/rust#144411 (Remove `hello_world` directory) - rust-lang/rust#144662 (compiletest: Move directive names back into a separate file) - rust-lang/rust#144666 (Make sure to account for the right item universal regions in borrowck) - rust-lang/rust#144668 ([test][run-make] add needs-llvm-components) r? `@ghost` `@rustbot` modify labels: rollup
I recently discovered a test with an empty
llvm-needs-components
entry (fixed in #143979) which meant that it didn't work correctly when building Rust with a limited set of LLVM targets.This change makes a pair of improvements to prevent this issue from creeping in again:
compiletest
will now raise an error if there is an empty value.target_specific_tests
tidy checker to map targets to LLVM components, to verify that any existingllvm-needs-components
contains the target being used.I also fixed all the issues flagged by the improved tidy checker.