rust-lang / rust

Empowering everyone to build reliable and efficient software.
https://www.rust-lang.org
Other
96.98k stars 12.54k forks source link

(Modules) Tracking issue for `(use) crate_name::` paths without `extern crate` #53128

Closed Centril closed 5 years ago

Centril commented 6 years ago

This is a sub-tracking issue for the RFC "Clarify and streamline paths and visibility" (rust-lang/rfcs#2126) dealing with the deprecation of extern crate as well as having use crate_name::foo::bar and crate_name::foo::bar Just Work™.

Unresolved questions:

None

joshtriplett commented 6 years ago

I feel that the deprecation of extern crate and the automatic inclusion of external crates in the prelude has gotten enough exposure and positive feedback that it's ready for us to commit to as the path we're going down. We shouldn't stabilize it until we stabilize the macro system changes, but we can handle that via a blocking concern; in the meantime, let's confirm that we have consensus.

Note that while we're still talking about the final system for paths in use statements, both of the proposals currently under consideration incorporate this deprecation of extern crate and automatic inclusion of external crates in the prelude.

@rfcbot fcp merge

rfcbot commented 6 years ago

Team member @joshtriplett has proposed to merge this. The next step is review by the rest of the tagged teams:

Concerns:

Once a majority of reviewers approve (and none object), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

joshtriplett commented 6 years ago

@rfcbot concern macros

We shouldn't stabilize this until we first stabilize the macro system changes needed to deprecate #[macro_use] extern crate as well.

matklad commented 6 years ago

Not really a concern, but some tradeoffs about automatic inclusion of external crates in the prelude, mostly from internals thread.

In Rust 2015, there are two ways to refer to extern crate foo:

Extern crates in prelude adds a third one:

This third variant is easier to type, but it gives the least amount of information to the reader.

It is also seems to be the case that adding extern crates to the prelude is not required to make other module systems changes "work". Specifically, ergonimics is not regressed compared to Rust 2015: ::foo::Bar style paths continue to work. use foo; foo::Bar pairs continue to work, extern crate foo; foo::Bar pair changes to use foo; foo::Bar. The only exception here is std in the root module, whose extern crate std is added implicitly. This is mitigated by the fact that popular types of std are in prelude and that std does not contain top-level items, so submodules are often usually imported anyway.

It is true that some use statements might be dropped with this feature in 2018.

The original motivation for this feature was to fix path confusion: the fact that paths in use statements and everywhere else work differently. This is only a partial fix though:

TL;DR

extern crates in prelude

sanmai-NL commented 6 years ago

@matklad: your critique ultimately hinges on the outcome of #53130, no? AFAICT, the ambiguity does not appear if we choose anchored_use_paths there.

matklad commented 6 years ago

53130 is about paths in use statements. The addition of crates to prelude affects paths outside of use statements. I.e., even with anchored_use_path (which is what currently is default with edition 2018 I believe), fn main() { regex::Regex::new(".*").unwrap().expect() } works.

sanmai-NL commented 6 years ago

Ah, right. I am only commenting as production user of Rust and haven’t paid close attention to the whole discussion/RFCs so please bear with me. :slightly_smiling_face: Why wouldn’t an obvious solution that springs me to mind, a uniform path syntax throughout (use statements and elsewhere), i.e. something along the lines of anchored_paths, work then?

matklad commented 6 years ago

a uniform path syntax throughout

The devil is in the details, and the most devilish detail here is which syntax to pick to refer to external crates. The two options are

sanmai-NL commented 6 years ago

I second that the second solution is obviously right, vis-à-vis. It’s a pity some programmers dislike verbosity so much they would rather introduce ambiguity...

But there’s a discrepancy still, given anchored_use_paths, in use statements referring to items in external crates, paths ~wouldn’t contain a leading :: whereas they do in paths elsewhere~. Just for consistency (and learnability) I prefer to have one single path syntax. Update: as you wrote, they currently may have the leading :: but it isn’t required.

Update The general idea I’m trying to sketch:

I’m sure this idea isn’t specified carefully enough yet, but I hope we can have fruitful discussion at least.

cramertj commented 6 years ago

@rfcbot concern no_std-std

I just opened https://github.com/rust-lang/rust/issues/53166. It's not a huge deal (it's a fairly odd corner case), but I'd like to get some amount of resolution on it before starting FCP.

SimonSapin commented 6 years ago

Would for example use proc_macro::TokenStream; "just work" without extern crate proc_macro;? (For sysroot crates not listed explicitly with --extern.)

petrochenkov commented 6 years ago

@SimonSapin This depends on the choice made with regards to https://github.com/rust-lang/rust/issues/53130.

eddyb commented 6 years ago

@SimonSapin @petrochenkov So there's one more concern, that @aturon and @joshtriplett raised when discussing #52923 (that I'm not sure is written down anywhere else), around the fact that ::test (the crate) and self::test (a local module) would both work if we keep the behavior you talk about, but "uniform paths" would have to choose one of:

