fix: Prevent panic in Remove Unused Parameter assist #12789
Conversation
|
As a style nit, I'm not really a fan of More importantly, it's somewhat inefficient for a largish number of ranges. I think there's a more efficient way to implement this by sorting the interval endpoints and keeping a list of the open ones, but I don't know if it's worth the complexity. On the third hand, I'm pretty sure someone will post a horrible file with thousands of function invocations, not unlike the 13 MB expression in #4500. |
|
Totally fair on the style front, I can re-implement as a for loop. Definitely true that it is somewhat inefficient, but I'm not sure I know how to implement the more efficient version you're describing. Also: Wow! 200K if/else pairs! |
|
Something like https://www.baeldung.com/cs/finding-all-overlapping-intervals#sweep-line-approach. But it's probably not something that should block this. |
DorianListens
force-pushed
the
dscheidt/unused-param-overlapping
branch
from
ce33398 to
0d6222e
Compare
|
@bors delegate+ |
|
✌️ @DorianListens can now approve this pull request |
Instead of calling `builder.delete` for every text range we find with `process_usage`, we now ensure that the ranges do not overlap before removing them. If a range is fully contained by a prior one, it is dropped. fixes rust-lang#12784
DorianListens
force-pushed
the
dscheidt/unused-param-overlapping
branch
from
0d6222e to
ffb6b23
Compare
|
I looked into implementing the sweep line style of finding the overlaps, and I think it's definitely doable. @bors r+ |
|
☀️ Test successful - checks-actions |
Instead of calling
builder.deletefor every text range we find withprocess_usage, we now ensure that the ranges do not overlap before removingthem. If a range is fully contained by a prior one, it is dropped.
fixes #12784