Skip to content

Commit cb5dd1d

Browse files
authored
Rollup merge of #111745 - Badel2:emitter-add-overflow, r=compiler-errors
Fix overflow in error emitter Fix #109854 Close #94171 (was already fixed before but missing test) This bug happens when a multipart suggestion spans more than one line. The fix is to update the `acc` variable, which didn't handle the case when the text to remove spans multiple lines but the text to add spans only one line. Also, use `usize::try_from` instead of `as usize` to detect overflows earlier in the future, and point to the source of the overflow (the original issue points to a different place where this value is used, not where the overflow had happened). And finally add an `if start != end` check to avoid doing any extra work in case of empty ranges. Long explanation: Given this test case: ```rust fn generate_setter() { String::with_capacity( //~^ ERROR this function takes 1 argument but 3 arguments were supplied generate_setter, r#" pub(crate) struct Person<T: Clone> {} "#, r#""#, ); } ``` The compiler will try to convert that code into the following: ```rust fn generate_setter() { String::with_capacity( //~^ ERROR this function takes 1 argument but 3 arguments were supplied /* usize */, ); } ``` So it creates a suggestion with 3 separate parts: ``` // Replace "generate_setter" with "/* usize */" SubstitutionPart { span: fuzz_input.rs:4:5: 4:20 (#0), snippet: "/* usize */" } // Remove second arg (multiline string) SubstitutionPart { span: fuzz_input.rs:4:20: 7:3 (#0), snippet: "" } // Remove third arg (r#""#) SubstitutionPart { span: fuzz_input.rs:7:3: 8:11 (#0), snippet: "" } ``` Each of this parts gets a separate `SubstitutionHighlight` (this marks the relevant text green in a terminal, the values are 0-indexed so `start: 4` means column 5): ``` SubstitutionHighlight { start: 4, end: 15 } SubstitutionHighlight { start: 15, end: 15 } SubstitutionHighlight { start: 18446744073709551614, end: 18446744073709551614 } ``` The 2nd and 3rd suggestion are empty (start = end) because they only remove text, so there are no additions to highlight. But the 3rd span has overflowed because the compiler assumes that the 3rd suggestion is on the same line as the first suggestion. The 2nd span starts at column 20 and the highlight starts at column 16 (15+1), so that suggestion is good. But since the 3rd span starts at column 3, the result is `3 - 4`, or column -1, which turns into -2 with 0-indexed, and that's equivalent to `18446744073709551614 as isize`. With this fix, the resulting `SubstitutionHighlight` are: ``` SubstitutionHighlight { start: 4, end: 15 } SubstitutionHighlight { start: 15, end: 15 } SubstitutionHighlight { start: 15, end: 15 } ``` As expected. I guess ideally we shouldn't emit empty highlights when removing text, but I am too scared to change that.
2 parents d77014a + cbb4100 commit cb5dd1d

File tree

6 files changed

+108
-22
lines changed

6 files changed

+108
-22
lines changed

Diff for: compiler/rustc_errors/src/emitter.rs

+19-16
Original file line numberDiff line numberDiff line change
@@ -2303,22 +2303,25 @@ impl EmitterWriter {
23032303

23042304
// Colorize addition/replacements with green.
23052305
for &SubstitutionHighlight { start, end } in highlight_parts {
2306-
// Account for tabs when highlighting (#87972).
2307-
let tabs: usize = line_to_add
2308-
.chars()
2309-
.take(start)
2310-
.map(|ch| match ch {
2311-
'\t' => 3,
2312-
_ => 0,
2313-
})
2314-
.sum();
2315-
buffer.set_style_range(
2316-
*row_num,
2317-
max_line_num_len + 3 + start + tabs,
2318-
max_line_num_len + 3 + end + tabs,
2319-
Style::Addition,
2320-
true,
2321-
);
2306+
// This is a no-op for empty ranges
2307+
if start != end {
2308+
// Account for tabs when highlighting (#87972).
2309+
let tabs: usize = line_to_add
2310+
.chars()
2311+
.take(start)
2312+
.map(|ch| match ch {
2313+
'\t' => 3,
2314+
_ => 0,
2315+
})
2316+
.sum();
2317+
buffer.set_style_range(
2318+
*row_num,
2319+
max_line_num_len + 3 + start + tabs,
2320+
max_line_num_len + 3 + end + tabs,
2321+
Style::Addition,
2322+
true,
2323+
);
2324+
}
23222325
}
23232326
*row_num += 1;
23242327
}

