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

fix(resolve): update the ambiguity glob binding recursively #112743

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions compiler/rustc_resolve/src/imports.rs
Original file line number Diff line number Diff line change
Expand Up @@ -321,6 +321,8 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
} else if !old_binding.vis.is_at_least(binding.vis, this.tcx) {
bvanjoi marked this conversation as resolved.
Show resolved Hide resolved
// We are glob-importing the same item but with greater visibility.
resolution.binding = Some(binding);
} else if binding.is_ambiguity() {
resolution.binding = Some(binding)
}
}
(old_glob @ true, false) | (old_glob @ false, true) => {
Expand Down Expand Up @@ -393,7 +395,6 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
let t = f(self, resolution);

match resolution.binding() {
_ if old_binding.is_some() => return t,
None => return t,
Some(binding) => match old_binding {
Some(old_binding) if ptr::eq(old_binding, binding) => return t,
Expand All @@ -402,7 +403,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
}
};

// Define `binding` in `module`s glob importers.
// Define or update `binding` in `module`s glob importers.
for import in module.glob_importers.borrow_mut().iter() {
let mut ident = key.ident;
let scope = match ident.span.reverse_glob_adjust(module.expansion, import.span) {
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/imports/duplicate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ mod g {
fn main() {
e::foo();
f::foo(); //~ ERROR `foo` is ambiguous
g::foo();
g::foo(); //~ ERROR `foo` is ambiguous
}

mod ambiguous_module_errors {
Expand Down
22 changes: 21 additions & 1 deletion tests/ui/imports/duplicate.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,26 @@ LL | pub use b::*;
| ^^^^
= help: consider adding an explicit import of `foo` to disambiguate

error[E0659]: `foo` is ambiguous
--> $DIR/duplicate.rs:36:8
|
LL | g::foo();
| ^^^ ambiguous name
|
= note: ambiguous because of multiple glob imports of a name in the same module
note: `foo` could refer to the function imported here
--> $DIR/duplicate.rs:24:13
|
LL | pub use a::*;
| ^^^^
= help: consider adding an explicit import of `foo` to disambiguate
note: `foo` could also refer to the function imported here
--> $DIR/duplicate.rs:25:13
|
LL | pub use b::*;
| ^^^^
= help: consider adding an explicit import of `foo` to disambiguate

error[E0659]: `foo` is ambiguous
--> $DIR/duplicate.rs:49:9
|
Expand All @@ -68,7 +88,7 @@ LL | use self::m2::*;
| ^^^^^^^^^^^
= help: consider adding an explicit import of `foo` to disambiguate

error: aborting due to 4 previous errors
error: aborting due to 5 previous errors

Some errors have detailed explanations: E0252, E0659.
For more information about an error, try `rustc --explain E0252`.
30 changes: 30 additions & 0 deletions tests/ui/resolve/issue-105235.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
// edition: 2021
// check-pass

mod abc {
pub struct Beeblebrox;
pub struct Zaphod;
}

mod foo {
pub mod bar {
use crate::abc::*;

#[derive(Debug)]
pub enum Zaphod {
Whale,
President,
}
}
pub use bar::*;
}

mod baz {
pub fn do_something() {
println!("{:?}", crate::foo::Zaphod::Whale);
}
}

fn main() {
baz::do_something();
}
16 changes: 16 additions & 0 deletions tests/ui/resolve/issue-112713.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
// edition: 2021

pub fn foo() -> u32 {
use sub::*;
C //~ERROR `C` is ambiguous
Copy link
Contributor Author

@bvanjoi bvanjoi Jun 21, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a breaking change. It will throw an ambiguous error after this PR, but it currently compiles without error.

}

mod sub {
mod mod1 { pub const C: u32 = 1; }
mod mod2 { pub const C: u32 = 2; }

pub use mod1::*;
pub use mod2::*;
}

fn main() {}
23 changes: 23 additions & 0 deletions tests/ui/resolve/issue-112713.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
error[E0659]: `C` is ambiguous
--> $DIR/issue-112713.rs:5:5
|
LL | C
| ^ ambiguous name
|
= note: ambiguous because of multiple glob imports of a name in the same module
note: `C` could refer to the constant imported here
--> $DIR/issue-112713.rs:12:13
|
LL | pub use mod1::*;
| ^^^^^^^
= help: consider adding an explicit import of `C` to disambiguate
note: `C` could also refer to the constant imported here
--> $DIR/issue-112713.rs:13:13
|
LL | pub use mod2::*;
| ^^^^^^^
= help: consider adding an explicit import of `C` to disambiguate

error: aborting due to previous error

For more information about this error, try `rustc --explain E0659`.
25 changes: 25 additions & 0 deletions tests/ui/resolve/issue-112743.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
// https://github.com/rust-lang/rust/pull/112743#issuecomment-1601986883

macro_rules! m {
() => {
pub fn EVP_PKEY_id() {}
};
}

mod openssl {
pub use self::evp::*;
pub use self::handwritten::*;

mod evp {
m!();
}

mod handwritten {
m!();
}
}
use openssl::*;

fn main() {
EVP_PKEY_id(); //~ ERROR `EVP_PKEY_id` is ambiguous
}
23 changes: 23 additions & 0 deletions tests/ui/resolve/issue-112743.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
error[E0659]: `EVP_PKEY_id` is ambiguous
--> $DIR/issue-112743.rs:24:5
|
LL | EVP_PKEY_id();
| ^^^^^^^^^^^ ambiguous name
|
= note: ambiguous because of multiple glob imports of a name in the same module
note: `EVP_PKEY_id` could refer to the function imported here
--> $DIR/issue-112743.rs:10:13
|
LL | pub use self::evp::*;
| ^^^^^^^^^^^^
= help: consider adding an explicit import of `EVP_PKEY_id` to disambiguate
note: `EVP_PKEY_id` could also refer to the function imported here
--> $DIR/issue-112743.rs:11:13
|
LL | pub use self::handwritten::*;
| ^^^^^^^^^^^^^^^^^^^^
= help: consider adding an explicit import of `EVP_PKEY_id` to disambiguate

error: aborting due to previous error

For more information about this error, try `rustc --explain E0659`.
21 changes: 21 additions & 0 deletions tests/ui/resolve/issue-56593-0.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
// check-pass

mod a {
pub trait P {}
}
pub use a::*;

mod b {
#[derive(Clone)]
pub enum P {
A
}
}
pub use b::P;

mod c {
use crate::*;
pub struct S(Vec<P>);
}

fn main() {}
18 changes: 18 additions & 0 deletions tests/ui/resolve/issue-56593-1.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
// check-pass

struct Foo;

mod foo {
use super::*;

#[derive(Debug)]
pub struct Foo;
}

mod bar {
use super::foo::*;

fn bar(_: Foo) {}
}

fn main() {}
26 changes: 26 additions & 0 deletions tests/ui/resolve/issue-56593-2.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
// check-pass

use thing::*;

#[derive(Debug)]
pub enum Thing {
Foo,
}

#[cfg(test)]
mod tests {
use super::*;

#[test]
fn test_thing() {
let thing = Thing::Foo;
Copy link
Contributor Author

@bvanjoi bvanjoi Jun 21, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a breaking change too: root::Thing currently refers to root::thing::Thing, but it will refer to root::Thing after this PR.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment has a typo, but I assume you mean root::thing::Thing -> root::Thing.
It's also clearly a bugfix.

Crater will finish soon and show what actually breaks after these changes.

}
}

mod thing {
pub enum Thing {
Bar,
}
}

fn main() {}