[1.38] #[bench] accepted on stable but not beta

[CAD97]

Initially reported by @e-oz here.

[playground]

fn main() {
    println!("Hello, world!");
}

#[bench]
fn example() {}

On stable, you're allowed to define #[bench] tests in your library, even if calling them requires nightly. This broke at least one stable user, so we should at least consider continuing to ignore #[bench]-annotated functions when not run via cargo bench.

Is there some way to get a crater run to determine which libraries are using #[bench] in an otherwise stable crate, so we can suggest that they move benchmarks into a dedicated folder?

[djrenren]

I think it's related to this: #62048

Unsure how it got marked stable in the first place.

[ehuss]

[bench] was unstabilized in #62507. A crater run was done during that PR, the conclusion was written here. I think it was deemed unusual enough to proceed.
cc @petrochenkov

[petrochenkov]

Reverting the unstabilization and making a beta backport is always an option, it's a one line change.

Complaints filed so far:
#62507 (comment) - route-recognizer broke, apparently it's used as a dependency somewhat commonly.
#50297 (comment) and below - unspecified dependency broke, may be the case of route-recognizer as well.
iron/iron#613 - iron/router doesn't build, again cause by route-recognizer

route-recognizer was fixed in http-rs/route-recognizer#25 and a new patch version was published.
@e-oz does this fix your use case?

[Centril]

I don't think we should re-stabilize especially given that some folks have already migrated.
(The one line change is not the issue, but forever committing to a stable #[bench].)

[CAD97]

I wish we had unsurpressable future-compat warnings so a cycle's worth of just a future-compat warning for #[bench] would actually be helpful for catching any other libraries mistakenly using (because it was stable that way) #[bench] as #[cfg(test)] #[bench].

Because we don't, I assume said grace period wouldn't help find any remaining uses, just delay the errorification.

So I guess the question is: #[bench] has been (mistakenly?) stable as #[cfg(false)] for standard builds since Rust 1.0. Is that enough to force us into supporting it for nominal stability's sake, or do we consider it a long-standing low priority bug that just got fixed?

[pietroalbini]

Accidental stabilizations are not part of our stability guarantee. Still, if possible I think we should try to avoid breaking crates.

[Centril]

I wish we had unsurpressable future-compat warnings so a cycle's worth of just a future-compat warning for #[bench] would actually be helpful for catching any other libraries mistakenly using (because it was stable that way) #[bench] as #[cfg(test)] #[bench].

Implemented in #59658; I'll be picking this up shortly to address the concerns raised there re. spam.

[petrochenkov]

@CAD97
If I'm reading your comment correctly, it has some misunderstanding.
#[cfg(false)] #[bench] still can be used on stable because bench is unconfigured before use.

What was unstabilized is the actual use of bench.

#[bench]
fn foo() {}

It previously didn't compile in --test mode due to requiring other unstable things in the fn signature, but without --test it compiled successfully and expanded into nothing (empty token stream / AST piece).

[petrochenkov]

I'm not sure the back compat warning is a good solution since #[bench] doesn't seem to be deprecated in any way, on the contrary, it's going for stabilization with custom test frameworks making progress.

[CAD97]

@petrochenkov That's just a wording issue. I meant that using #[bench] had the effect of using #[cfg(test)] #[bench].

[e-oz]

@petrochenkov
I can't update version of route-recognizer because requirements of version are written not in mine Config.toml - it's dependency of iron/router (and updating iron version means rewriting my project from scratch, it's few months of work without any noticeable result).

[CAD97]

@e-oz The new version of route-recognizer is semver-compatible with every other published version. A cargo update should re-resolve your lock file to use the latest version of route-recognizer.

[e-oz]

@CAD97
it worked for small project, it doesn't work for the big one - cargo update updates too many libs, not all of them care about semver rules.
But thanks for advice anyway.

[fbstj]

@e-oz check out the --package option of cargo update and run something like cargo update --package route-recognizer to only update that package.

[e-oz]

@fbstj thank you very much! Works great.

[petrochenkov]

Despite the small number of root regressions, there seems to be a huge amount of indirect breakage - #63628 (comment), we cannot leave this as is.

If someone wants to implement a lint reporting #[bench] without feature(test), that would be great.
Reverting the unstabilization would be the simplest solution.

[Centril]

If we are "fixing" this (allowing #[bench] on stable), then I think a warning must also be there... otherwise we are just stabilizing the feature.

[CAD97]

At the very least, the issue that it's pointing at should have a note about the destabilization. We can fix that now to help avoid future confusion: @Centril, do you mind editing the OP of #50297 to add a clear note about such?

[petrochenkov]

Fixed in #64066.