Diff for: compiler/rustc_errors/src/lib.rs

+5-6
Original file line numberDiff line numberDiff line change
@@ -330,12 +330,11 @@ impl CodeSuggestion {
330330
});
331331
buf.push_str(&part.snippet);
332332
let cur_hi = sm.lookup_char_pos(part.span.hi());
333-
if cur_hi.line == cur_lo.line && !part.snippet.is_empty() {
334-
// Account for the difference between the width of the current code and the
335-
// snippet being suggested, so that the *later* suggestions are correctly
336-
// aligned on the screen.
337-
acc += len - (cur_hi.col.0 - cur_lo.col.0) as isize;
338-
}
333+
// Account for the difference between the width of the current code and the
334+
// snippet being suggested, so that the *later* suggestions are correctly
335+
// aligned on the screen. Note that cur_hi and cur_lo can be on different
336+
// lines, so cur_hi.col can be smaller than cur_lo.col
337+
acc += len - (cur_hi.col.0 as isize - cur_lo.col.0 as isize);
339338
prev_hi = cur_hi;
340339
prev_line = sf.get_line(prev_hi.line - 1);
341340
for line in part.snippet.split('\n').skip(1) {

Diff for: tests/ui/suggestions/issue-109854.rs

+12
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
fn generate_setter() {
2+
String::with_capacity(
3+
//~^ ERROR this function takes 1 argument but 3 arguments were supplied
4+
generate_setter,
5+
r#"
6+
pub(crate) struct Person<T: Clone> {}
7+
"#,
8+
r#""#,
9+
);
10+
}
11+
12+
fn main() {}

Diff for: tests/ui/suggestions/issue-109854.stderr

+31
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
error[E0061]: this function takes 1 argument but 3 arguments were supplied
2+
--> $DIR/issue-109854.rs:2:5
3+
|
4+
LL | String::with_capacity(
5+
| ^^^^^^^^^^^^^^^^^^^^^
6+
...
7+
LL | / r#"
8+
LL | | pub(crate) struct Person<T: Clone> {}
9+
LL | | "#,
10+
| |__- unexpected argument of type `&'static str`
11+
LL | r#""#,
12+
| ----- unexpected argument of type `&'static str`
13+
|
14+
note: expected `usize`, found fn item
15+
--> $DIR/issue-109854.rs:4:5
16+
|
17+
LL | generate_setter,
18+
| ^^^^^^^^^^^^^^^
19+
= note: expected type `usize`
20+
found fn item `fn() {generate_setter}`
21+
note: associated function defined here
22+
--> $SRC_DIR/alloc/src/string.rs:LL:COL
23+
help: remove the extra arguments
24+
|
25+
LL - generate_setter,
26+
LL + /* usize */,
27+
|
28+
29+
error: aborting due to previous error
30+
31+
For more information about this error, try `rustc --explain E0061`.

Diff for: tests/ui/suggestions/issue-94171.rs

+5
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
fn L(]{match
2+
(; {`
3+
//~^^ ERROR mismatched closing delimiter
4+
//~^^ ERROR unknown start of token
5+
//~ ERROR this file contains an unclosed delimiter

Diff for: tests/ui/suggestions/issue-94171.stderr

+36
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
error: unknown start of token: `
2+
--> $DIR/issue-94171.rs:2:5
3+
|
4+
LL | (; {`
5+
| ^
6+
|
7+
help: Unicode character '`' (Grave Accent) looks like ''' (Single Quote), but it is not
8+
|
9+
LL | (; {'
10+
| ~
11+
12+
error: mismatched closing delimiter: `]`
13+
--> $DIR/issue-94171.rs:1:5
14+
|
15+
LL | fn L(]{match
16+
| ^^ mismatched closing delimiter
17+
| |
18+
| unclosed delimiter
19+
20+
error: this file contains an unclosed delimiter
21+
--> $DIR/issue-94171.rs:5:52
22+
|
23+
LL | fn L(]{match
24+
| -- unclosed delimiter
25+
| |
26+
| missing open `[` for this delimiter
27+
LL | (; {`
28+
| - - unclosed delimiter
29+
| |
30+
| unclosed delimiter
31+
...
32+
LL |
33+
| ^
34+
35+
error: aborting due to 3 previous errors
36+

0 commit comments

Comments
 (0)