Alternatively, we can make ::crate_name not work for any crate not in the extern_prelude, and still require extern crate / --extern for those crates until a better mechanism (preferably involving Cargo.toml) is introduced.

joshtriplett commented 6 years ago

@eddyb For the record, for the moment I'd favor the solution of having the ::crate_name syntax not work for crates not named in Cargo.toml / provided via --extern. The goal of removing extern crate is to avoid duplication and having to list the external crate twice, but it needs listing at least once.

SimonSapin commented 6 years ago

I haven’t followed the details of the different path proposals, I mostly wanted to remind that there can be more crate than Cargo dependencies and std. If such crates still need extern crate to be used, then extern crate still need to be taught to new users.

aturon commented 6 years ago

@SimonSapin to clarify, @joshtriplett is talking specifically about "sysroot" crates (like test) -- he mentioned that passing in via --extern is fine. Today these crates are available without saying anything to the compiler or Cargo, and there's an opportunity to clean up this behavior a bit.

SimonSapin commented 6 years ago

@aturon Are you suggesting that all sysroot crates would always be specified through --extern? If so it seems concerning that mentioning a ::proc_macro::Something path anywhere in the crate is all it takes to start linking against the compiler.

joshtriplett commented 6 years ago

@SimonSapin

Are you suggesting that all sysroot crates would always be specified through --extern?

Not automatically; they'd need to be specified explicitly in order to use them, via some "allow the use of this sysroot crate" option (and equivalent in Cargo.toml).

If so it seems concerning that mentioning a ::proc_macro::Something path anywhere in the crate is all it takes to start linking against the compiler.

That's exactly the concern, and we want to avoid that.

In the short term, we'll probably just continue requiring extern crate proc_macro;. In the long term, we'd like to have some kind of --extern-builtin proc_macro, and a corresponding Cargo.toml dependency line like proc_macro = "builtin".

Nemo157 commented 6 years ago

I just encountered another case where you still require some explicit inclusion of a dependency with this new system: crates which have a public API of only a lang item like panic_implementation, e.g. panic-abort. Without any reference to the dependency it is not linked into the final binary so no implementation is found.

cramertj commented 6 years ago

@Nemo157 yup, that's not wonderful, although I can sort of justify asking for crate linkage via extern crate because it feels like a special feature.

SimonSapin commented 6 years ago

Sounds like another case that means we can’t or shouldn’t actually deprecate extern crate, which seems to be the premise of this issue.

joshtriplett commented 6 years ago

@SimonSapin The premise of this issue is not just needing it in the common case. For the several reasons observed so far, I doubt we'll be able to fully deprecate it for a while.

SimonSapin commented 6 years ago

That is not what the current title and description of the issue suggest.

alexreg commented 6 years ago

What about just use crate_name; to replace the case of extern crate crate_name; (with no other references). If there are references to things within the crate from the dependent crate, then use crate_name; should be linted as superfluous. That would work for me. It offers a more consistent syntax / interface to importing at least.

Nemo157 commented 6 years ago

