2018 edition module/path changes don't work for libtest benchmarks

[dekellum]

In the project branch with current head of dekellum/body-image@15b9789, I'm attempting to preview the 2018 edition changes and test cargo fix. Besides my own education the process has uncovered a few issues in my project where I can actually back-port fixes that are also relevant today with rust 1.27.2. Thanks for the 2018 improvements and cargo fix!

I also found one limitation of the module/path changes that seems worth mentioning hear in case its an oversight, and because I can't find any other issue addressing it. It seems as of the rust nightlies I'm testing I still, oddly, need to use extern crate test in the benches/*.

Given starting benches/*.rs benchmarks with the following:

#![feature(test)]
extern crate test;

As of rustc 1.30.0-nightly (\5c875d938 2018-09-24), this produces a warning like in other places, as expected:

warning: `extern crate` is not idiomatic in the new edition                                                                             
--> benches/barc.rs:6:1                                                                                                                
  |                                                                                                                                     
6 | extern crate test;                                                    
  | ^^^^^^^^^^^^^^^^^^ help: convert it to a `use`                                                                                      
  |                                                                                                                                     

However, as of rustc 1.31.0-nightly (\4699283c5 2018-10-13), the warning goes away?

Next, in both of these nightly releases, if I make the expected modification dropping extern crate test and use test::Bencher (see the change of dekellum/body-image@15b9789), I get the following error:

error[E0432]: unresolved import `test`                                                                                                  
  --> benches/async_streams.rs:12:5                                                                                                     
   |                                                                                                                                    
12 | use test::Bencher;                                                                                                                 
   |     ^^^^ Could not find `test` in `{{root}}`                                                                                       

Note finally that I was able to remove all the other extern crate statements for the benches/* dependencies.

I suspect the non-stable, nightly-only nature of libtest benchmarks somehow has contributed to this, but many projects include benches/*. Should this be fixed or at least noted as a shortcoming somewhere?

meta

rustc 1.31.0-nightly (4699283c5 2018-10-13)
binary: rustc
commit-hash: 4699283c5b549d1559c198123a67fef498aa6a44
commit-date: 2018-10-13
host: x86_64-unknown-linux-gnu
release: 1.31.0-nightly
LLVM version: 8.0

[hellow554]

This is still existent in 1.32.0-nightly!

[Mark-Simulacrum]

Cc @petrochenkov @rust-lang/compiler

[eddyb]

I believe the intent was that you'd still need to use extern crate in that case, and that's why the warning went away in the first place.

[petrochenkov]

Yes, everything looks as expected. test is not passed with --extern, so extern crate test; has to be used.

[dekellum]

"Expected", from the compiler's perspective, with some knowledge of how the test feature and test (pseudo-)crate for benchmarks is specifically handled? Benchmarks are commonly in use, and the 2018 guide gives the impression that extern crate can be removed:

https://rust-lang-nursery.github.io/edition-guide/rust-2018/module-system/path-clarity.html

Here's a brief summary:

• extern crate is no longer needed

Idiom warnings suggest removing it in all other cases. At the very least, shouldn't this make it into a note in the edition guide? Not a pretty exception to the rule, but I don't think I'm the last user that will not expect this special case.

[Mark-Simulacrum]

Idiom lints are not yet properly supported, and the test crate is unstable. I do think it might be useful to provide a better diagnostic here in the long-run, but for most users I don't expect this to be a big problem for users since we don't automatically remove extern crate test I believe, at least not without enabling idiom lints explicitly. Idiom lints are well-documented as not being ready for 'production' use.

[dekellum]

In rust-lang-nursery/edition-guide#121, the hint was offered that this discrepancy (extern crate still required) applies to all sysroot crates, like proc_macro, not just test. With this hint I found tracking issue #53128, which discusses sysroot crates, with differing options for handling these, but I can't find any definitive resolution or plan for it?

[cramertj]

I can't find any definitive resolution or plan for it?

The plan for now is to continue using extern crate, but add an --extern name_of_crate flag (w/ no =) that would say "pull this crate from the sysroot and add it to the extern prelude". Cargo and other build systems could use this to allow crates to opt-in to receiving sysroot crates.

[eddyb]

@cramertj --extern crate_name has been added whenever the changes to the extern prelude were made (you can check, it's in nightly). I did that so Cargo could add an unstable (i.e. behind a Cargo feature-gate) equivalent, but it hasn't happened yet, not sure how to proceed.