rustc_resolve: don't treat uniform_paths canaries as ambiguities unless they resolve to distinct Def's. #54201
Conversation
…ss they resolve to distinct Def's.
rust-highfive
added
the
S-waiting-on-review
|
Per conversation in Discord we should have a test that has something like This LGTM otherwise :) |
Do you mean one |
petrochenkov
added
S-waiting-on-author
|
@petrochenkov I agree in principle, although it's a bit more thorny than that, because one of the initial goals was to be independent of the rest of the import, but also handling |
|
Ok, that's an orthogonal issue, regardless of that resolutions with same |
|
📌 Commit 514c6b6 has been approved by |
bors
added
S-waiting-on-bors
|
@bors p=5 edition critical |
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
|
☀️ Test successful - status-appveyor, status-travis |
| // Currently imports can't resolve in non-module scopes, | ||
| // we only have canaries in them for future-proofing. | ||
| if external_crate.is_none() && results.module_scope.is_none() { | ||
| return; |
In particular, this allows this pattern that @cramertj mentioned in #53130 (comment):
The canaries for the inner
use log::...;, in the macro namespace, see thelogmacro imported at the module scope, and the (same)logmacro, imported in the block scope insidemain.Previously, these two possible (macro namspace)
logresolutions 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
logmacro 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