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

#[bench] attribute is stable and test crate can be replaced #38998

Closed
brson opened this issue Jan 11, 2017 · 7 comments
Closed

#[bench] attribute is stable and test crate can be replaced #38998

brson opened this issue Jan 11, 2017 · 7 comments
Labels
A-libtest Area: `#[test]` / the `test` library C-bug Category: This is a bug. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@brson
Copy link
Contributor

brson commented Jan 11, 2017

Per @SimonSapin's comment. It is possible to swap out the test crate via cargo and get the #[bench] functionality on stable.

This has been abused in the wild for a long time, and is a fairly major blunder. Swapping out the test crate exposes a large swath of ad-hoc test API's to the stable ecosystem (rustc generates calls to these APIs).

Fixing this will require providing a plausible alternative and a long deprecation and migration period.

@brson brson added I-wrong T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. labels Jan 11, 2017
@brson brson changed the title #[bench] is stable and test crate can be replaced #[bench] attribute is stable and test crate can be replaced Jan 11, 2017
@SimonSapin
Copy link
Contributor

As I said in that comment, I found zero evidence that anyone else used benchmarks this way or even knew it was possible. None of rustc-test’s reverse dependencies use #[bench]. (They use the API to dynamically generate tests.) I myself only did it in an empty temporary crate to check if it was possible. Do a crater run to be more confident, but I believe nothing will be affected if the #[bench] attribute is made unstable right now. (Gated on the test feature which is already enabled by crates that use it on Nightly without the dependency shadowing trick.)

As to exposing test API’s, that’s only API of the rustc-test crate (or other similar crate) which is a copy, separate from Rust’s original src/libtest. The former won’t be affected automatically if changes are are made to the latter. And if porting a breaking change is desired, it can be made in a SemVer-breaking version number, as usual for anything on crates.io.

@Mark-Simulacrum Mark-Simulacrum added A-libtest Area: `#[test]` / the `test` library C-bug Category: This is a bug. and removed I-wrong labels Jul 26, 2017
@DrRibosome
Copy link

Hmm, ive been using this to get bench on stable for a bit now (but I dont have crates pushed to crates.io). It would be annoying for sure if this broke given that benching is useful, but I want to work on stable

@SimonSapin
Copy link
Contributor

Since my previous comment, std made a breaking change to the unstable private data structures that rustc-test relies on: servo/rustc-test#4. rustc-test working on "stable" Rust is an illusion: it’s really using a loophole in the stability system to access unstable features.

@ehuss
Copy link
Contributor

ehuss commented Feb 26, 2020

#[bench] has been transitioned to a "soft unstable" lint, which will eventually become a hard error. Closing in favor of the tracking issue #64266. Feel free to reopen if I'm misunderstanding the intent here.

@ehuss ehuss closed this as completed Feb 26, 2020
@rbtcollins
Copy link
Contributor

I think this should be reopened, or something...

https://doc.rust-lang.org/cargo/commands/cargo-bench.html documents the #[bench] attribute - I quote

Benchmarks are built with the --test option to rustc which creates an executable with a main function that automatically runs all functions annotated with the #[bench] attribute. Cargo passes the --bench flag to the test harness to tell it to run only benchmarks.

No mention of stability is made there.

Either the docs should not specify using #[bench], or they should specify the stability requirements, or the bug here is that the crate can be replaced, not that bench was available - since it was documented as being available.

@ehuss
Copy link
Contributor

ehuss commented May 8, 2020

@rbtcollins Posted an update to the docs here: rust-lang/cargo#8227

bors added a commit to rust-lang/cargo that referenced this issue May 8, 2020
@rbtcollins
Copy link
Contributor

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-libtest Area: `#[test]` / the `test` library C-bug Category: This is a bug. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

6 participants