Skip to content

rustc_resolve: allow use crate_name; under uniform_paths. #54005

New issue

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

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

Already on GitHub? # to your account

Merged
merged 1 commit into from
Sep 7, 2018

Conversation

eddyb
Copy link
Member

@eddyb eddyb commented Sep 6, 2018

Specifically, use crate_name; and use crate_name::{self, ...}; are now allowed, whereas previously there would produce a (false positive) ambiguity error, as the ambiguity detection code was seeing the crate_name import as a locally-available definition to conflict with the crate.

r? @petrochenkov cc @aturon @joshtriplett @Centril

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 6, 2018
@joshtriplett
Copy link
Member

LGTM, awesome!

@aturon
Copy link
Member

aturon commented Sep 6, 2018

Setting priority to land for beta ASAP

@bors p=100

@Centril
Copy link
Contributor

Centril commented Sep 6, 2018

This is great 🎉💯❤️

@petrochenkov
Copy link
Contributor

@bors r+

@bors
Copy link
Collaborator

bors commented Sep 6, 2018

📌 Commit 31fce91 has been approved by petrochenkov

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 6, 2018
@bors
Copy link
Collaborator

bors commented Sep 6, 2018

⌛ Testing commit 31fce91 with merge ad7b22e...

bors added a commit that referenced this pull request Sep 6, 2018
rustc_resolve: allow `use crate_name;` under `uniform_paths`.

Specifically, `use crate_name;` and `use crate_name::{self, ...};` are now allowed, whereas previously there would produce a (false positive) ambiguity error, as the ambiguity detection code was seeing the `crate_name` import as a locally-available definition to conflict with the crate.

r? @petrochenkov cc @aturon @joshtriplett @Centril
@bors
Copy link
Collaborator

bors commented Sep 7, 2018

☀️ Test successful - status-appveyor, status-travis
Approved by: petrochenkov
Pushing ad7b22e to master...

@bors bors merged commit 31fce91 into rust-lang:master Sep 7, 2018
@eddyb eddyb deleted the uniform-paths-self-resolve branch September 7, 2018 08:38
bors added a commit that referenced this pull request Sep 10, 2018
rustc_resolve: inject `uniform_paths` canaries regardless of the feature-gate, on Rust 2018.

This PR is an attempt at future-proofing "anchored paths" by emitting the same ambiguity errors that `#![feature(uniform_paths)]` would, with slightly changed phrasing (see added UI tests).

Also, on top of #54005, this PR allows this as well:
```rust
use crate_name;
use crate_name::foo;
```
In that any ambiguity between an extern crate and an import *of that same crate* is ignored.

r? @petrochenkov cc @aturon @Centril @joshtriplett
bors added a commit that referenced this pull request Sep 14, 2018
rustc_resolve: don't treat uniform_paths canaries as ambiguities unless they resolve to distinct Def's.

In particular, this allows this pattern that @cramertj mentioned in #53130 (comment):
```rust
use log::{debug, log};
fn main() {
    use log::{debug, log};
    debug!(...);
}
```
The canaries for the inner `use log::...;`, *in the macro namespace*, see the `log` macro imported at the module scope, and the (same) `log` macro, imported in the block scope inside `main`.

Previously, these two possible (macro namspace) `log` resolutions would be considered ambiguous (from a forwards-compat standpoint, where we might make imports aware of block scopes).

With this PR, such a case is allowed *if and only if* all the possible resolutions refer to the same definition (more specifically, because the *same* `log` macro is being imported twice).
This condition subsumes previous (weaker) checks like #54005 and the second commit of #54011.

Only the last commit is the main change, the other two are cleanups.

r? @petrochenkov cc @Centril @joshtriplett
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants