Block import resolution only on 'pub' imports #27547
Conversation
When resolving 'use' statements, only consider pub imports of the target module as blocking. Closes rust-lang#4865
|
r? @Aatch (rust_highfive has picked a reviewer for you, use r? to override) |
elinorbgr
changed the title
|
Does this change work in the case of non-public, but visible imports? (i.e., in a nested module). I think it does because glob imports only import public items, not visible ones. Does this have a backwards compatibility hazard there? I.e., will this prevent us making globs import visible items in the future if we want to? |
| // except according to those terms. | ||
|
|
||
| pub mod a { | ||
| use b::*; |
|
About visible items, yeah, there might be a compatibility hazard. This change rely on the fact that item imported by non- However, I will possibly make it harder to introduce help messages in a kind of "maybe you forgot to add I'll address your nits tomorrow, it's almost midnight for me. 😉 |
|
@nrc For the case of importing a module into itself, is it enough to just add an help message, like this: or should I make a specific error (and add en error code ?) |
|
@vberger I don't think you can have different messages for the same error code. Probably the easiest thing to do is to add the extra detail as a note. |
|
@nrc well, the error message E0432 already have several messages on it, but they all are a variation on "Unresolved import": The second half of the error message is actually fully-customizable, and I already have the code working to generate the message So, would that be fine ? |
|
@nrc see my last commit for the implementation |
elinorbgr
force-pushed
the
more_perseverant_resolve
branch
from
b44c993
to
5847ea7
Compare
|
The diagnostics stuff just checks the string literal, so if the second part is just |
|
lgtm, thanks for the changes! @bors: r+ |
|
📌 Commit 5847ea7 has been approved by |
|
⌛ Testing commit 5847ea7 with merge 8d6a10d... |
|
💔 Test failed - auto-linux-32-opt |
|
Looks like #27619 |
|
@bors retry |
As noted in my previous PR #27439 , the import resolution algorithm has two cases where it bails out: - The algorithm will delay an import if the module containing the target of the import still has unresolved glob imports - The algorithm will delay a glob import of the target module still has unresolved imports This PR alters the behaviour to only bail out when the above described unresolved imports are `pub`, as non-pub imports don't affect the result anyway. It is still possible to fail the algorithm with examples like ```rust pub mod a { pub use b::*; } pub mod b { pub use a::*; } ``` but such configurations cannot be resolved in any meaningful way, as these are cyclic imports. Closes #4865
As noted in my previous PR #27439 , the import resolution algorithm has two cases where it bails out:
This PR alters the behaviour to only bail out when the above described unresolved imports are
pub, as non-pub imports don't affect the result anyway.It is still possible to fail the algorithm with examples like
but such configurations cannot be resolved in any meaningful way, as these are cyclic imports.
Closes #4865