Glob Time Travel

[vlad20012]

I tried this code:

mod foo {
    pub mod bar {
        pub mod bar {
            pub fn foobar() {}
        }
    }
}

use foo::*;
use bar::bar;
use bar::foobar;



fn main() {
    bar::foobar();
}

I expected to see this happen: Compilation error. This use bar::bar; shadows one (glob-imported) bar with another bar. As far as I know, this should be forbidden.

Instead, this happened: this code successfully compiled.

Meta

rustc --version --verbose:

rustc 1.47.0-nightly (d7f945163 2020-07-19)
binary: rustc
commit-hash: d7f94516345a36ddfcd68cbdf1df835d356795c3
commit-date: 2020-07-19
host: x86_64-unknown-linux-gnu
release: 1.47.0-nightly
LLVM version: 10.0

c.c. @matklad
c.c. @petrochenkov

[HeroicKatora]

This use bar::bar; shadows one (glob-imported) bar with another bar. As far as I know, this should be forbidden.

Shadowing a glob import with a specific item is allowed and will unambiguously resolve to the specific one. This is also how the prelude works. The following compiles fine as well:

// implicit: use std::prelude::v1::*; containing Vec
struct Vec;

fn a() -> Vec { unimplemented!() }

playground.

[matklad]

To clarify, the problem is not shadowing, but time travel. We shadow identifier which have been already used to resolve another import, so we end up with bar meaning two things at the same time in the same scope.

[petrochenkov]

This is a change from Rust 1.44, in Rust 1.43 this is an ICE "inconsistent resolution for an import".
Possibly a consequence of #70236.

[petrochenkov]

I'm not sure this is a bug.

When resolving use bar::bar; we are looking at all names in scope except for the names introduced by use bar::bar; itself. Otherwise we'd have cycles even in trivial cases like use my_crate; where my_crate is a name from extern prelude.

If you take this detail into consideration, then all paths seem to be resolved correctly.

[petrochenkov]

cc #62769 (somewhat related)

[vlad20012]

I think, the difference with use my_crate; is that use my_crate; doesn't replace one resolution with another. We already have a name my_crate in the scope that is resolved to a particular extern crate.

In my example, use bar::bar; uses bar name from the scope, and then rebinds this bar name to another item.

I can complicate my example a bit.

mod foo {
    pub mod bar {
        pub mod bar {
            pub fn foobar() { println!("111") }
        }
        pub fn foobar() { println!("222") }
    }
}

use foo::*;
use bar::bar;
use bar::foobar;

fn main() {
    foobar();
}

What this program prints, "111" or "222"? It'd say, it depends on imports resolution order, but looks like it always prints "111".

[petrochenkov]

It'd say, it depends on imports resolution order

The intent for the resolution results is to never depend on internal resolution order.
(cc #53778 (comment), order-dependence can probably happen in practice due to the current not very principled implementation, but hopefully still can be eliminated in backward-compatible-in-practice way by rewriting the main resolution/expansion loop more carefully.)

but looks like it always prints "111"

That's what I'd expect.

• foobar resolves to use bar::foobar
• bar in bar::foobar resolves to use bar::bar (because non-globs shadow globs)
• first bar in bar::bar resolves to use foo::* (which the only bar in scope after excluding use bar::bar itself)

[petrochenkov]

I'd still be ok with replacing the former ICE with an error though, since that's a more conservative choice.

[petrochenkov]

Actually enabling the property "name resolve to the same thing whether some items were excluded during its resolution to avoid cycles or not" may be useful for using something like "inference variables" for unresolved imports and making import resolution more like unification in type inference.

That's an idea I had for quite some, but I hadn't realized that merging #70236 went against it.

[petrochenkov]

Sigh, the issue already affects three stable releases - from 1.44 to 1.46.
I'll try to prioritize it.

[petrochenkov]

Addressed in #77421.

[camelid]

Assigning P-high so this isn't lost. See the relevant discussion.

[petrochenkov]

The fix caused some regressions - #77586.

[vlad20012]

@petrochenkov The code in the issue produce no errors using Rust 1.67.1. Should we re-open the issue or is it not considered a bug now?

[petrochenkov]

@vlad20012
The fix was reverted due to breakage (#78784) and the current behavior is considered by design since then.

[petrochenkov]

(Also it's not exactly a time travel - #77586 (comment).)