Skip to content

Commit

Permalink
Rollup merge of #111745 - Badel2:emitter-add-overflow, r=compiler-errors
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
matthiaskrgr committed May 21, 2023
2 parents d77014a + cbb4100 commit cb5dd1d
Show file tree
Hide file tree
Showing 6 changed files with 108 additions and 22 deletions.
35 changes: 19 additions & 16 deletions compiler/rustc_errors/src/emitter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2303,22 +2303,25 @@ impl EmitterWriter {

// Colorize addition/replacements with green.
for &SubstitutionHighlight { start, end } in highlight_parts {
// Account for tabs when highlighting (#87972).
let tabs: usize = line_to_add
.chars()
.take(start)
.map(|ch| match ch {
'\t' => 3,
_ => 0,
})
.sum();
buffer.set_style_range(
*row_num,
max_line_num_len + 3 + start + tabs,
max_line_num_len + 3 + end + tabs,
Style::Addition,
true,
);
// This is a no-op for empty ranges
if start != end {
// Account for tabs when highlighting (#87972).
let tabs: usize = line_to_add
.chars()
.take(start)
.map(|ch| match ch {
'\t' => 3,
_ => 0,
})
.sum();
buffer.set_style_range(
*row_num,
max_line_num_len + 3 + start + tabs,
max_line_num_len + 3 + end + tabs,
Style::Addition,
true,
);
}
}
*row_num += 1;
}
Expand Down
11 changes: 5 additions & 6 deletions compiler/rustc_errors/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -330,12 +330,11 @@ impl CodeSuggestion {
});
buf.push_str(&part.snippet);
let cur_hi = sm.lookup_char_pos(part.span.hi());
if cur_hi.line == cur_lo.line && !part.snippet.is_empty() {
// Account for the difference between the width of the current code and the
// snippet being suggested, so that the *later* suggestions are correctly
// aligned on the screen.
acc += len - (cur_hi.col.0 - cur_lo.col.0) as isize;
}
// Account for the difference between the width of the current code and the
// snippet being suggested, so that the *later* suggestions are correctly
// aligned on the screen. Note that cur_hi and cur_lo can be on different
// lines, so cur_hi.col can be smaller than cur_lo.col
acc += len - (cur_hi.col.0 as isize - cur_lo.col.0 as isize);
prev_hi = cur_hi;
prev_line = sf.get_line(prev_hi.line - 1);
for line in part.snippet.split('\n').skip(1) {
Expand Down
12 changes: 12 additions & 0 deletions tests/ui/suggestions/issue-109854.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
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#""#,
);
}

fn main() {}
31 changes: 31 additions & 0 deletions tests/ui/suggestions/issue-109854.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
error[E0061]: this function takes 1 argument but 3 arguments were supplied
--> $DIR/issue-109854.rs:2:5
|
LL | String::with_capacity(
| ^^^^^^^^^^^^^^^^^^^^^
...
LL | / r#"
LL | | pub(crate) struct Person<T: Clone> {}
LL | | "#,
| |__- unexpected argument of type `&'static str`
LL | r#""#,
| ----- unexpected argument of type `&'static str`
|
note: expected `usize`, found fn item
--> $DIR/issue-109854.rs:4:5
|
LL | generate_setter,
| ^^^^^^^^^^^^^^^
= note: expected type `usize`
found fn item `fn() {generate_setter}`
note: associated function defined here
--> $SRC_DIR/alloc/src/string.rs:LL:COL
help: remove the extra arguments
|
LL - generate_setter,
LL + /* usize */,
|

error: aborting due to previous error

For more information about this error, try `rustc --explain E0061`.
5 changes: 5 additions & 0 deletions tests/ui/suggestions/issue-94171.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
fn L(]{match
(; {`
//~^^ ERROR mismatched closing delimiter
//~^^ ERROR unknown start of token
//~ ERROR this file contains an unclosed delimiter
36 changes: 36 additions & 0 deletions tests/ui/suggestions/issue-94171.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
error: unknown start of token: `
--> $DIR/issue-94171.rs:2:5
|
LL | (; {`
| ^
|
help: Unicode character '`' (Grave Accent) looks like ''' (Single Quote), but it is not
|
LL | (; {'
| ~

error: mismatched closing delimiter: `]`
--> $DIR/issue-94171.rs:1:5
|
LL | fn L(]{match
| ^^ mismatched closing delimiter
| |
| unclosed delimiter

error: this file contains an unclosed delimiter
--> $DIR/issue-94171.rs:5:52
|
LL | fn L(]{match
| -- unclosed delimiter
| |
| missing open `[` for this delimiter
LL | (; {`
| - - unclosed delimiter
| |
| unclosed delimiter
...
LL |
| ^

error: aborting due to 3 previous errors

0 comments on commit cb5dd1d

Please sign in to comment.