Stop using identity args for RPIT wf checks #114574
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @TaKO8Ki (or someone else) soon. Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (
|
rustbot
added
S-waiting-on-review
…he args from the single use of a RPIT in its parent function's return type
oli-obk
commented
Aug 7, 2023
Comment on lines
+648
to
652
| ty::Alias(ty::Projection, alias) => { | ||
| if let Some(ty::ImplTraitInTraitData::Trait { fn_def_id, .. }) = self.tcx.opt_rpitit_info(alias.def_id) && fn_def_id == self.fn_def_id { | ||
| self.tcx.type_of(alias.def_id).instantiate(self.tcx, alias.args).visit_with(self)?; | ||
| } | ||
| _ => (), | ||
| } |
There was a problem hiding this comment.
This was removed in #114267, but we would probably have had issues if RPITITs were behind binders or sth. Now that we always do this for every RPIT, there are no more hidden ICEs under rare circumstances, instead we now always find the usage unconditionally and ICE if we don't find it.
This was referenced Aug 7, 2023
|
Superseded by #114602 |
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Aug 8, 2023
…s, r=oli-obk Map RPIT duplicated lifetimes back to fn captured lifetimes Use the [`lifetime_mapping`](https://doc.rust-lang.org/nightly/nightly-rustc/rustc_hir/hir/struct.OpaqueTy.html#structfield.lifetime_mapping) to map an RPIT's captured lifetimes back to the early- or late-bound lifetimes from its parent function. We may be going thru several layers of mapping, since opaques can be nested, so we introduce `TyCtxt::map_rpit_lifetime_to_fn_lifetime` to loop through several opaques worth of mapping, and handle turning it into a `ty::Region` as well. We can then use this instead of the identity substs for RPITs in `check_opaque_meets_bounds` to address rust-lang#114285. We can then also use `map_rpit_lifetime_to_fn_lifetime` to properly install bidirectional-outlives predicates for both RPITs and RPITITs. This addresses rust-lang#114601. I based this on rust-lang#114574, but I don't actually know how much of that PR we still need, so some code may be redundant now... 🤷 --- Fixes rust-lang#114597 Fixes rust-lang#114579 Fixes rust-lang#114285 Also fixes rust-lang#114601, since it turns out we had other bugs with RPITITs and their duplicated lifetime params 😅. Supersedes rust-lang#114574 r? `@oli-obk`
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
and instead load the args from the single use of a RPIT in its parent function's return type
follow up to #113661
fixes #114285
Before this PR, we used the RPIT identity substs for checking its hidden type's bounds. This means various late bound lifetimes from the function now became early bound lifetimes on the RPIT. That worked just fine, until you had nested RPITs that capture lifetimes, too. By accident it mostly worked, unless closures were involved.
While investigating this mess, I realized that we were going about all of this the wrong way around. Instead of trying to patch up the nested opaque types' hidden types, we could also just find the correct generic args of the opaque type that we're checking and not have to patch up anything after the fact.
So now, we walk the return type of the function, find the hidden type within that return type, and take its generic arguments. These generic arguments are in terms of the function signature, so they will have all late bound lifetimes still be in a binder. So we dismiss the binder by replacing all late bound vars in it with new inference vars, and then run the previous checks (minus the patching up, which was now dead code).