Rust: (Modules) Tracking issue for Picking a Module Path System variant

Created on 6 Aug 2018  Â·  104Comments  Â·  Source: rust-lang/rust

This is a sub-tracking issue for the RFC "Clarify and streamline paths and visibility" (rust-lang/rfcs#2126)
as well as the internals post [Relative Paths in Rust 2018].

The issue deals with revamping paths and making a decision about whether we should pick:

  • The uniform_paths variant.
    Proposed in [Relative Paths in Rust 2018].
  • The anchored_use_paths variant.
    You have to use self::, super::, and crate::, or an external crate name explicitly....
    See rust-lang/rfcs#2126).

Unresolved questions:

  • [ ] Which variant do we pick?
B-RFC-approved C-tracking-issue T-lang disposition-merge finished-final-comment-period

Most helpful comment

EDIT: see this important update to the proposal.

We're getting close to the cut-off point for stabilizations for the Edition, and of course this feature is one of the most important ones remaining to get nailed down.

Based on the commentary in this tracking issue, the previous tracking issue, and various forum posts, I think it's fair to say that the strong majority of people who have tried any variant of 2018 modules prefer it to 2015 modules. And rustfix migration seems to be working well to limit the amount of manual churn required. So from a high level, I think we're in good shape to stabilize some variant for Rust 2018.

However, there's less agreement on which variant to prefer:

  • Anchored paths, where use statements always start with either an external crate name or a keyword. Importing from submodules requires a leading self::

  • Uniform paths, where use statements can also begin with a local item name, and hence work just like paths elsewhere

The "uniform paths" variant makes paths work more consistently, and eliminates a common stumbling block around forgetting self:: in use statements. However:

  • It also makes it a bit more difficult to tell what's being imported. It's possible that we should encourage a style in which local use statements are grouped separately, but in that case use self::{ ... } is arguably more clear, and requiring it would help enforce the style.

  • With the other changes to paths, it's possible that forgetting self:: will become less common -- we don't really know yet.

  • It's not totally clear yet how often local items and extern crate names will clash, requiring disambiguation in use statements.

  • Finally, as @rpjohnst mentions, there are a few remaining implementation concerns around uniform paths -- though I suspect we can eliminate these over the next release cycle.

Given the limited time we have, I want to propose that we take a conservative route. We would ship the anchored paths variant, but with future-proofing that would make it possible to move to uniform paths later. The future proofing is simple: if foo is both an external crate name and a local item name, then a use statement must either say use ::foo or use self::foo, just as it would in the uniform paths variant.

With this conservative approach, we can undergo the major path migration as part of Rust 2018, and reap much of the benefit. Then we can take our time to determine whether to eliminate the need for self:: after we've collectively gotten more comfortable with 2018-style paths.

The main downside, of course, is that churn is potentially split over multiple steps, rather than done all at once. However, the churn of removing no-longer-needed self:: is different from the migration churn; it's purely an idiom adjustment. We already plan to grow the set of "idiom lints" over time, and use rustfix to perform them, so this is probably not a huge issue.

In the interest of getting to a decision, I'm going to formally propose we take this conservative route and stabilize the feature:

@rfcbot fcp merge

All 104 comments

Could you give more details about what has been implemented concretely, and what renaming operations are needed exactly? It comes down to: how can I start testing anything right now?

A priori, I vote for anchored_use_paths, for being the most explicit/teachable. One suggestion: I find the crate keyword a bit odd since it does not seem related to visibility at all. I suppose this has been debated before but it would be better if it becomes pubcrate or whatever.

cc @eddyb re. @sanmai-NL's question.

@sanmai-NL Now that #52923 has landed, the next nightly should have both options available:

  • anchored_use_paths has been the Rust 2018 default for some(?) time now
  • uniform_paths you get by being on Rust 2018 and adding #![feature(uniform_paths)]

I'd suggesting trying the latter after upgrading to the edition, and report back if the ambiguity errors end up being too excessive.

AFAIK, the upgrade involves taking a Rust 2015 crate, adding #![feature(rust_2018_preview)] without changing the edition, running rustfix to apply the suggestions it gives for making same-crate paths explicit (i.e. prepending all paths that don't start with a crate name, with crate::), and only then do you switch the edition (usually in Cargo.toml) to Rust 2018.
You can remove #![feature(rust_2018_preview)] afterwards, since it's redundant.

I absolutely despise the "anchored paths variant," and I am very excited to see that we're considering an alternative. Any alternative, honestly. >.>

...That said, uniform paths look great. :) I'd much prefer to let people use absolute paths (the anchored version) if they like, but not force people to do it.

(No idea where the appropriate place to provide feedback for this would be, so I just left it here. Sorry.)

Using uniform_paths, there is an error I find confusing.
Considering:

#![feature(rust_2018_preview)]
#![feature(uniform_paths)]
use hex;
fn main() {
    println!("hex {}", hex::encode("01"));
}

I get this error:

error: import from `hex` is ambiguous
 --> src/main.rs:3:5
  |
3 | use hex;
  |     ^^^
  |     |
  |     could refer to external crate `::hex`
  |     could also refer to `self::hex`
  |
  = help: write `::hex` or `self::hex` explicitly instead
  = note: relative `use` paths enabled by `#![feature(uniform_paths)]`

