fix: flatten nested HRTB predicate binders - #23139
Conversation
rustbot
added
the
S-waiting-on-review
8796ae2 to
d0a84a4
Compare
|
Now, lifetime search is changed to preferentially match the last name added in the same binder, correctly handle nested lifetime shadowing, and remove the previously incorrect index offset. |
| ); | ||
| } | ||
|
|
||
| #[test] |
There was a problem hiding this comment.
The tests shouldn't be in this file, rather in regression.rs.
| } | ||
|
|
||
| #[test] | ||
| fn nested_hrtb_starts_own_predicate_binder() { |
There was a problem hiding this comment.
This test crosses the border into testing implementation details, I don't think we should have it, though not so sure.
| if let LifetimeRef::Named(lt_name) = &self.store[lifetime] { | ||
| self.bound_vars.iter().rev().enumerate().find_map(|(debruijn, (binder, _))| { | ||
| binder.iter().enumerate().find_map(|(index, l)| { | ||
| binder.iter().enumerate().rev().find_map(|(index, l)| { |
There was a problem hiding this comment.
Why this rev()?
| names.truncate(old_names_len); | ||
| *bound_vars = old_bound_vars; |
There was a problem hiding this comment.
Pretty sure this is wrong and maybe can cause problems. Why do you truncate here?
There was a problem hiding this comment.
hum...The original plan was:truncate was originally used to restore the parent binder after a generate clause, preventing inner HRTB variables from leaking into sibling bounds.
It feels better to construct a complete flattened binder, temporarily replacing the current binder entry now...
I will fix the above problem together
There was a problem hiding this comment.
What's "a sibling bound"? At this level of lowering there is no such thing.
There was a problem hiding this comment.
It should said: lower_type_bound should remain state-neutral when returning.🤔
There was a problem hiding this comment.
Because the concrete caller is PathLoweringContext::assoc_type_bindings_from_type_bound, the iterates over binding.bounds and invokes lower_type_bound multiple times using the same TyLoweringContext.like:
T: Outer<
Assoc: for<'a> A<'a> + for<'b> B<'b>
>Each for<...> only scopes its own TypeBound.
If the binder entry is not restored after lowering A, the context still contains 'a when lowering B.
And, B is lowered with ['a, 'b], and 'b receives index 1, while the correct binder is only ['b], with 'b at index 0.
A local trace with Assoc: for<'a> A<'a> + B confirmed the contamination like：
with restoration: A = ['a], B = []
without restoration: A = ['a], B = ['a]So I think restoring the previous entry is required to preserve the lexical scope of each TypeBound🤔
But there seems also have a problem now, because currently I only create an empty predicate frame when the top-level bound itself is ForLifetime.
I used DeepL to translate some of the content, but after reading it over, I think it should match what I intended to say :)
There was a problem hiding this comment.
It's still not correct in this case because the latter bound will override the BoundVarKinds for the earlier bound.
4 participants
两个issue的问题都在于关联类型 bound 递归 lowering 时，clause binder 只记录最内层 lifetime，引用的外层 lifetime 以 ^1/^2 形式逃逸。因此我修复了lower.rs中将 overarching 和内层 HRTB 变量合并到同一个 predicate binder，同时保留dyn Trait了作为真正独立的existential binder。
<The problem is that when the associated type bound is recursively lowering, the clause binder only records the innermost lifetime, and the outer lifetime of the reference escapes in the form of ^1/^2.
So I fix merging overarching and inner HRTB variables into the same predicate binder in lower.rs while retaining the dyn Trait's truly independent existential binder.>
In simpler terms: Outer lifetimes in nested binders are not flattened/renumbered correctly
Fix: #23063 and Fix #23064