Fix overflow in error emitter

[Badel2]

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:

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:

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.

[rustbot]

r? @compiler-errors

(rustbot has picked a reviewer for you, use r? to override)

[compiler-errors]

Can this be simplified to len + cur_lo - cur_hi? maybe that's conceptually not simpler, idk.

[Badel2]

Not sure, this could be acc += len - col_diff, but it doesn't seem much cleaner to introduce a new variable. So I'll leave it like it is.

[Badel2]

I already found the first bug thanks to the usize::try_from, but I will revert that change because it may introduce too much noise. That's because the integer overflow I am trying to solve can only be seen with debug assertions, and usize::try_from().unwrap() will trigger even in normal builds.

This test case:

fn main() {
    let a = 0.le('r',3.+0,3.+0,                      3);
}

Has an overflow because the spans overlap. The error message is mostly correct:

error[E0061]: this method takes 1 argument but 4 arguments were supplied
 --> fuzz_input.rs:2:15
  |
2 |     let a = 0.le('r',3.+0,3.+0,                      3);
  |               ^^ ---      ----                       - unexpected argument of type `{integer}`
  |                  |        |
  |                  |        unexpected argument
  |                  unexpected argument of type `char`
  |
2

help: remove the extra arguments
  |
2 -     let a = 0.le('r',3.+0,3.+0,                      3);
2 +     let a = 0.le(,3.+0);
  |

But these are the spans:

SubstitutionPart { span: fuzz_input.rs:2:18: 2:21 (#0), snippet: "" }
SubstitutionPart { span: fuzz_input.rs:2:26: 2:54 (#0), snippet: "" }
SubstitutionPart { span: fuzz_input.rs:2:31: 2:55 (#0), snippet: "" }

SubstitutionHighlight { start: 17, end: 17 }
SubstitutionHighlight { start: 22, end: 22 }
SubstitutionHighlight { start: 18446744073709551615, end: 18446744073709551615 }

Note that this bug is already present in master, it is not caused by the changes in this PR. The spans overlap because they get expanded in the "delete next comma" logic from here:

rust/compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs

Line 954 in 17a6810

span = span.until(next);

Here are some logs that show how the spans are modified inside that function:

span: fuzz_input.rs:2:18: 2:21 (#0)
Include previous comma: expanded fuzz_input.rs:2:54: 2:55 (#0) to fuzz_input.rs:2:31: 2:55 (#0)
span: fuzz_input.rs:2:31: 2:55 (#0)
Include previous comma: expanded fuzz_input.rs:2:27: 2:31 (#0) to fuzz_input.rs:2:26: 2:31 (#0)
Delete next comma: expanded fuzz_input.rs:2:26: 2:31 (#0) to fuzz_input.rs:2:26: 2:54 (#0)
span: fuzz_input.rs:2:26: 2:54 (#0)

I tried to fix it by removing the "delete next comma" logic, and that fixes the overflow, but it breaks other tests.

[compiler-errors]

Please squash this into one commit. No use in keeping the revert in the history.

[compiler-errors]

Thanks

@bors r+

[bors]

📌 Commit cbb4100 has been approved by compiler-errors

It is now in the queue for this repository.