Skip to content
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

url 0.5.10 and 1.5.1 beta regression #43684

Closed
est31 opened this issue Aug 5, 2017 · 5 comments
Closed

url 0.5.10 and 1.5.1 beta regression #43684

est31 opened this issue Aug 5, 2017 · 5 comments
Labels
C-bug Category: This is a bug. regression-from-stable-to-beta Performance or correctness regression from stable to beta. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@est31
Copy link
Member

est31 commented Aug 5, 2017

url versions 0.5.10 and 1.5.1 regressed from stable to beta (full log 1, full log 2):

Aug 04 02:42:21.790 INFO kablam! error[E0560]: struct `__test::test::TestDesc` has no field named `allow_fail`
Aug 04 02:42:21.790 INFO kablam!   --> tests/unit.rs:20:1
Aug 04 02:42:21.790 INFO kablam!    |
Aug 04 02:42:21.790 INFO kablam! 20 | / fn size() {
Aug 04 02:42:21.790 INFO kablam! 21 | |     use std::mem::size_of;
Aug 04 02:42:21.790 INFO kablam! 22 | |     assert_eq!(size_of::<Url>(), size_of::<Option<Url>>());
Aug 04 02:42:21.790 INFO kablam! 23 | | }
Aug 04 02:42:21.790 INFO kablam!    | |_^ `__test::test::TestDesc` does not have this field
Aug 04 02:42:21.790 INFO kablam! 

cc @seanmonstar @SimonSapin @Hoverbear

@arielb1 arielb1 added I-nominated regression-from-stable-to-beta Performance or correctness regression from stable to beta. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Aug 5, 2017
@arielb1
Copy link
Contributor

arielb1 commented Aug 5, 2017

wtf? need to investigate

@SimonSapin
Copy link
Contributor

TL;DR: this regression itself is not a bug in Rust. If something here is arguably a bug, it’s the loophole that allowed me to indirectly depend on unstable APIs on the stable channel.

This is servo/rust-url#371. It was fixed in servo/rustc-test#4 and servo/rust-url#372, we haven’t published a new version on crates.io since. It only affects compiling tests, not the library itself.

https://github.com/servo/rustc-test 0.1 is a copy of an old version of Rust’s src/libtest with #[unstable] attributes removed and a some other small changes to make it run on stable. When declaring the former as Cargo dependency, it ends up being used instead of the latter for extern crate test;.

#42219 added a new field to test::TestDesc, so rustc-test 0.2 conditionally compiles to include that field or not based on the rustc version and adds a constructor that works in both cases. This 0.2 change was needed because not all the relevant code is in the test crate. src/libsyntax/test.rs also generates a static &[TestDescAndFn] when rustc --test is used.

We’ve discussed all this before in https://internals.rust-lang.org/t/test-and-external-test-harnesses/3145. I now think that the future of “proper” external test harnesses involves procedural macros 2.0, but I’ve been waiting for them to be closer to stabilization to open that discussion.

@arielb1
Copy link
Contributor

arielb1 commented Aug 5, 2017

Is this also the cause of #43683 (in unic-idna-mapping)?

@SimonSapin
Copy link
Contributor

Yes.

@Mark-Simulacrum Mark-Simulacrum added the C-bug Category: This is a bug. label Aug 10, 2017
@arielb1
Copy link
Contributor

arielb1 commented Aug 10, 2017

The issue looks fixed in rust-url. Closing.

@arielb1 arielb1 closed this as completed Aug 10, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: This is a bug. regression-from-stable-to-beta Performance or correctness regression from stable to beta. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

4 participants