whereas self::hex doesn't exist in the module.

Because the code is working without use hex maybe suggesting to remove it will be more clear.

@kohensu see https://github.com/rust-lang/rust/issues/53408, I think the plan is to either special case allow that or improve the error message to just suggest removing it.

@Nemo157
Sorry, I did not see that.
Thanks for the reference to the issue.

@archer884: What’s more interesting is your motivation to feel that way?

As a user (who perhaps hasn't been paying as much attention as I should) I think the uniform_paths option feels strictly better. Special case sugar for when there's no possible ambiguity would be _nice_, but by no means necessary; optimising for clarity is definitely a greater win.

Special case sugar for when there's no possible ambiguity would be nice

Not sure what you mean. Does #53427 (by treating each namespace independently) solve that for you?

@sanmai-nl:

Either Eric Lippert or Anders Hejlsberg had an article on this awhile back: people want loud syntax for things at first, but then as time goes on they want the syntax to be unobtrusive and elegant instead. If we make long, annoying imports the standard (and ONLY) option, we will absolutely regret it.

"Maybe not today, maybe not tomorrow, but someday." :)

That’s an argument from authority, and a very aspecific one. Conciseness is a criterion independent from explicitness and also from simplicity.

@sanmai-NL, it's not. It's just a chance for you to read the thoughts of another person who covers the issue in greater depth. Google is your friend.

Very poor way to argue your case. You seem not to even recall which author you mean, but you ask me to Google for something.

Ok, I'm done being badgered here. Glad to see rust is still so inclusive. :)

On Fri, Aug 17, 2018, 9:42 AM Sander Maijers notifications@github.com
wrote:

Very poor way to argue your case. You seem not to even recall which author
you mean, but you ask me to Google for something.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/rust-lang/rust/issues/53130#issuecomment-413887752,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AApeRmCERlVvJYp9wVS9mBYTeJaUolXMks5uRtZFgaJpZM4VxEE_
.

cc me

Sad to see how some people dramatize a technical discussion. I hope only comments that are motivated and consider the earlier discussion are taken seriously. Just chiming in doesn’t bring us any further.

@liigo You can also use the "Subscribe" button on the right, without posting a comment.

Is there a reason why use core::mem does not work?

  |
1 | use core::mem;
  |     ^^^^ Did you mean `self`?

error: aborting due to previous error

For more information about this error, try `rustc --explain E0432`.
error: Could not compile `uds`.

Aside, I see that #53427 fixed use log::* but I hit the same ambiguity with use simple_logger:

   |
14 | use simple_logger;
   |     ^^^^^^^^^^^^^
   |     |
   |     refers to external crate `::simple_logger`
   |     defines `self::simple_logger`, shadowing itself
   |
   = help: remove or write `::simple_logger` explicitly instead
   = note: relative `use` paths enabled by `#![feature(uniform_paths)]`

Aren't the modules a separate namespace?

@luben For your second comment: just delete the line use simple_logger; entirely. extern_prelude makes that redundant; you can just call simple_logger::init() or similar.

@eddyb Should the special-case version of that message reference extern_prelude instead? Because it's extern_prelude that makes use cratename; redundant.

@luben You need to use #![no_std] to get core instead of std.
Any particular usecase for accessing core without #![no_std]?

Thanks for both the responses. Makes sense. Actually I didn't need core::mem as I can use std::mem instead: I have copy/pasted some futures related code that turns to work without uniform_paths.

Any particular usecase for accessing core without #![no_std]?

