Pre-RFC: allow conflicting names on import, only complain if used ambiguously

[josh]

Consider a module that wants to use trait methods from both std::fmt::Write and std::io::Write (via write! or writeln!), and thus needs to import both. For anything other than a trait, it would work to use std::fmt; and use std::io; and then refer to the qualified names fmt::Write and io::Write. However, using the methods of a trait requires importing that trait directly, necessitating a rename:

use std::fmt::Write as FmtWrite;
use std::io::Write as IoWrite;

This applies even if the module will never use those names, and only wants to call trait methods like write_fmt.

I’d like to propose changing the compiler to allow importing names that conflict without producing an error, and only emitting an error if code uses the names ambiguously. For instance, the following would work:

use std::fmt::{self, Write};
use std::io::{self, Write};

impl io::Write for ... { ... }
...
try!(writeln!(...));

But the following code would produce an error:

use std::fmt::{self, Write};
use std::io::{self, Write};

impl Write for ... { ... }

The error message would look roughly like this:

error[E0405]: trait name `Write` ambiguous
 --> <anon>:1:6
  |
1 | impl Write for u32 {
  |      ^^^^^ `Write` is ambiguous
  |
  = help: Use one of the following qualified names:
  = help:   `fmt::Write`
  = help:   `io::Write`

In practice, this would only apply to trait names, since any other name never referenced directly would produce a warning about the unused import. I’d have no objection to the compiler continuing to detect name conflicts on non-traits early, and only deferring name conflicts for traits.

Does this seem reasonable?

[scott]

I like this idea a lot, and it doesn’t seem complicated or confusing. You basically get the same error as before, but only when it’s actually a problem.

[Kimundi]

Hm, seems this would only be a slight extension of the accepted name resolution change RFC (see below), to allow ambiguity even in explicitly imported names as long as you only anonymously access them, like with bringing traits methods in scope.

Changes to name resolution

[retep998]

An alternative solution to this problem…

Issue: use some::Trait as _;

[petrochenkov]

https://github.com/rust-lang/rfcs/issues/1311 seems to be a better solution.

You’d normally want to avoid duplicated non-glob imports, so duplicates need to be at least linted against unless explicitly wanted. “Explicitly wanted” will mean #[allow(...)] in this case, which is less convenient than Trait as _.

EDIT: The lint can be adjusted to report only true duplicates (same name + same item) though.

[jseyfried]

Like @retep998 and @petrochenkov, I prefer the solution from https://github.com/rust-lang/rfcs/issues/1311, in which

use std::fmt::Write as _;
use std::io::Write as _;

would allow trait methods from both traits to be used without importing any trait names.

As @Kimundi mentioned above, #![feature(item_like_imports)] (RFC 1560) allows unused conflicting glob imports. If a conflicting glob import is used, we emit an ambiguity error similar to what @josh proposed (example gist).

However, if two globs both import a trait named Write, neither traits’ methods may be used today since we only allow methods from traits that have an unambiguous name (in the relevant module). It might be a good idea to change this.

Similarly, if a glob-imported trait is shadowed by an explicitly imported trait, only the explicitly imported trait’s methods may be used (since the shadowed trait is unnameable).

[josh]

@retep998, @petrochenkov: I like RFC 1311 as a way to not have a name in scope at all. However, that requires a separate use line (since you can’t use as as part of a larger use declaration). I’d like to just write use std::io::{self, Read, Write};.

And I agree that the lint should only report duplicate imports of the same item (as well as unused imports, and name conflicts at the point of usage).

[eddyb]

use m::{A as B, x as y}; either works or the compiler just supports it for no reason (can’t test atm but I know I’ve seen the infra to support renames in multi-import lists).

[josh]

I stand corrected; apparently you can. I’ve never actually seen any code do so, and in particular I’ve seen many different modules import a pile of names in one declaration followed by a line or two of separate use lines for renamed names. I’d assumed from that that Rust didn’t support it, but apparently it does and many authors of Rust code just didn’t know that.

I still think the deferred-conflict approach would provide more user friendliness, such as in the common case of the two Write traits, but the approach in RFC 1311 would no longer add significant verbosity.

[DanielKeep]

[…] but apparently it does and many authors of Rust code just didn’t know that.

/me raises hand

I’m really not sure why it never occurred to me to try this before.

[quodlibetor]

It wasn’t always. I think a lot of people got used to writing use lines before it was possible?

[withoutboats]

I like this solution better than 1311. Not throwing errors when there’s not a problem seems like the simplest solution.

[leonardo]

This is how D language import works, it gives errors only when there’s a true ambiguity in the code.

The same happens in D with its with statement:

struct Foo { int x, y; }
void main() {
    int x = 0;
    auto f = Foo(10, 20);
    with(f) {
        y++; // OK, no shadowing.
    }
}

[bruno_medeiros]

I like this solution better than 1311. Not throwing errors when there’s not a problem seems like the simplest solution.

Agreed. It doesn’t require the extra syntax of #1331, and it’s a rule that would work well with glob imports too. (ie, not just for traits)

[troplin]

I like this solution better than 1311. Not throwing errors when there’s not a problem seems like the simplest solution.

OTOH this would mean that use and pub use behave quite differently. I’m not sure if I like this.

Many of those new convenience/ergonomics “features” since Rust 1.0 make the language appear simple to newcomers but they actually making it even more complex to really understand.

[withoutboats]

1. Conceiving of the pub modifier on the use as one of many ways to actually employ a symbol you have imported does not seem confusing to me.
2. “Quite different” is a stretch. I import a trait with a name conflict in like 1 in 100 modules; and I have never wanted to re-export it.

[petrochenkov]

pub modifier on the use as one of many ways to actually employ a symbol

That’s a bad idea There are only two ways to use an imported name: 1) name it, 2) that method resolution thing for traits. (And 2 is not a use that should generate a conflict during resolution, type checker reports ambiguous methods regardless of names used to import traits).