One thought is whether lang items such as panic_implementation should be imported into binary crates. Then you could have use panic_abort::panic_implementation; to make it obvious why it’s being imported. I’m not sure how that would work with stds implementations though, could it be added to the implicit prelude in binary crates only?

SimonSapin commented 6 years ago

#[global_allocator] is stable since 1.28 and does not need to be imported that way.

eddyb commented 6 years ago

I'd suggest use crate_name as _; to replace the link-only case. This matches use foo::Trait as _; to access the methods without naming the trait.

alexreg commented 6 years ago

@eddyb Ah, that's a slightly upgrade on my suggestion, I agree!

solb commented 6 years ago

Clarification question: how will this change the behavior for those of us who sometimes invoke rustc directly (or via Make) instead of using Cargo? Currently, if I have a compiled Rust library libfoo.rlib or libfoo.so in my library search path, I can ask it to be linked in simply by typing extern crate foo;. I find the symmetry with the foo.rs/mod foo; case beautiful, and I hope we're not sacrificing it with the crate/module changes?

joshtriplett commented 6 years ago

On August 16, 2018 2:45:40 PM PDT, Sol Boucher notifications@github.com wrote:

Clarification question: how will this change the behavior for those of us who sometimes invoke rustc directly (or via Make) instead of using Cargo? Currently, if I have a compiled Rust library libfoo.rlib or libfoo.so in my library search path, I can ask it to be linked in simply by typing extern crate foo;. I find the symmetry with the foo.rs/use foo; case beautiful, and I hope we're not sacrificing it with the crate/module changes?

If you pass a crate with --extern then it'll show up in the extern prelude and you can just use it.

If you don't pass --extern, it won't show up in the extern prelude, and this new syntax won't work for it. @aturon and I have been talking about ways to improve that case, but that isn't what this issue addresses.

eddyb commented 6 years ago

Note that for now, implementation-wise, paths that start with :: (under uniform_paths, at least) will load the crate from the sysroot or a library search path, regardless of --extern being used or not. But if you can, I'd highly suggest replacing library search paths with --extern.

Nemo157 commented 6 years ago

I'd suggest use crate_name as _; to replace the link-only case.

I like this syntax, two issues when trying to use it for panic-abort:

  1. It's still an unstable feature (not a big problem, but if extern crate is deprecated and this is the official recommendation it'll need stabilising first).

  2. It gives an unused import error: warning: unused import: `panic_abort as _`, that's fixed for extern crate panic_abort as _, but I'm not sure how that could work for use, maybe it can detect when the path refers to the root of a package and assume it's always used, while continuing to warn for internal modules?

eddyb commented 6 years ago

It's still an unstable feature

I'd say we should include it in Rust 2018 on nightly, right away. Any objections, @rust-lang/lang?

It gives an unused import error:

That's still a bug, things with a (starting with) _ name should not warn about being unused.

eddyb commented 6 years ago

Then again, ugh, we might want to warn on use foo::Trait as _;? Has this been discussed before, @petrochenkov?

petrochenkov commented 6 years ago

Right now warnings are issued for use path as _;/extern crate as _; exactly like for use path as name;/extern crate as name;. If the import is not used, then unused_imports is reported. If the extern crate is not used, then unused_extern_crates is reported.

If extern crate or import is used in some undetectable way (e.g. for linking only), then, IMO, it should have explicit #[allow(unused_extern_crates)] on it and a comment explaining why this is necessary at all.

There was some discussion about it in https://github.com/rust-lang/rust/pull/42588 and unused_extern_crates was made warn-by-default as a result, requiring explicit annotations for link-only extern crates. It was reverted later due to another issue (https://github.com/rust-lang/rust/pull/44825) though.

petrochenkov commented 6 years ago

https://github.com/rust-lang/rust/pull/53479 still looks reasonable though, since there's no other way to use a crate gensymed with _ except for linking. This logic can be transferred on use crate_name as _;, I think. as _ is as good explicit annotation as #[allow(unused_extern_crates)] (but having comment explaining why this import is necessary would still be nice).

