Move some tests out of tests/ui #140551
Move some tests out of tests/ui #140551
Conversation
rustbot
added
A-tidy
| // Regression test for issue #13560, the test itself is all in the dependent | ||
| // libraries. The fail which previously failed to compile is `no_link_crate`. |
There was a problem hiding this comment.
Suggestion:
| // Regression test for issue #13560, the test itself is all in the dependent | |
| // libraries. The fail which previously failed to compile is `no_link_crate`. | |
| // Regression test for #13560. Previously, it was possible to trigger an assert in crate numbering if a series of crates being loaded included a "syntax-only" extern crate. |
Discussion: is this regression test actually testing anything now? This cnum adjustment fix was introduced in #13565 to fix #13560, but looking at the current impl, I'm not observing any special enum adjustments related to "syntax-only" crates. #[no_link] AFAICT corresponds to CrateDepKind::MacrosOnly in crate loading, and even there I don't observe this adjustment.
cc @bjorn3 in case you have advice on this test. I can't say I understand what #[no_link]-the-builtin-attribute is supposed to do, or what its purpose is in this test. The original test involved
#![crate_type = "rlib"]
#![feature(phase)]
#[phase(syntax)] extern crate t1 = "issue-13560-1";
#[phase(syntax, link)] extern crate t2 = "issue-13560-2";and based on vibes (#![feature(phase)] predates zulip, and there are practically no discussions on this feature), I suspect the current form of the test does not reflect the OG version.
There was a problem hiding this comment.
Discussion: honestly, I have no idea what this is actually testing.
- The original version was
src/test/compile-fail/macro-crate-unknown-crate.rs(introduced in RFC: Externally loadable syntax extensions #11151). - 60be2f5 changed
#[phase]->#![feature(plugin)]and added a#[no_link]. - Changed to current test name in macros: Future proof
#[no_link]#37247.
The original test intention was to check that we produce an error if the declared extern crate we can't find.
#[feature(phase)];
#[phase(syntax)]
extern mod doesnt_exist; //~ ERROR can't find crate
fn main() {}I suppose we can keep this repurposed test to check that even with #[no_link] we report an error if we can't find the crate. Can you leave a test comment to that effect?
|
@rustbot author |
rustbot
removed
the
S-waiting-on-review
|
Reminder, once the PR becomes ready for a review, use |
rustbot
added
the
S-waiting-on-author
|
renamed the xc files, will do the rest tomorrow. |
|
Pushed a separate commit for the feedback, lmk when you want me to squash @rustbot ready |
rustbot
added
S-waiting-on-review
| //! But it appears we don't mess with crate numbering for | ||
| //! `#[no_link]` crates anymore, so this test doesn't seem | ||
| //! to test anything now. |
There was a problem hiding this comment.
Remark: I think we'll leave this as an exercise to the person modifying #[no_link]-the-attribute...
There was a problem hiding this comment.
Let's not rub it in ;)
|
@rustbot author |
rustbot
added
S-waiting-on-author
|
squashed. @rustbot ready |
rustbot
added
S-waiting-on-review
|
@bors r+ rollup |
bors
added
S-waiting-on-bors
Rollup of 7 pull requests Successful merges: - rust-lang#139675 (Add the AVX10 target features) - rust-lang#140286 (Check if format argument is identifier to avoid error err-emit) - rust-lang#140456 (Fix test simd/extract-insert-dyn on s390x) - rust-lang#140551 (Move some tests out of tests/ui) - rust-lang#140588 (Adjust some ui tests re. target-dependent errors) - rust-lang#140617 (Report the `unsafe_attr_outside_unsafe` lint at the closest node) - rust-lang#140626 (allow `#[rustfmt::skip]` in combination with `#[naked]`) r? `@ghost` `@rustbot` modify labels: rollup
Rollup merge of rust-lang#140551 - mejrs:test4, r=jieyouxu Move some tests out of tests/ui r? `@jieyouxu`
r? @jieyouxu