---

Pretty basic assumption of the current resolution pass is that key (name, namespace) has a single resolution. So every used path a::b::c can be either resolved unambiguously, or unresolved. This preventive error reporting is the same general idea that Rust employs in other places (i.e. impl coherence, early checking of trait bounds). Glob conflicts like

// OK, no error reported
use a::*; // imports type a::S
use b::*; // imports type b::S

are shoehorned into this single resolution system by creating special “ambiguity items”

// Previous snippet is desugared into something like
ambiguity_item S {
    a::S,
    b::S,
}
// key: `(S, type_ns)` => value: `ambiguity_item S`

these items behave like usual items, in particular, they can be reexported with pub.

---

I guess it’s possible to change the import resolution algorithm to permit arbitrary number of items with the same name in a namespace, i.e. key: (name, namespace) => value: Vec<resolution>. In this case preventive error reporting will be done by a separate post-processing pass, which would report conflicts only when they are guaranteed to happen on use.

// Post-processing pass reports a preventive error, because there's no way name `S` can be unambiguously used
struct S {}
struct S {}

mod m {
    pub struct S {}
    pub fn S() {}
}
mod m {
    pub use m::S;
    pub fn S() {}
}

// Post-processing pass doesn't report a preventive error, because there IS a way name `S` can be unambiguously used - it can be used in context requiring type namespace.
// The error is reported only if `S` is used in value namespace (possibly in other crate!).
use m::S;
use n::S;

// Post-processing pass doesn't report a preventive error, because there IS a way `Tr` can be unambiguously used - it can be used for method resolution without using the name `Tr`.
trait Tr {}
trait Tr {}

This scheme would also require recording all sets of ambiguous names in crate metadata, that would help with resolving https://github.com/rust-lang/rust/issues/36837. @jseyfried should probably know in more detail how good, bad or implementable this idea is. Anyway, this needs an RFC and stabilizing RFC 1560 first.

[withoutboats]

This preventive error reporting is the same general idea that Rust employs in other places (i.e. impl coherence, early checking of trait bounds).

These preventative checks are strongly motivated to avoid users getting errors they can’t solve. If we check coherence only on invokation, and a library never invokes its traits, clients of that library will get coherence issues that can only be solved by patching the library.

Your example seems to show a case in which this would also be true by not preventatively checking dual imports. (the example with the “possibly in other crate” comment). Can you elaborate on that example? That’s a very good reason not to allow this.

[petrochenkov]

Your example seems to show a case in which this would also be true by not preventatively checking dual imports. (the example with the “possibly in other crate” comment). Can you elaborate on that example? That’s a very good reason not to allow this.

Not much to elaborate really. Crate 1:

mod m {
    pub struct S {}
    pub fn S() {}
}
mod m {
    pub use m::S;
    pub fn S() {}
}

pub use m::S;
pub use n::S;

Crate 2:

pub use crate1::S; // m::S{type}, m::S{value}, n::S{value}

let a = S; // Unresolvable ambiguity

This situation already exists with glob imports (https://github.com/rust-lang/rust/issues/36837 has a concrete example), but only under some esoteric conditions. “Ambiguity items” are not encoded in metadata right now, this makes resolution behavior inconsistent between local crate and other crates. The changes proposed in this thread would make the situation more common.

[josh]

In this proposal, you would not be allowed to pub use m::S and pub use n::S; that would generate an error. So, ambiguous names should never end up in crate metadata, because you can’t export them.

You’d be allowed to use m::S and use n::S, as long as you never used the name S for anything except invoking trait methods on objects that impl one of those traits.