Do not emit invalid suggestion in E0191 when spans overlap #115077
Conversation
|
(rustbot has picked a reviewer for you, use r? to override) |
rustbot
added
S-waiting-on-review
| @@ -597,7 +597,28 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o { | |||
| } | |||
| } | |||
| } | |||
| if !suggestions.is_empty() { | |||
| suggestions.sort_by_key(|&(span, _)| span); | |||
There was a problem hiding this comment.
Are you certain that the suggestion spans will end up sorted in such a way that two non-adjacent keys never overlap? If not, then since this is the error path, I feel like it may just be easier to do the O(N^2) full pairwise comparison here.
There was a problem hiding this comment.
SpanData manually implements Ord on (lo, hi, ctxt). That means that by sorting any two adjacent spans that don't overlap imply that the spans further apart from each other also don't overlap.
There was a problem hiding this comment.
What about spans that overlap only at their end? Like spans that look like: [(1, 5), (2, 3), (4, 5)], where the first and last elements overlap?
There was a problem hiding this comment.
Wait, maybe it doesn't matter, since by construction that would imply any intermediate spans overlap. Idk.
There was a problem hiding this comment.
Your last comment is exactly right.
| let mut overlaps = false; | ||
| let mut sugg = suggestions.iter().peekable(); | ||
| while let Some((span, _)) = sugg.next() { | ||
| let Some((next, _)) = sugg.peek() else { | ||
| break; | ||
| }; | ||
| overlaps |= span.overlaps(*next); | ||
| } |
There was a problem hiding this comment.
I think this loop can be simplified? Will need to be tweaked if the above assumption is not true, though.
| let mut overlaps = false; | |
| let mut sugg = suggestions.iter().peekable(); | |
| while let Some((span, _)) = sugg.next() { | |
| let Some((next, _)) = sugg.peek() else { | |
| break; | |
| }; | |
| overlaps |= span.overlaps(*next); | |
| } | |
| let overlaps = suggestions.windows(2).any(|pair| pair[0].0.overlaps(pair[1].0)); |
|
Please squash this ideally if you apply any of my suggestions. |
|
r=me after confirming whether the assumption about overlapping spans being always sorted adjacently is true or false, then either applying the simplification suggested above or just changing it to an O(N^2) overlap check like: or something like that. |
|
@bors r=compiler-errors |
bors
added
S-waiting-on-bors
…mpiler-errors Rollup of 6 pull requests Successful merges: - rust-lang#114959 (fix rust-lang#113702 emit a proper diagnostic message for unstable lints passed from CLI) - rust-lang#115011 (Warn on elided lifetimes in associated constants (`ELIDED_LIFETIMES_IN_ASSOCIATED_CONSTANT`)) - rust-lang#115077 (Do not emit invalid suggestion in E0191 when spans overlap) - rust-lang#115087 (Add disclaimer on size assertion macro) - rust-lang#115090 (Always use `os-release` rather than `/lib` to detect `NixOS` (bootstrap)) - rust-lang#115101 (triagebot: add dependency licensing pings) r? `@ghost` `@rustbot` modify labels: rollup
Fix #115019.