fix: Off-by-one in lifetime binders when lowering dyn Trait<'a> - #23107
Merged
ChayimFriedman2 merged 1 commit intoAug 11, 2026
Merged
Conversation
rustbot
added
the
S-waiting-on-review
Contributor
Author
|
Relevant code snippet that triggered the panic: |
Contributor
|
I'm hesitant to merge this since I fear this will collide with the lifetime elision work. CC @dfireBird. |
Wilfred
commented
Aug 10, 2026
| let db = self.db; | ||
| self.lower_type_bound(b, dummy_self_ty, false).for_each(|(b, _)| { | ||
| let clauses: Vec<_> = match b { | ||
| TypeBound::Path(_, TraitBoundModifier::None) => { |
Contributor
Author
There was a problem hiding this comment.
If I'm understanding #22927 correctly, I think this can change to TypeBound::Path(None, _, TraitBoundModifier::None) => if that PR merges first. I think the PRs are strictly complementary.
Member
|
I think this would be fine to merge. Thanks for catching this. |
ChayimFriedman2
requested changes
Aug 10, 2026
| TypeBound::Path(_, TraitBoundModifier::None) => { | ||
| // `dyn Trait<'a>` is an existential predicate that introduces a binder. | ||
| self.with_shifted_in(&[], |ctx| { | ||
| ctx.lower_type_bound(b, dummy_self_ty, false).collect() |
Contributor
There was a problem hiding this comment.
I don't like the extra collect(). You can avoid it by extracting the for_each into a named closure and doing it in both arms,
Contributor
Author
There was a problem hiding this comment.
Refactored to a match that returns an iterator.
`dyn Trait<'a>` does not have an explicit `for<'a>`, but it still has a binder. If we don't call with_shifted_in(), we end up with lifetimes whose de Bruijn index points to the wrong level. This could cause r-a to refer to the wrong lifetime variable, or even panic when the enclosing binder doesn't declare a lifetime variable. See the new unit test. This bug meant r-a would previously crash when generating a SCIP file for the dachshund crate. AI disclosure: Code partly written by GPT-5, all comments, commit messages and mistakes are mine.
Wilfred
force-pushed
the
fix-empty-bound-vars-binder
branch
from
August 11, 2026 10:41
a59cd0b to
5c18792
Compare
ChayimFriedman2
approved these changes
Aug 11, 2026
rustbot
removed
the
S-waiting-on-review
This was referenced Aug 11, 2026
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
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.
dyn Trait<'a>does not have an explicitfor<'a>, but it still has a binder. If we don't call with_shifted_in(), we end up with lifetimes whose de Bruijn index points to the wrong level.This could cause r-a to refer to the wrong lifetime variable, or even panic when the enclosing binder doesn't declare a lifetime variable. See the new unit test.
This bug meant r-a would previously crash when generating a SCIP file for the dachshund crate.
AI disclosure: Code partly written by GPT-5, all comments, commit messages and mistakes are mine.