Skip to content

Commit c9f2482

Browse files
committed
Auto merge of rust-lang#12493 - y21:issue12491, r=Alexendoo
fix span calculation for non-ascii in `needless_return` Fixes rust-lang#12491 Probably fixes rust-lang#12328 as well, but that one has no reproducer, so 🤷 The bug was that the lint used `rfind()` for finding the byte index of the start of the previous non-whitespace character: ``` // abc\n return; ^ ``` ... then subtracting one to get the byte index of the actual whitespace (the `\n` here). (Subtracting instead of adding because it treats this as the length from the `return` token to the `\n`) That's correct for ascii, like here, and will get us to the `\n`, however for non ascii, the `c` could be multiple bytes wide, which would put us in the middle of a codepoint if we simply subtract 1 and is what caused the ICE. There's probably a lot of ways we could fix this. This PR changes it to iterate backwards using bytes instead of characters, so that when `rposition()` finally finds a non-whitespace byte, we *know* that we've skipped exactly 1 byte. This was *probably*(?) what the code was intending to do changelog: Fix ICE in [`needless_return`] when previous line end in a non-ascii character
2 parents 5a11fef + d3f8f3e commit c9f2482

File tree

4 files changed

+36
-2
lines changed

4 files changed

+36
-2
lines changed

clippy_lints/src/returns.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -465,8 +465,8 @@ fn last_statement_borrows<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>)
465465
// Go backwards while encountering whitespace and extend the given Span to that point.
466466
fn extend_span_to_previous_non_ws(cx: &LateContext<'_>, sp: Span) -> Span {
467467
if let Ok(prev_source) = cx.sess().source_map().span_to_prev_source(sp) {
468-
let ws = [' ', '\t', '\n'];
469-
if let Some(non_ws_pos) = prev_source.rfind(|c| !ws.contains(&c)) {
468+
let ws = [b' ', b'\t', b'\n'];
469+
if let Some(non_ws_pos) = prev_source.bytes().rposition(|c| !ws.contains(&c)) {
470470
let len = prev_source.len() - non_ws_pos - 1;
471471
return sp.with_lo(sp.lo() - BytePos::from_usize(len));
472472
}

tests/ui/crashes/ice-12491.fixed

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
#![warn(clippy::needless_return)]
2+
3+
fn main() {
4+
if (true) {
5+
// anything一些中文
6+
}
7+
}

tests/ui/crashes/ice-12491.rs

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
#![warn(clippy::needless_return)]
2+
3+
fn main() {
4+
if (true) {
5+
// anything一些中文
6+
return;
7+
}
8+
}

tests/ui/crashes/ice-12491.stderr

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
error: unneeded `return` statement
2+
--> tests/ui/crashes/ice-12491.rs:5:24
3+
|
4+
LL | // anything一些中文
5+
| ____________________________^
6+
LL | | return;
7+
| |______________^
8+
|
9+
= note: `-D clippy::needless-return` implied by `-D warnings`
10+
= help: to override `-D warnings` add `#[allow(clippy::needless_return)]`
11+
help: remove `return`
12+
|
13+
LL - // anything一些中文
14+
LL - return;
15+
LL + // anything一些中文
16+
|
17+
18+
error: aborting due to 1 previous error
19+

0 commit comments

Comments
 (0)