Ignore own item bounds in parent alias types in for_each_item_bound
#120942
Conversation
rustbot
added
S-waiting-on-review
|
whoops r? lcnr |
|
For the record, I understand that this approach also doesn't fix the new trait solver, which still suffers this problem. Didn't want to make a change there until we decide what the best approach for this problem actually is. |
|
I think we can go with the same approach in the new solver. Given that when assembling item bounds for @bors r+ rollup=never |
bors
added
S-waiting-on-bors
|
(this should partially revert the perf regression in #120862) |
Ignore own item bounds in parent alias types in `for_each_item_bound` Fixes rust-lang#120912 I want to get a vibe check on this approach, which is very obviously a hack, but I believe something that is forwards-compatible with a more thorough solution and "good enough for now". The problem here is that for a really deep rigid associated type, we are now repeatedly considering unrelated item bounds from the parent alias types, meaning we're doing a *lot* of extra work in the MIR inliner for deeply substituted rigid projections. This feels intimately related to rust-lang#107614. In that PR, we split *supertrait* bounds (bound which share the same `Self` type as the predicate which is being elaborated) and *implied* bounds (anything that is implied by elaborating the predicate). The problem here is related to the fact that we don't maintain the split between these two for `item_bounds`. If we did, then when recursing into a parent alias type, we'd want to consider only the bounds that are given by [`PredicateFilter::All`](https://doc.rust-lang.org/nightly/nightly-rustc/rustc_hir_analysis/astconv/enum.PredicateFilter.html#variant.SelfOnly) **except** those given by [`PredicateFilter::SelfOnly`](https://doc.rust-lang.org/nightly/nightly-rustc/rustc_hir_analysis/astconv/enum.PredicateFilter.html#variant.SelfOnly).
|
@bors retry |
|
☀️ Test successful - checks-actions |
|
Finished benchmarking commit (37b6533): comparison URL. Overall result: no relevant changes - no action needed@rustbot label: -perf-regression Instruction countThis benchmark run did not return any relevant results for this metric. Max RSS (memory usage)This benchmark run did not return any relevant results for this metric. CyclesResultsThis is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
Binary sizeThis benchmark run did not return any relevant results for this metric. Bootstrap: 628.588s -> 631.732s (0.50%) |
Fixes #120912
I want to get a vibe check on this approach, which is very obviously a hack, but I believe something that is forwards-compatible with a more thorough solution and "good enough for now".
The problem here is that for a really deep rigid associated type, we are now repeatedly considering unrelated item bounds from the parent alias types, meaning we're doing a lot of extra work in the MIR inliner for deeply substituted rigid projections.
This feels intimately related to #107614. In that PR, we split supertrait bounds (bound which share the same
Selftype as the predicate which is being elaborated) and implied bounds (anything that is implied by elaborating the predicate).The problem here is related to the fact that we don't maintain the split between these two for
item_bounds. If we did, then when recursing into a parent alias type, we'd want to consider only the bounds that are given byPredicateFilter::Allexcept those given byPredicateFilter::SelfOnly.