joshtriplett commented 6 years ago

@eddyb I had the same thought yesterday, which is why I worked on https://github.com/rust-lang/rust/pull/53479 . I do think we should stabilize underscore_imports as part of Rust 2018, since it fits naturally as a complimentary part of the module system updates. We do need the remaining issue fixed, though, namely that use foo as _; generates an unused import warning.

aturon commented 6 years ago

@joshtriplett

We shouldn't stabilize this until we first stabilize the macro system changes needed to deprecate #[macro_use] extern crate as well.

To be clear, when you say "this" do you mean specifically the deprecation? The title and description on this tracking issue are at odds over exactly what's covered. I agree re: deprecation (at least for the macro case) but feel we need to stabilize the path part of this with Rust 2018.

joshtriplett commented 6 years ago

@aturon Yes, exactly. We need to avoid having deprecation warnings for extern crate if you still need #[macro_use] extern crate for macros. But I think the changes to support use for all macros are on track at this point.

alexreg commented 6 years ago

@joshtriplett But #[macro_use] extern crate is only needed in Rust 2015, right?

mark-i-m commented 6 years ago

Huh? I was under the impression that you still need macro_use in 2018 (for now).

alexreg commented 6 years ago

@mark-i-m No. You can employ use syntax to import macros from other crates, even macro_rules! ones. It's only if you want to use macro_rules! internally that you need to employ macro_use. But the recommended way forward is to use Macros 2.0 macro declarations. That's how I understood it at least... someone correct me if I'm horribly mistaken!

mark-i-m commented 6 years ago

Woah, you are right! That's a major ergonmics improvement IMHO :)

EDIT: ugh auto incorrect

rpjohnst commented 6 years ago

Going back to @matklad's comment:

The only exception here is std in the root module, whose extern crate std is added implicitly. This is mitigated by the fact that popular types of std are in prelude and that std does not contain top-level items, so submodules are often usually imported anyway.

This is actually, IIRC, if not the main reason then definitely the first reason for adding extern crates to the prelude. Path confusion is fixed in principle without that, but with std being as common as it is these mitigations aren't enough to fix it for beginners IMO.

So the two choices in my mind are a) remove extern crate std from the crate root to make it match other dependencies, which is a pain because it increases the frequency of use std;, or b) add all dependencies including std to the prelude, so they all feel the way std does in the 2015 crate root. Leaving it as is leaves path confusion un-fixed.

::foo::bar always refers to an extern crate, and you have to use that leading :: in use statements as well (currently, it is optional in use, and that leads to two-flavors of paths and the need for self::). To me personally this looks like the obviously right solution, but turns out a lot of folks really don't like the leading ::.

Whenever this is brought up, I can't help but think back to some of the original module system threads, where two major themes were a) use paths to other crates and across the module hierarchy are super common, so absolute paths are an important default there, and b) every attempt to add more sigils to paths failed, despite the benefits they brought (and there were a lot of attempts!).

I also think the ambiguity here is overplayed. C# and Java, both notoriously verbose, have this property and it's never been an issue there. You can generally tell from context, for two reasons: top-level crate names are distinct enough and well-known enough not to be confused with local items, and paths that use them tend to be longer than paths referring to local items.

Centril commented 6 years ago

(There's not a ticky box here for me, but consider it ticked anyways...)

sanmai-NL commented 6 years ago

@rpjohnst:

which is a pain because it increases the frequency of use std;

I miss a motivation for this qualification.

joshtriplett commented 6 years ago

@rfcbot resolved macros

The features needed for this are on track.

aturon commented 6 years ago

re: the no-std concern, we discussed this in the lang team meeting today and I've posted a summary comment here.

cramertj commented 6 years ago

@rfcbot resolve no_std-std

I'm happy with the proposed solution of a deny-by-default warning when #![no_std] is used, which crate authors can allow based on their own optional std feature.

rfcbot commented 6 years ago

:bell: This is now entering its final comment period, as per the review above. :bell: