[implied_bounds_in_impls]: include (previously omitted) associated types in suggestion
#11459
Conversation
|
r? @blyxyas (rustbot has picked a reviewer for you, use r? to override) |
rustbot
added
the
S-waiting-on-review
blyxyas
changed the title
implied_bounds_in_impls]: include omitted associated types in suggestionimplied_bounds_in_impls]: include (previously omitted) associated types in suggestion
| let implied_span_extended = if let Some(next_bound) = opaque_ty.bounds.get(index + 1) { | ||
| poly_trait.span.to(next_bound.span().shrink_to_lo()) | ||
| } else if index > 0 | ||
| && let Some(prev_bound) = opaque_ty.bounds.get(index - 1) | ||
| { | ||
| prev_bound.span().shrink_to_hi().to(poly_trait.span.shrink_to_hi()) | ||
| } else { | ||
| poly_trait.span | ||
| }; |
There was a problem hiding this comment.
I'm kinda worried about this check, it's kinda inconsistent (one checking index and the other not), but it may not matter if we make sure it doesn't fail.
Could you add a test with 3 bounds? Maybe some with generics and some without them?
There was a problem hiding this comment.
The reason why index - 1 has a check and the other doesn't is mainly that - 1 (I assume you're referring to this) can panic with debug assertions due to overflow if it's the first bound, while + 1 in this context can't.
I mean, a slice with usize::MAX elements (which is needed here for an overflow to occur) is not only realistically impossible because nobody has that much ram but also theoretically impossible, since afaik allocations in rust can never be bigger than isize::MAX.
Only if there's neither an element before or after it (i.e. only one bound) will it not extend the span
|
@bors r+ |
|
☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test |
Fixes #11435
It now includes associated types from the implied bound that were omitted in the second bound. Example:
Suggestion before this change:
It didn't include
<Item = u32>onExactSizeIterator. Now, with this change, it does.We also now extend the span to include not just possible
+ahead of it, but also behind it (an example for this is in the linked issue as well).Note: The overall diff is a bit noisy, because building up the suggestion involves quite a bit more logic now and I decided to extract that into its own function. For that reason, I split this PR up into two commits. The first commit contains the actual "logic" changes. Second commit just moves code around.
changelog: [
implied_bounds_in_impls]: include (previously omitted) associated types in suggestionchangelog: [
implied_bounds_in_impls]: include the+behind bound if it's the last bound