Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

2018: current module shadows crate in use statement #52705

Closed
durka opened this issue Jul 25, 2018 · 1 comment · Fixed by #52923
Closed

2018: current module shadows crate in use statement #52705

durka opened this issue Jul 25, 2018 · 1 comment · Fixed by #52923
Labels
A-rust-2018-preview Area: The 2018 edition preview
Milestone

Comments

@durka
Copy link
Contributor

durka commented Jul 25, 2018

You can use the current module, even without starting the path with crate::, and this can shadow extern crates.

Cargo.toml:

cargo-features = ["edition"]

[package]
edition = "2018"
name = "xyz"
version = "0.1.0"
authors = ["Alex Burka <aburka@seas.upenn.edu>"]

[dependencies]
png = "0.12.0"

src/main.rs:

#![feature(rust_2018_preview)]

mod png;

fn main() {
    println!("Hello, world!");
}

src/png.rs:

use png as png_ext;

fn foo() -> png_ext::DecodingError { unimplemented!() }

This produces the error:

error[E0412]: cannot find type `DecodingError` in module `png_ext`
 --> src/png.rs:3:22
  |
3 | fn foo() -> png_ext::DecodingError { panic!() }
  |                      ^^^^^^^^^^^^^ not found in `png_ext`

(Note that it did not say "can't find png_ext". There was speculation on IRC that the use statement shouldn't have made png_ext available for starting paths, only self::png_ext.)

Further details that point to what's really going on:

  • Changing line 3 to self::png_ext::DecodingError has no effect
  • Renaming the module to anything but png resolves the error
  • Changing the use line to use crate::png as png_ext; has no effect
  • Adding a type to the module called DecodingError resolves the error

So we have a module importing itself, even though it didn't use a crate:: path, and shadowing a crate. This is not one of the features we should be borrowing from the Python module system!

@petrochenkov
Copy link
Contributor

Sigh, single segment imports (use ident [as something], but not use ident::ident::...) use some special hack in fn finalize_import to implement feature(extern_absolute_paths) due to some deficiencies in import resolution infrastructure.
Apparently that hack doesn't always work correctly (namely, in presence of module with the same name), this wasn't supposed to happen.

@alexcrichton alexcrichton added the A-rust-2018-preview Area: The 2018 edition preview label Jul 26, 2018
@Mark-Simulacrum Mark-Simulacrum added this to the Rust 2018 RC milestone Jul 31, 2018
bors added a commit that referenced this issue Aug 14, 2018
#[feature(uniform_paths)]: allow `use x::y;` to resolve through `self::x`, not just `::x`.

_Branch originally by @cramertj, based on @petrochenkov's [description on the internals forum](https://internals.rust-lang.org/t/relative-paths-in-rust-2018/7883/30?u=petrochenkov)._
_(note, however, that the approach has significantly changed since)_

Implements `#[feature(uniform_paths)]` from #53130, by treating unqualified `use` paths as maybe-relative. That is, `use x::y;`, where `x` is a plain identifier (not a keyword), is no longer the same as `use ::x::y;`, and before picking an external crate named `x`, it first looks for an item named `x` in the same module (i.e. `self::x`) and prefers that local item instead.

Such a "maybe-relative" `x` can only resolve to an external crate if it's listed in "`extern_prelude`" (i.e. `core` / `std` and all the crates passed to `--extern`; the latter includes Cargo dependencies) - this is the same condition as being able to refer to the external crate from an unqualified, non-`use` path.
All other crates must be explicitly imported with an absolute path, e.g. `use ::x::y;`

To detect an ambiguity between the external crate and the local item with the same name, a "canary" import (e.g. `use self::x as _;`), tagged with the `is_uniform_paths_canary` flag, is injected. As the initial implementation is not sophisticated enough to handle all possible ways in which `self::x` could appear (e.g. from macro expansion), this also guards against accidentally picking the external crate, when it might actually get "shadowed" later.
Also, more canaries are injected for each block scope around the `use`, as `self::x` cannot resolve to any items named `x` in those scopes, but non-`use` paths can, and that could be confusing or even backwards-incompatible.

Errors are emitted only if the main "canary" import succeeds while an external crate exists (or if any of the block-scoped ones succeed at all), and ambiguities have custom error reporting, e.g.:
```rust
#![feature(uniform_paths)]
pub mod foo {
    use std::io;
    pub mod std { pub mod io {} }
}
```
```rust
error: import from `std` is ambiguous
 --> test.rs:3:9
  |
3 |     use std::io;
  |         ^^^ could refer to external crate `::std`
4 |     pub mod std { pub mod io {} }
  |     ----------------------------- could also refer to `self::std`
  |
  = help: write `::std` or `self::std` explicitly instead
  = note: relative `use` paths enabled by `#![feature(uniform_paths)]`
```
Another example, this time with a block-scoped item shadowing a module-scoped one:
```rust
#![feature(uniform_paths)]
enum Foo { A, B }
fn main() {
    enum Foo {}
    use Foo::*;
}
```
```rust
error: import from `Foo` is ambiguous
 --> test.rs:5:9
  |
4 |     enum Foo {}
  |     ----------- shadowed by block-scoped `Foo`
5 |     use Foo::*;
  |         ^^^
  |
  = help: write `::Foo` or `self::Foo` explicitly instead
  = note: relative `use` paths enabled by `#![feature(uniform_paths)]`
```

Additionally, this PR, because replacing "the `finalize_import` hack" was a blocker:
* fixes #52140
* fixes #52141
* fixes #52705

cc @aturon @joshtriplett
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-rust-2018-preview Area: The 2018 edition preview
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants