Skip to content

Conversation

bvanjoi
Copy link
Contributor

@bvanjoi bvanjoi commented Jun 29, 2023

Fixes #85992

Why #85992 panic

During resolve_imports, the path_res of the import issue_85992_extern_2::Outcome is pointing to external::issue_85992_extern_2 instead of crate::issue_85992_extern_2. As a result import.imported_module.set had been executed.

Attached 1: the state of early_resolve_ident_in_lexical_scope during the resolve_imports for use issue_85992_extern_2::Outcome is as follows:

iter in visit_scopes scope result.binding
init - -
0 CrateRoot Err(Determined)
1 ExternPrelude pointing to the issue_85992_extern_2(external)

However, during finalization for issue_85992_extern_2::Outcome, the innermost_result was pointed to crate::issue_85992_extern_2 and no ambiguity was generated, leading to a panic.

Attached 2: the state of early_resolve_ident_in_lexical_scope during the finalize_import for use issue_85992_extern_2::Outcome is as follows:

iter in visit_scopes scope result.binding innermost_result
init - - None
0 CrateRoot pointing to use crate::issue_85992_extern_2 (introdcued by dummy) same as result but with a Some wapper
1 ExternPrelude pointing to the issue_85992_extern_2(external) smae as above

Try to solve

Skip assertion judgment when NonModule is dummy

r? @petrochenkov

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jun 29, 2023
@petrochenkov
Copy link
Contributor

That assert is too strict, IMO.

If a dummy binding can get in the way during finalize_import, then it can be relaxed to

            PathResult::NonModule(partial_res) => {
                if no_ambiguity && partial_res.full_res() != Some(Res::Err) {
                    assert!(import.imported_module.get().is_none());
                }

Changing the early_resolve_ident_in_lexical_scope infra can have more far reaching consequences, and I don't really want to think about them if the issue can be fixed in a more local way.
@rustbot author

@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 30, 2023
@bvanjoi bvanjoi changed the title fix(resolve): only use valid binding for innermost_result fix(resolve): skip assertion judgment when NonModule is dummy Jul 1, 2023
@bvanjoi
Copy link
Contributor Author

bvanjoi commented Jul 1, 2023

@rustbot ready

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jul 1, 2023
@petrochenkov
Copy link
Contributor

Thanks!
@bors r+ rollup

@bors
Copy link
Collaborator

bors commented Jul 1, 2023

📌 Commit 549f48d has been approved by petrochenkov

It is now in the queue for this repository.

@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 Jul 1, 2023
bors added a commit to rust-lang-ci/rust that referenced this pull request Jul 1, 2023
…iaskrgr

Rollup of 6 pull requests

Successful merges:

 - rust-lang#113168 (fix(resolve): skip assertion judgment when NonModule is dummy)
 - rust-lang#113174 (Better messages for next on a iterator inside for loops)
 - rust-lang#113182 (Error when RPITITs' hidden types capture more lifetimes than their trait definitions)
 - rust-lang#113196 (Fix associated items effective visibility calculation for type privacy lints)
 - rust-lang#113226 (Fix try builds on the msvc builder)
 - rust-lang#113227 (Update cargo)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit b63bc06 into rust-lang:master Jul 1, 2023
@rustbot rustbot added this to the 1.72.0 milestone Jul 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Assertion failed: import.imported_module.get().is_none()
4 participants