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

item_like_imports: Can "ambiguity error" items be reexported? #36837

Open
petrochenkov opened this issue Sep 29, 2016 · 4 comments
Open

item_like_imports: Can "ambiguity error" items be reexported? #36837

petrochenkov opened this issue Sep 29, 2016 · 4 comments
Assignees
Labels
A-resolve Area: Name resolution C-enhancement Category: An issue proposing an enhancement or a PR with one. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@petrochenkov
Copy link
Contributor

petrochenkov commented Sep 29, 2016

Is the next code well formed or not?

#![feature(item_like_imports)]

mod ty1 {
    pub type A = u8;
}
mod ty2 {
    pub type A = u8;
}
mod ambig {
    // Create an ambiguity item in type namespace.
    pub use ty1::*;
    pub use ty2::*;
}
mod merge {
    pub use ambig::*; // <- reexports an ambiguity error, or reexports nothing?
    pub use ty1::*; // <- does this contribute to the ambiguity error or create a valid name?
}

fn main() {
    let _: merge::A; // <- Valid or an ambiguity error?
}

Currently the behavior of this code depends on whether the "merge" step is done in the same crate or in some other crate.
In a single crate scenario the snippet above fails to compile with A is ambiguous error.
If merge is moved to another crate, then all erroneous resolutions are filtered away and are not represented in metadata, pub use ambig::* becomes an empty import and merge::A unambiguously means ty1::A.
Supposedly, local and cross-crate behavior should be the same.

cc @jseyfried @nrc

@petrochenkov
Copy link
Contributor Author

My opinion so far is that reexported Def::Errs (ambiguity and others) should never affect resolution results, but may affect diagnostics and give better error messages, i.e. the code above is valid.

@jseyfried
Copy link
Contributor

jseyfried commented Sep 29, 2016

I agree that ideally your example would be valid, but I don't think that's possible to implement, at least not without introducing a lot of ambiguous imports ("deadlocking") in code that compiles today.

For example, if we resolve pub use ty2::*; first and resolve pub use ambig::*; next, then the resolution of A in merge is determined to be ty2::A, so it could be re-exported in other places or used in other imports' module paths (if it were a module instead of a type alias).

If we couldn't assume that the resolution of A in merge was ty2::A in the above case, I don't think we'd be able to make enough progress to support a lot of existing code.

@jseyfried
Copy link
Contributor

jseyfried commented Sep 29, 2016

More specifically, if we wanted to support that example then we wouldn't be able to glob-import a name unless we knew that the name would never become ambiguous in the glob-imported module.

Thus, we wouldn't be able to support the following, which compiles today:

mod foo {
    pub mod quux {}
}
mod bar {
    pub use foo::*;
    pub use baz::quux::*; // We need to resolve this to deduce that `bar::quux` isn't ambiguous,
}
mod baz {
    pub use bar::*; // but that requires importing `quux` here, which we wouldn't be able to do until we have deduced that `quux` isn't ambiguous.
}

@steveklabnik steveklabnik added A-type-system Area: Type system and removed A-type-system Area: Type system labels Oct 7, 2016
@petrochenkov petrochenkov added the A-resolve Area: Name resolution label Feb 19, 2017
@Mark-Simulacrum Mark-Simulacrum added the C-enhancement Category: An issue proposing an enhancement or a PR with one. label Jul 26, 2017
@petrochenkov petrochenkov self-assigned this Mar 14, 2020
@Noratrieb Noratrieb added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Apr 5, 2023
bors added a commit to rust-lang-ci/rust that referenced this issue Dec 15, 2023
resolve: re-export ambiguity as warning

Fixes rust-lang#36837

Expose these ambiguous bindings as warnings instead of simply concealing them. This change introduces potential breaking alterations. For instance, code that previously compiled successfully may now fail to compile as expected:

```rs
// lib.rs
mod a {
  pub type C = i8;
}

mod b {
  pub type C = i16;
}

pub use a::*;
pub use b::*;

// main.rs
extern crate lib;

mod a {
    pub type C = i32;
}

use lib::*;
use a::*;

fn main() {
    let _: C = 1; // it will throw an ambiguity error but previous it will not.
}
```
r? `@petrochenkov`
@petrochenkov
Copy link
Contributor Author

Cross-crate encoding for reexported ambiguities was implemented in #114682 but not merged.

Issue "Tracking Issue for ambiguous_glob_imports lint" is also relevant here because cross-crate reexports for ambiguities that are usable for backward compatibility are a part of that warning.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-resolve Area: Name resolution C-enhancement Category: An issue proposing an enhancement or a PR with one. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants