resolve: Remove artificial import ambiguity errors by petrochenkov · Pull Reques...

r? @jackh726

(rustbot has picked a reviewer for you, use r? to override)

Change description for the lang team.

This PR removes unconditional errors that are currently reported if more than one candidate for a name is found during resolution of a use item.

Before this PR:

struct S; // Outer

fn main() {
    struct S; // Inner

    use S as _; // ERROR `S` is ambiguous

    let s = S; // OK, the Inner `S` is preferred
}

After this PR:

struct S; // Outer

fn main() {
    struct S; // Inner

    use S as _; // OK, the Inner `S` is preferred

    let s = S; // OK, the Inner `S` is preferred
}

These errors are not technically necessary, but they were implemented in 2018 together with the new import semantic because some people were uncomfortable with name shadowing working in imports specifically.

Now we are living with the new import resolution rules for nearly 5 years already, and it should be reasonable to expect that name shadowing that is possible in any other contexts is possible in import contexts too.

Removal of these errors reduces chances of breaking changes when adding any new names to preludes, with #[diagnostic::foo] attributes (rust-lang/rfcs#3368) being a recent example of such addition.

Other ambiguity errors that may be reported during import resolution (glob vs macro-expanded name, etc) and that are actually necessary to prevent things like time travel, are not removed.

Other ambiguity errors that may be reported during import resolution (glob vs macro-expanded name, etc) and that are actually necessary to prevent things like time travel, are not removed.

struct S;

mod foo { pub struct S; }

fn main() {
    use self::foo::*;
    
    use S as _; // ERROR `S` is ambiguous
    
    let _ = S;
}

So this, using a glob import, will still be erroring?

@est31
I think this specific case will error with

  = note: ambiguous because of a conflict between a name from a glob import and an outer scope during import or macro resolution

https://play.rust-lang.org/?version=nightly&mode=debug&edition=2021&gist=6492f42d9a13a034d74a720e219b516d
(It's a part of the "time travel" prevention.)

I was a bit confused by the _ in the example, but if I'm understanding correctly, it's just showing that the use resolution is different from the in-a-type resolution.

I see that this also fails today

mod foo { pub struct S; }

fn main() {
    mod foo { pub struct S; }

    use foo::S; // error[E0659]: `foo` is ambiguous

    let s = S;
}

which I assume would also be resolved by this.

But in general, today's meeting discussion seemed positive towards making the use lookup consistent with the type lookup, so

@rfcbot fcp merge

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

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), 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!

cc @rust-lang/lang-advisors: FCP proposed for lang, please feel free to register concerns.
See this document for info about what commands tagged team members can give me.

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