-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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
rustc_resolve: inject uniform_paths
canaries regardless of the feature-gate, on Rust 2018.
#54011
Conversation
This looks excellent :) |
a9a6c62
to
8cd293d
Compare
☔ The latest upstream changes (presumably #54021) made this pull request unmergeable. Please resolve the merge conflicts. |
8cd293d
to
449516f
Compare
@bors r+ |
📌 Commit 449516f has been approved by |
…rochenkov rustc_resolve: inject `uniform_paths` canary always on Rust 2018. **NOTE**: this PR is based on rust-lang#53988, only the second commit is new. 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). r? @petrochenkov cc @aturon @Centril @joshtriplett
449516f
to
49dd5c1
Compare
@bors r=petrochenkov |
📌 Commit 49dd5c180bdee6e6c0b2fd0ab43b20e08859a6d4 has been approved by |
@bors p=4 |
⌛ Testing commit 49dd5c180bdee6e6c0b2fd0ab43b20e08859a6d4 with merge e14424d052ef6d3e4cc926c010791b240e2cfcd4... |
💔 Test failed - status-appveyor |
☔ The latest upstream changes (presumably #53778) made this pull request unmergeable. Please resolve the merge conflicts. |
That pattern doesn't work, only Do we want to further expand the relaxation to ignore any ambiguity between the crate and any explicit import of the crate in question, in general? |
Ah, I see; I managed to test this incorrectly. You're right, this pattern doesn't work.
If possible, yes. |
49dd5c1
to
9625800
Compare
Assuming we want that relaxation, @bors r=petrochenkov |
📌 Commit 962580076ae48fd528c3ce4ed2ccdfb63fe63698 has been approved by |
⌛ Testing commit 962580076ae48fd528c3ce4ed2ccdfb63fe63698 with merge 6fe2df1d16b21451b6e0e0a5c9620ee4982c5c14... |
💔 Test failed - status-appveyor |
RLS has a legitimate error in it: error: `cargo` import is ambiguous
--> tools\rls\src\build\mod.rs:15:5
|
15 | use cargo::util::important_paths;
| ^^^^^ can refer to external crate `::cargo`
...
35 | mod cargo;
| ---------- may refer to `self::cargo` in the future
|
= help: write `::cargo` or `self::cargo` explicitly instead
= note: in the future, `#![feature(uniform_paths)]` may become the default
error: aborting due to previous error
[RUSTC-TIMING] rls test:false 5.373
error: Could not compile `rls`. What should we do here? |
…ure-gate, on Rust 2018.
…t of the same crate.
uniform_paths
canary always on Rust 2018.uniform_paths
canaries regardless of the feature-gate, on Rust 2018.
9625800
to
d5da94a
Compare
@bors r=petrochenkov |
📌 Commit d5da94a has been approved by |
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
☀️ Test successful - status-appveyor, status-travis |
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
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:
In that any ambiguity between an extern crate and an import of that same crate is ignored.
r? @petrochenkov cc @aturon @Centril @joshtriplett