More perseverant resolve (second try) #27439
Conversation
There is not situation where `foo` would be unresolved but not `foo::*`.
Most errors generated by resolve might be caused by not-yet-resolved glob imports. This changes the behavior of the resolve imports algorithms to not fail prematurally on first error, but instead buffer intermediate errors and report them only when stuck. Fixes rust-lang#18083
The precedent resolve modification changed the order in which imports are handled, so 2 tests needed to be updated.
|
Hm this has gotten more hairy since the last time I looked at it, so I'm gonna transfer review: r? @nrc |
|
I just finished testing, the crates |
|
@fzzy This is due to an other difficulty in the import resolution algorithm, that this PR does not change:
In your example case, I think it can be possible to relax the two rules I described just above to only block on |
|
@vberger It would mean so much to me, and make my code so much nicer, if you made it so glob imports only blocked on pub imports. I hope to see such a PR soon. |
|
lgtm, I'd like to get a crater run before landing though |
|
@brson: can we get a crater run for this PR please? (Sorry, can't find the instructions you sent out) |
|
@nrc Are the descriptions in the tests ok now ? |
|
Kicking off some crater builds now |
|
@vberger yep, lgtm, thanks! |
|
Crater says zero regressions |
(This is a second try at #26242. This time I think things should be ok.) The current algorithm handling import resolutions works sequentially, handling imports in the order they appear in the source file, and blocking/bailing on the first one generating an error/being unresolved. This can lead to situations where the order of the `use` statements can make the difference between "this code compiles" and "this code fails on an unresolved import" (see #18083 for example). This is especially true when considering glob imports. This PR changes the behaviour of the algorithm to instead try to resolve all imports in a module. If one fails, it is recorded and the next one is tried (instead of directly giving up). Also, all errors generated are stored (and not reported directly). The main loop of the algorithms guaranties that the algorithm will always finish: if a round of resolution does not resolve anything new, we are stuck and give up. At this point, the new version of the algorithm will display all errors generated by the last round of resolve. This way we are sure to not silence relevant errors or help messages, but also to not give up too early. **As a consequence, the import resolution becomes independent of the order in which the `use` statements are written in the source files.** I personally don't see any situations where this could be a problem, but this might need some thought. I passed `rpass` and `cfail` tests on my computer, and now am compiling a full stage2 compiler to ensure the crates reporting errors in my previous attempts still build correctly. I guess once I have checked it, this will need a crater run? Fixes #18083. r? @alexcrichton , cc @nrc @brson
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
(This is a second try at #26242. This time I think things should be ok.)
The current algorithm handling import resolutions works sequentially, handling imports in the order they appear in the source file, and blocking/bailing on the first one generating an error/being unresolved.
This can lead to situations where the order of the
usestatements can make the difference between "this code compiles" and "this code fails on an unresolved import" (see #18083 for example). This is especially true when considering glob imports.This PR changes the behaviour of the algorithm to instead try to resolve all imports in a module. If one fails, it is recorded and the next one is tried (instead of directly giving up). Also, all errors generated are stored (and not reported directly).
The main loop of the algorithms guaranties that the algorithm will always finish: if a round of resolution does not resolve anything new, we are stuck and give up. At this point, the new version of the algorithm will display all errors generated by the last round of resolve. This way we are sure to not silence relevant errors or help messages, but also to not give up too early.
As a consequence, the import resolution becomes independent of the order in which the
usestatements are written in the source files. I personally don't see any situations where this could be a problem, but this might need some thought.I passed
rpassandcfailtests on my computer, and now am compiling a full stage2 compiler to ensure the crates reporting errors in my previous attempts still build correctly. I guess once I have checked it, this will need a crater run?Fixes #18083.
r? @alexcrichton , cc @nrc @brson