[draft] Revised approach to backport of PR #96970 #97036
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
This carries on with some of the ideas from PR 96970, but generalizes them slightly to try to narrow the cases where the error fires. (It also sidesteps the whole question of trying to deal with lifetime resolution during ast lowering by just looking at names.) More specifically: This tracks the depth of Return Position Opaque Ty (aka Return Position Impl Trait aka RPIT). As it descends the AST, each time it does lower_poly_trait_ref, it tracks what lifetimes are introduced there, and what depth they were introduced at. (As I write this, I realize that I probably didn't handle shadowed lifetimes quite right; see below.) Anyway, when it hits a use of a lifetime, it checks the depth. If the depths don't match and we're in a nested RPIT, then it errors. (Compare this to PR 96970, which just errors unconditionally in the nested RPIT, without checking if the depths match and thus it's a `for<'a>` that was introduced at the current level as well, and thus should not be a problem.) Regarding shadowing mistake: my code probably generates an error in a situation like `impl for<'a> Trait1<Assoc=impl for<'a> Trait2<'a>>`, or something along those lines. But that should be easy to fix.) A follow-up commit will include the blessed test output, so you can see the effect of this and compare it against PR 96970. (I did not put in any new unit tests, but obviously we should make those too.)
… them against PR 96970 and it seems consistent but I have not inspected them carefully.
rustbot
added
the
T-compiler
|
r? @cjgillot (rust-highfive has picked a reviewer for you, use r? to override) |
|
rust-highfive
added
the
S-waiting-on-review
|
Let me mark this as a draft. At the very least, it should add a test of #96194 itself, as well as a test of the variant that @compiler-errors developed (playground) after I started musing about mixing the ordering a bit. |
pnkfelix
changed the title
This comment was marked as resolved.
This comment was marked as resolved.
oli-obk
changed the title
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.
Revised approach to backport of PR #96970
See discussion on zulip