@eddyb futures-rs uses the opposite (accessing std with #![no_std]) a lot for supporting an optional std feature, a good example is futures-core::stream, this has all the core supported code first, then near the bottom the allocation using code behind a #[cfg(feature = "std")] gate.

This works in 2015 by having an unconditional #![no_std] along with #[cfg(feature = "std")] extern crate std; to optionally add the crate. It also worked in 2018 anchored_use_paths with an unconditional #[no_std] and no explicit import because libstd was available in the search path (see https://github.com/rust-lang/rust/issues/53166#issuecomment-411196219).

I'm not sure how to support this as easily with uniform_paths. Adding #[cfg(feature = "std")] extern crate std; to the root requires updating all the use std::*; imports to use crate::std::*;. Using #![cfg_attr(not(feature = "std"), no_std)] requires switching all imports between use core::*; and use std::*; based on the feature. Out of these two options I think the first is definitely the most reasonable, but that requires extern crate which may be deprecated in 2018, and it's not _quite_ as nice as the old system of having normal looking core/std imports everywhere.

EDIT: While editing this post I managed to drop one of the main points. The solution I'd most be interested in seeing is exactly what @eddyb asked about above, being able to access core even without #![no_std]. That way futures-rs could use #![cfg_attr(not(feature = "std"), no_std)], when the std feature is inactive it would only import via core rooted paths, when std is active it would import via both std and core rooted paths.

You can just write use ::std::... and it still works with uniform_paths, btw.
That syntax works with ::core::... as well, too, irrespective of #![no_std].
(paths starting with :: explicitly look up a crate and are not limited to the extern_prelude filter)

@eddyb Is there any particular reason not to have core accessible even without #[no_std]?

Doing so allows more easily writing code that works with or without #[no_std], if you only use functions in core.

@JoshTriplett Not really any that I can think of (maybe modules named core causing ambiguities?).
We might want to keep only one to avoid confusion over which to use, but it also seems useful to have both.
I wonder what @rust-lang/lang and @rust-lang/libs think of allowing core, as well as std, to be used everywhere without #![no_std].

I think allowing core everywhere sounds like a good idea. In particular, it seems to me like it would make it easier to transition to #![no_std] support.

How would it affect the plans for a stable alloc crate, that RFC is based on using extern crate?

I assume a std-aware cargo would subsume this functionality, and you could explicitly list which facade crates you depend on in your Cargo.toml (hopefully with optionality based on features).

The question of core in prelude was briefly discussed recently in https://internals.rust-lang.org/t/2018-edition-end-of-week-post-2018-07-20/8019.
I see no problems with adding it.

The only restriction that, I think, we need to enforce is not adding crates that are not dependencies of std into prelude (e.g. proc_macro), and not adding crates that are not dependencies of core when no_std is enabled.
This way we can avoid special-casing these crates in the compiler and just put them into core/std library preludes.
This actually may be possible right now since https://github.com/rust-lang/rust/pull/52923 fixed some bugs in single-segment imports.

I haven't seen one more option mentioned.

--extern currently has the next syntax:

--extern NAME=PATH

We can make the PATH optional

--extern NAME

and search the crate NAME in library search directories in this case, and if it's not found then it's an error.
The crate NAME would still be added into extern prelude.

Example:

// `proc_macro` is available in extern prelude and is searched in library search directories
rustc --extern=proc_macro lib.rs

If we have a mechanism like --extern proc_macro we could add Cargo support like this:

[dependencies]
proc_macro = "builtin"
# expanded form (should we allow version constraint here?)
proc_macro = { builtin = true }

Although it seems more bikesheddy than the --extern side.
EDIT: nevermind, I forgot the name = "1.2.3" shorthand also works like this

@eddyb Big fan of that syntax, and the --extern NAME syntax.

Same.

With a potential extension to replace no_std:

[dependencies]
core = "builtin"
std = { builtin = true, optional = true }

Although this will conflict with having a std feature that activates other dependencies features... (but hopefully that can be fixed for optional dependencies in general) and my first thoughts on implementation seem difficult to make backwards compatible with existing no_std crates.

@Nemo157 Ohhhh right, having both name = "foo" and name = { foo = true } is fine!
I updated my original comment, I like this a lot.

cc @rust-lang/cargo

Absolute beginner perspective: I find the anchored_use_paths clearer and easier to understand. I've been dabbling in and out of Rust for over 2 years now reading all the articles I can find etc. Admittedly, I did very little actual coding other than to try the exercises in books etc. So take my opinion for what it's worth... I think it's more approachable especially in conjunction with all the other new changes. Keep up the good work.

We discussed this in the cargo meeting today and felt that we should not make a decision for the unstable crates now, and that we should put proc_macro in the prelude. Possibly, we should rename it.

The other crates are all unstable, and we can leave them requiring extern crate for now.

we should put proc_macro in the prelude

What does this mean? Most crates do not want to link to the compiler.

Possibly, we should rename it.

proc_macro and core are stable.

@SimonSapin You can rename the re-export in the prelude.

@withoutboats Uff, but for non-proc_macro crates, I feel like we should limit it, and not load arbitrary things off the disk just because they happen to "fit".
Maybe someone has an alloc or syntax or test crate-level module and they accidentally put :: in front out of it (e.g. out of habit), now they're getting some random internal crate!

Maybe I should've poked around and asked to join the Cargo team meeting, although I just got more ideas that would work well.

Specifically, we can teach rustfix to add e.g. rustc_driver = "builtin" to Cargo.toml, right?
Oh and it should be "bundled" instead, IMO. A lot of the crates that would be loaded this way don't really feel "builtin".

Also, can we make it an unstable Cargo feature and not worry too much about it?
Since these are all unstable crates. We can stabilize --extern name instead for people not using Cargo, that seems as least hazardous as it gets.

@SimonSapin

What does this mean? Most crates do not want to link to the compiler.

Assuming we can land #49219 in the next week or so, we wouldn't care.

proc_macro would be just an rlib with only a dependency on std, nothing more. In that PR, libtest depends on it (as a hack), so if we build libtest for a target, we get proc_macro too.

We should probably just put proc_macro in the extern_prelude of --crate-type=proc-macro, nothing should realistically break then (oh but libraries like proc-macro2 depend on it... can we get Cargo to pass --extern proc_macro for all of those dependencies? or is that a bad idea?)

Maybe we should just try adding proc_macro to the prelude for everyone on Rust2018 without no_std and see what happens. It shouldn't cause any problems (worst case, uniform_paths ambiguities), at least after #49219.

I agree with @mthgr: I really like anchored_use_paths. They make reading code much clearer. I often tried to read through foreign code bases and the imports were all over the place. So I had to manually check which extern crates are added and which modules exist.

Also, I don't get the argument about being "verbose" or hard to write. I have been using nested imports since they landed (and I don't understand why nested imports aren't used everywhere by nowc'mon people, less repetitionlet's not discuss this here). And with nested imports, using use crate::{...} can actually be shorter than or at least as short as the old non-anchored_use_paths variant. Example with three modules:

// old
use cat::{Ear, Nose, meow};
use cheese::{Color, Holes};
use dog::GodBoy;

// new
use crate::{
    cat::{Ear, Nose, meow},
    cheese::{Color, Holes},
    dog::GodBoy,
};

Sure, it's two lines more, but I also didn't need to repeat the boring use keyword. And I obviously also didn't need to repeat crate:: for each import.

@alexcrichton Would it be realistic to turn off "plain" -L in the new edition (IIUC, -L deps= is needed by Cargo and we don't have a way around it)? We can tell people to migrate to --extern foo=... (we should even be able to print out where Rust2015 would find each of them, just not allow it by default anymore) and keep --extern foo unstable, for sysroot crates?

Eventually, if we want to stabilize, say, alloc, we could stabilize a proper Cargo.toml mechanism.

cc @rust-lang/cargo

Regarding crates in the sysroot, in the Cargo meeting we agreed that long-term we want to find a solution for those that involves explicitly specifying them in Cargo.toml. However, there was some hesitation to adopt something like proc_macro = "builtin" or proc_macro = { builtin = true }, primarily because people wanted something that would align with the future xargo-style integration of std and core, with a syntax that would allow non-builtin versions of all these crates too. So, in the interim, we felt like it made sense to expose only the stable crates in the sysroot (std, core, and proc_macro), and avoid exposing the unstable crates (like test), which means that stable code can avoid using extern crate, while unstable code still has to use extern crate for a little longer until we come up with a good general syntax for handling such dependencies.

As I previously said in https://github.com/rust-lang/rust/issues/53130#issuecomment-414615070, I'm wary of adding proc_macro into extern prelude because proc_macro is not a dependency of std and this means it cannot be added to std prelude, so this means it has to be hardcoded into the compiler.

I made an experiment yesterday and confirmed that use crate as std; placed into rust/src/libstd/prelude/v1.rs works as expected without any compiler hardcoding.
(Except that imports with feature(uniform_paths) can't look into the library prelude yet, but that needs to be fixed if the paths are supposed to be actually uniform.)

@petrochenkov How is that supposed to work, would extern_prelude remain solely for explicitly supplied --extern crates, and core / std would move to the 2018 prelude?

Where would use std; resolve? Unshadowed self::std from the use prelude::* import?
Would explicit self::std actually work, as well? Right now it doesn't/shouldn't.
What would the relationship between --extern crates and contents of the prelude be?


I just looked at #49219 again and the main things it needs from std are:

  • String, Vec, BTreeMap (available in alloc)
  • HashMap, thread_local!, sync::Once and panic control (only in std)

Some of this code is server-side-only so maybe it can be moved to a separate unstable crate.

However, the client::BRIDGE thread_local! and the Once for the panic control must be in the same crate - we can maybe use #[thread_local] static for that, and/or parking_lot?
The big problem is the panic control itself, because proc_macro must catch and silence panics entirely client-side (the server could be using a separate libstd, and only talks to the client through the bridge).

So I don't think it's feasible to make proc_macro a dependency of libstd, in the short term.
(cc @alexcrichton)

@joshtriplett Is that implying ::foo should not load non-extern_prelude crates, and extern crate must be kept around instead?

What about using an unstable Cargo.toml feature to handle e.g. rustc_driver = "bundled"?

@eddyb I was under the impression that it already worked that way in the current version. Does it not?

The proposal was that extern_prelude would include all the crates passed via --extern and all the stable sysroot crates, and those would be the only ones that worked via use and ::foo; unstable sysroot crates would still need extern crate for now, and we'd make a plan to handle them post-edition.

I'd like to have a Cargo.toml syntax for that (corresponding to the rustc syntax --extern foo without an =path), but that would need forward compatibility with std-aware cargo.

I was under the impression that it already worked that way in the current version. Does it not?

Nothing changed, just use foo::...; (without leading ::) under uniform_paths, considers only extern_prelude, leading :: paths with and without uniform_paths are like extern crate still.

but that would need forward compatibility with std-aware cargo

That's only if it's stable, right? But Cargo now has unstable features (e.g. edition and default-run).

@eddyb

How is that supposed to work, would extern_prelude remain solely for explicitly supplied --extern crates, and core / std would move to the 2018 prelude?

Yes.
(std and core can be added to the 2015 library prelude too though.)

Where would use std; resolve? Unshadowed self::std from the use prelude::* import?

With uniform_path? Into name std from the prelude (not from any module).
#[prelude_import] use ... doesn't plant anything into a module. It's not even a use item really, just some unstable vaguely appropriate syntax to put the attribute #[prelude_import] on.

Would explicit self::std actually work, as well? Right now it doesn't/shouldn't.

No, it wouldn't since #[prelude_import] doesn't plant anything into a module.

What would the relationship between --extern crates and contents of the prelude be?

Unrelated?
Two preludes interact slightly in the sense that names from extern prelude shadow names from std library prelude as described in this comment.

The new uniform paths seemed appealing, but when I tried to write something that I thought should "just work," it didn't.

// #![feature(uniform_paths)] // Fails with error "can refer to self::rgb"
pub use rgb;
use rgb::*;

mod k { pub use rgb; }

I'm trying to re-export a crate and then use that crate myself. The error message does correctly suggest doing the following:

#![feature(uniform_paths)]
pub use ::rgb;
use ::rgb::*;

mod k { pub use ::rgb; }

The first scenario without uniform_paths makes more sense to me, but I suppose it's a small thing.

@norcalli Ah, I see. That does seem unfortunate.

@eddyb I think that pub use some_crate; should probably work, by just exporting the crate and not duplicating the existing name from the extern_prelude. Would that be possible?

As for the ambiguity in use rgb::*, that's a bit harder, but I wonder if we could somehow realize that it's the same rgb in both cases?

@norcalli: Exactly, this is how you find out that implicitness does not pay off in the long term.

@norcalli @joshtriplett
This is a limitation of the current implementation, not a fundamental issue with uniform paths.
@sanmai-NL
I don't see how implicitness is relevant to that example.

@petrochenkov:
A path that starts with :: to indicate the next segment refers to an external crate is more explicit than one without ::. The latter case, the less explicit one, is not supported now. You can indeed go ahead and improve the implementation. And we can keep improving uniform_paths until all corner cases are covered. Or we could, hypothetically, stick with the plain and simple anchored_use_paths and spend Rust development time in more productive ways.

Personally, I tend to prefer the anchored paths variant. I admittedly haven't had the opportunity to use either on a major project yet, but from the smaller projects I have played around with, I find that both are major ergonomics improvements over the current system.

However, I haven't actually run into a use case where the total uniformity of uniform paths actually helped me; I haven't needed it. In other words, on the smaller projects I have worked on, anchored use paths are _uniform enough_ for my use cases.

At the same, I find there are a couple of aspects of uniform paths that bother me a bit:

  • I really hate :: prefixes. I find it to be ugly syntax. But more than that, I find it annoying that there is a crate root despite the fact that most imports will just use a crate name or crate. This comes back to the fact that absolute names are still somehow ambiguous. This feels wrong to me: absolute paths should be unambiguous.
  • The mental model seems a bit less straightforward: paths are absolute except when there is a relative path that could work instead. I do realize that this is actually how paths work in expressions in the anchored paths variant, but it feels less intuitive for use paths.

Of course, it is possible that such ambiguities don't show up much in practice and I am just overreacting, but overall, my leaning is towards anchored use paths.

@mark-i-m To clarify something, :: still exists in the anchored variant. And absolute paths are not ambiguous.

You can always write absolute paths if you want to.

Can vs. must. One way vs. multiple ways. It’s a significant philosophical difference.

@joshtriplett sorry, I was unclear. What I mean is that it will be much less necessary to write :: prefixes under anchored paths, and users don't need to be aware of the crate root. It can just be an implementation detail.

To me this allows pathes that start with a name to always be "absolute" in use paths, in the sense that they are not relative to the current module. The same is not true of uniform paths because they may disambiguate to a local module if there is a name conflict, right?

What I mean is that it will be much less necessary to write :: prefixes under anchored paths, and users don't need to be aware of the crate root.

I personally don't think this is likely; It seems to me that conflicts with module names and crate names is rather unlikely and that fixing those problems where they occur will also be quite easy.
To me, the prospect of not having to litter code with self:: seems much more appealing on balance and something that will give larger dividends in practice.

To me, the prospect of not having to litter code with self:: seems much more appealing on balance and something that will give larger dividends in practice.

Agreed. When porting some stuff, I found anchored_use_paths to be a chore with all those self:: prefixes, up to the point where the 2015 edition seemed nicer. I'm on the side of "make the easy things easy, and the hard ones possible". Imagine seeing some Rust code for the first time where a quarter of what's on the first screenfull is a ton of self::.

Having paths implicitly start with self is natural -- you don't see people write "type .\foo.txt".

I'm interested in fully implementing and testing uniform_paths because it gives some fundamentally new abilities compared to both current system and anchored paths, like https://github.com/rust-lang/rust/issues/18084 / https://github.com/rust-lang/rfcs/issues/959.
(This doesn't work in the current implementation yet.)

To me, the prospect of not having to litter code with self:: seems much more appealing on balance and something that will give larger dividends in practice.

Fair enough. I think we just disagree. I would rather see self:: than ::, though I'm not fond of either...

@mark-i-m but do you agree that having to write one (self::) is more likely than writing ::?

Fair enough. I think we just disagree. I would rather see self:: than ::, though I'm not fond of either...

Well, I didn't really have to use ::.

On September 5, 2018 8:24:31 AM PDT, Mazdak Farrokhzad notifications@github.com wrote:

@mark-i-m but do you agree that having to write one (self::) is more
likely than writing ::?

I think this is a fundamental point worth emphasizing.

self:: both comes up more often and is harder to explain the need for, since it feels like it should be the default (and since in the absolute_use_paths variant use paths and expressions have different rules).

:: should come up much less often.

So far, the majority of issues I've seen have been little implementation quirks of the disambiguation approach, such as use somecrate; (now handled) or pub use somecrate (needs handling) or use somecrate::* (needs handling to not generate a conflict on somecrate itself).

The fact that we still have so many implementation quirks suggests to me that we should be conservative and stick with (a forward-compatible variant of?) anchored paths for now, which already brings most of the wins.

EDIT: see this important update to the proposal.

We're getting close to the cut-off point for stabilizations for the Edition, and of course this feature is one of the most important ones remaining to get nailed down.

Based on the commentary in this tracking issue, the previous tracking issue, and various forum posts, I think it's fair to say that the strong majority of people who have tried any variant of 2018 modules prefer it to 2015 modules. And rustfix migration seems to be working well to limit the amount of manual churn required. So from a high level, I think we're in good shape to stabilize some variant for Rust 2018.

However, there's less agreement on which variant to prefer:

  • Anchored paths, where use statements always start with either an external crate name or a keyword. Importing from submodules requires a leading self::

  • Uniform paths, where use statements can also begin with a local item name, and hence work just like paths elsewhere

The "uniform paths" variant makes paths work more consistently, and eliminates a common stumbling block around forgetting self:: in use statements. However:

  • It also makes it a bit more difficult to tell what's being imported. It's possible that we should encourage a style in which local use statements are grouped separately, but in that case use self::{ ... } is arguably more clear, and requiring it would help enforce the style.

  • With the other changes to paths, it's possible that forgetting self:: will become less common -- we don't really know yet.

  • It's not totally clear yet how often local items and extern crate names will clash, requiring disambiguation in use statements.

  • Finally, as @rpjohnst mentions, there are a few remaining implementation concerns around uniform paths -- though I suspect we can eliminate these over the next release cycle.

Given the limited time we have, I want to propose that we take a conservative route. We would ship the anchored paths variant, but with future-proofing that would make it possible to move to uniform paths later. The future proofing is simple: if foo is both an external crate name and a local item name, then a use statement must either say use ::foo or use self::foo, just as it would in the uniform paths variant.

With this conservative approach, we can undergo the major path migration as part of Rust 2018, and reap much of the benefit. Then we can take our time to determine whether to eliminate the need for self:: after we've collectively gotten more comfortable with 2018-style paths.

The main downside, of course, is that churn is potentially split over multiple steps, rather than done all at once. However, the churn of removing no-longer-needed self:: is different from the migration churn; it's purely an idiom adjustment. We already plan to grow the set of "idiom lints" over time, and use rustfix to perform them, so this is probably not a huge issue.

In the interest of getting to a decision, I'm going to formally propose we take this conservative route and stabilize the feature:

@rfcbot fcp merge

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

  • [x] @Centril
  • [x] @aturon
  • [x] @cramertj
  • [x] @eddyb
  • [x] @joshtriplett
  • [ ] @nikomatsakis
  • [ ] @nrc
  • [x] @pnkfelix
  • [x] @scottmcm
  • [x] @withoutboats

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.

I personally think that anchored paths are fantastic, and although uniform paths might be a good extension, anchored paths, as the conservative choice, are good to stabilize now.

I think a conservative path makes sense. I think @aturon you more or less summarized my own feelings of indecision pretty well.

I would personally be excited to move towards uniform_paths as I think self:: is a stumbling block for beginners and proficient rustaceans alike, and that it results in poor ergonomics; but for now the move to a forward compatible anchored-paths solution is a good first step.

One situation i've avoided thinking about so far is how this interacts with the intra-doc links feature in rustdoc. Since it picks apart the path itself, any change to how use statements process paths will also need to be carried over to rustdoc. Presumably we'll need to keep the current behavior for the 2015 edition as well.

@rfcbot concern forward-compatibility

While I understand the desire to go with the more cautious choice at this phase, I'm not sure that "anchored use paths with forward compatibility" is a cautious choice. In particular, of the few concerns I've seen expressed about uniform_paths, the biggest ones have been the cases in which it raises ambiguity errors in cases where they feel it shouldn't. Effectively, adding "forward compatibility' ambiguity errors to anchored use paths gives it the primary remaining pain point (the "implementation quirks") of uniform paths, without the corresponding benefit, and I think making the result more usable will require exactly the same fixes that would polish the last nits of uniform paths.

For instance, we've already made use somecrate; emit a much more straightforward error (effectively "you can just remove this"). We need to make pub use somecrate; work for a somecrate that's already in scope due to extern_prelude, and make use somecrate::*; not issue an error about somecrate itself already being in scope (when self::somecrate refers to exactly the same thing as ::somecrate).

I'd be particularly interested in hearing @eddyb's thoughts comparing the remaining work needed for uniform paths to the hypothetical (not yet written) code for forward-compatibility of anchored use paths. It seems to me that, aside from implementing the latter, we'd still need the same work to make both more usable in cases like pub use somecrate; and use somecrate::*;.

By way of clarification: I'm absolutely open to making the cautious choice at this point; please don't take this as an argument against doing so at all. I'm specifically suggesting that I don't think the proposed path forward gives us that.

Effectively, adding "forward compatibility' ambiguity errors to anchored use paths gives it the primary remaining pain point (the "implementation quirks") of uniform paths, without the corresponding benefit, and I think making the result more usable will require exactly the same fixes that would polish the last nits of uniform paths.

But we can move in either direction with @aturon's proposal: either we can eliminate self:: or we can eliminate the ambiguity errors. In general, the forward compatible choice between two incompatible alternatives usually gives you some of the downsides of both, but only temporarily.

@withoutboats I'm not arguing against that. And to be clear, I'm not trying to use advance one or the other alternative here.

I think part of my concern here is that if we're considering this route, I'd want to see some additional detail on what precise ambiguity errors would be raised. And I'm concerned that this proposal would involve doing some additional work that isn't done (or started) yet, at a point that feels rather last-minute, on a piece of code that has proven quite subtle.

Effectively, adding "forward compatibility' ambiguity errors to anchored use paths gives it the primary remaining pain point (the "implementation quirks") of uniform paths, without the corresponding benefit, and I think making the result more usable will require exactly the same fixes that would polish the last nits of uniform paths.

This was my initial reaction too. However, after thinking a bit, my stance has changed: with uniform paths, the "implementation quirks" show up as errors that "there is no name foo in bar". OTOH, with the forward-compat errors, one would get an error that "bar is ambiguous and you should use ::bar or self::bar". The second is vastly better IMHO.

@mark-i-m Can you clarify the cases you're thinking of here, please? What issue are you referring to?

Very little of my Rust code, sprinkled between crates and toy endeavours, uses self in use. It only happens when it's necessary, and I can't compile with out. The experience shifting to the new module system would be a lot more seamless if I don't need to run rustfix to make my code valid for every file that I work with. That is without addressing the ergonomics issues around self pointed out above. Even with the ambiguity that can come with it, I think that uniform would be the correct choice.

@joshtriplett Mm, that's an excellent point, and I admit that I don't have a firm grasp on the remaining problem cases here. It'd be really helpful to gather them in this tracking issue in the issue description.

To be clear, though, addressing these remaining cases is necessary for either the conservative approach or stabilizing uniform paths. I don't see this issue as a deciding factor on which approach we take -- and notably, there are multiple other concerns that people have with uniform paths in general, which I tried to outline in my summary comment. Note also the feedback on the users thread, which is largely in favor of anchored paths.

@mark-i-m Can you clarify the cases you're thinking of here, please? What issue are you referring to?

@joshtriplett Sure. Consider a crate foo, which conflicts with module self::foo.

Under uniform paths, using a foo::bar path may result in a compile error due to:

  • bar is not found in foo because we looked in the wrong foo (probably the most common case by far)
  • bar is the wrong type because there happens to be a bar in both, but they have different types
  • Perhaps some other unlucky coincidences

Under the forward-compatibility plan, one would instead get an error telling you to disambiguate your paths in all of these cases. This seems like better UX to me. Or am I misunderstanding?

@mark-i-m No, that's definitely not correct! With uniform_paths, if you use foo::bar, and both the crate ::foo and the local module self::foo exist, you should always get an error saying that foo is ambiguous and could refer to either the crate ::foo or the local module self::foo. Those ambiguity warnings are already present in uniform_paths.

...and now I'm wondering if other people have the same concern.

Given the concerns @joshtriplett raised -- the most important of which is that the future-proofing is not currently implemented and may take some time to land -- I want to revise the proposal slightly:

  • For the upcoming release candidate 1, which will serve as a beta for the edition, we stabilize anchored paths as-is, but also allow you to opt in to uniform paths (on the beta channel) so that we can continue testing it and especially testing the ambiguity code.

  • Over the next release cycle, we work to address remaining "false ambiguities" and to fully future-proof anchored paths for the final Edition release. (Or, potentially, we reach a firm consensus on one or the other path variants and just ship it directly, rather than the conservative version).

The main downside here is that you will be able to write code that compiles on the initial beta, but not in the final edition. I'm not sure there's a reasonable alternative.

I'm going to update the previous summary comment to point to this one as well; @joshtriplett I believe this tweaked proposal addresses your concerns, and I will confirm that those who've already reviewed are OK with it.

@aturon Sounds good to me, thank you!

@rfcbot resolved forward-compatibility
@rfcbot reviewed

Oh, I should further note that the "stable-as-is" anchored paths will only ever exist on the beta channel; we plan on removing the ability to opt in to the new Edition before it hits stable, for our extended beta.

@aturon I didn't realize anyone actually uses the beta channel. I only ever used stable or nightly (if I need to do bare-metal stuff). Do you think it will get the exposure we need there?


@joshtriplett Thanks for the correction! That certainly assuages my UX concerns. I still feel that I lean towards anchored paths for the same reasons mentioned before, though (in https://github.com/rust-lang/rust/issues/53130#issuecomment-418430367).

Also, I realize that self::foo would be more common than ::foo, but I prefer self:: to ::foo personally. Apart from syntactic preferences, self:: feels more consistent with all the other ways a path could start and simplifies the mental model IMHO.

@mark-i-m The extended beta plan is here.

I like the idea of making the conservative choice here. I think that even with the anchored version, if there is both a crate named foo and a module named foo, then use foo::bar is a potential source of bugs because someone might think they're using the module's bar when they're actually using the crate's bar.

@aturon

For the upcoming release candidate 1, which will serve as a beta for the edition, we stabilize anchored paths as-is, [...]

Does this mean that on RC1, a user will be able to write foo::bar even tho there is a local module foo?
If so, I'm slightly uncomfortable with the inertia given to keep anchored_paths.

@mikeyhew: I would personally not so much consider it a source of bugs, since two modules are unlikely to be API compatible in the first place, so a compilation error will come up. But it is a waste of developer time and limits the ability for novices to get up to speed when they are confused by such things.

FWIW the uniform_paths canaries (which are used to detect and report ambiguities as errors) are not tied to uniform_paths name resolution, and we could have them enabled with anchored_paths.

Do we want to? If we reach some decision here, let me know to make a quick PR.

EDIT: PR is up at #54011.

@eddyb That definitely seems worth trying! Especially if we can also resolve the issue with pub use some_extern_crate leading to ambiguities -- basically wanting to get resolution to notice if the local item and the external crate are the same thing and not error out.

I just opened #54005 for allowing use crate_name; with uniform_paths, assuming we want that.

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

Can we disambiguate the uniform_paths by using a three colons instead of two when referring to a crate?
Can we also fix the ugly leading :: by using crate::: instead?

The result could be the best synthesis of uniform and anchored variants possible that takes the best parts from them and eliminates the worst.

Example:

use futures:::Future;

fn my_poll() -> futures:::Poll { ... }

mod foo {
    pub struct Bar;

    fn func() {
        println!("foo::func is called");
        super::func();     // Calls a function from top level
    }

    pub mod bar {
        fn func() {
            println!("bar::func is called");
            super::func(); // Calls a function from previous module
        }

        crate fn summary() {
            func();        // Calls a function from current module
            super::func(); // Calls a function from previous module
            crate::func(); // Calls a function from top level
            outer:::func();// Calls a function from external crate 
        }
    }
}

use foo::Bar;

enum SomeEnum {
    V1(usize),
    V2(String),
}

pub fn func() {
    println!("crate::func is called");
    let five = std:::sync::Arc::new(5);
    use SomeEnum::*;
    match ... {
        V1(i) => { ... }
        V2(s) => { ... }
    }
}

Advantages over RFC uniform_paths:

  • Cleaner and more explicit syntax, easier to learn and understand
  • No ambiguities between modules and crates
  • Better code highlighting in code editors

Advantages over RFC anchored_use_paths:

  • Not a verbose, hard to read and write syntax
  • More explicit, since it not clashes with self function parameter
  • Less chances to shoot itself in foot by moving relative path to different location
  • No need to develop a habit to type self very often

@I60R If anchored is going to be used, I think it should be used as-is. Personally I find that code even harder to read, and it would be very strange to have to get used to disambiguating crates with :::

Can we disambiguate the uniform_paths by using a three colons instead of two when referring to a crate?

That's a really subtle distinction, and I don't think it buys us much for the cost and subtlety.

Can we also fix the ugly leading :: by using crate::: instead?

I don't know why people fixate on a syntax that, in the ideal case, you'll almost never have to write. The only time you will ever need to write an explicit leading :: is if you have a local item with the same name as a crate.

Some data: I just got done updating Fuchsia to the latest nightly, and I hit a bunch of errors as a result of the change to be future-compatible with uniform_paths. Most of the errors came from one specific import, use log::log; (or some variant thereof).

The errors looked something like this:

error: `log` import is ambiguous
   --> /usr/local/google/home/cramertj/src/fuchsia/garnet/bin/recovery_netstack/core/src/wire/mod.rs:31:17
    |
31  |               use log::{debug, log};
    |                   ^^^          --- shadowed by block-scoped `log`
    | 
   ::: /usr/local/google/home/cramertj/src/fuchsia/garnet/bin/recovery_netstack/core/src/wire/ipv6.rs:11:18
    |
11  |   use log::{debug, log};
    |                    --- may refer to `self::log` in the future
...
115 |               return debug_err!(
    |  ____________________-
116 | |                 Err(ParseError::Format),
117 | |                 "unexpected IP version: {}",
118 | |                 packet.fixed_hdr.version()
119 | |             );
    | |_____________- in this macro invocation
    |
    = help: write `self::log` explicitly instead
    = note: in the future, `#![feature(uniform_paths)]` may become the default

The code that caused the error is available here.

The issue was fixed by adding use ::log;. We should probably suggest using ::log or self::log in the error message, not just self::log (which would have been incorrect in this case).

I had a similar issue importing use fdio::fdio_sys::*; because fdio_sys included a struct named fdio.

@cramertj I think that you can just use log 0.4.4 or newer, and then stop importing log::log at all, at least when you don't call it directly. From a quick look at the code you linked to, I don't see any calls to log!.

@cramertj It's not suggesting ::log because you have an ambiguity in the macro namespace, not the type/module namespace.

You hit the "nested scopes" problem, where self::foo currently only looks in the same module, but use foo::...;, under uniform_paths, in the future, might end up finding foo in a scope between the current module and the import.

However, you happen to be importing the same entity, so I think we should just silence the error in that case, like we did for imports of an external crate.

EDIT: PR up at #54201.

The final comment period, with a disposition to merge, as per the review above, is now complete.

See also: https://github.com/rust-lang/rust/issues/53130 proposing to stabilize on uniform paths.

@aturon haha; came here to link to that issue as well =P

You linked to the current issue tho ;)
Here's the right issue: https://github.com/rust-lang/rust/issues/55618.

Some follow-up issues:

Was this page helpful?
0 / 5 - 0 ratings