ICE when attempting to split error message that contains " found" before "expected" into lines

[tormol]

In librustc::session::split_msg_into_multiline().

With the error "type mismatch resolving for<'r> <F2 as core::ops::FnOnce<(&'r mut core::option::Option<T>, found_opts::FoundOpt)>>::Output == core::result::Result<(), collections::string::String>: expected type parameter, found ()",

it finds the " found" in " found_opts::" before " found ()".

Then pos1 becomes (182, 191) and pos2= (91, 97), which makes &msg[pos1.1..pos2.0] at line 360 fail.

[durka]

Kind of unfortunate that the formatting is done by splitting on substrings. It seems like it would be better to either include the line breaks in the error message from the start, or to do the substring search before substituting in user input.

[cuviper]

I ran into this using a closure that captured a local called found. I minimized mine to:

fn main() {
    let found = false;
    (0..10).filter(|i| found)
}

If I change that to unfound, the successfully wrapped error is:

found.rs:3:5: 3:32 error: mismatched types:
 expected `()`,
    found `core::iter::Filter<core::ops::Range<_>, [closure@found.rs:3:20: 3:31 unfound:_]>`
(expected (),
    found struct `core::iter::Filter`) [E0308]
found.rs:3     (0..10).filter(|i| unfound)
               ^~~~~~~~~~~~~~~~~~~~~~~~~~~
found.rs:3:5: 3:32 help: run `rustc --explain E0308` to see a detailed explanation
error: aborting due to previous error

[cuviper]

It's hard to tell from out-of-date PR diffs, but I found #19870 (comment) -- it sounds like it started out printing newlines at the error source, then @nrc thought this was fragile and asked for expected/found substring matching.

I don't understand why that was preferred though, as this issue reveals how fraught substring-matching can be. Even if we resolve the ICE of slicing backwards, I fear it may still be imprecise about finding the right "expected" and "found" pieces.

[cuviper]

For example, this isn't an ICE, but it's still wrapped incorrectly:

fn main() {
    let expected = false;
    (0..10).filter(|i| expected)
}

expected.rs:3:5: 3:33 error: mismatched types:
 expected `()`,
    found `core::iter::Filter<core::ops::Range<_>, [closure@expected.rs:3:20: 3:32
 expected:_]>` (expected (),
    found struct `core::iter::Filter`) [E0308]
expected.rs:3     (0..10).filter(|i| expected)
                  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
expected.rs:3:5: 3:33 help: run `rustc --explain E0308` to see a detailed explanation
error: aborting due to previous error

[nrc]

I don't recall what the earlier approach was and why I thought it was fragile. Probably to avoid mixing error formatting with error production. I agree substring matching is a bit gross.

I think the proper fix here is for the error producing code to provide a struct with the expected/found values and then the error handler should format these (i.e., stick in the newlines, etc.). This should be easier now that errors are more structured.

[sophiajt]

With #32756 this is what I see for @cuviper 's examples:

error: mismatched types [--explain E0308]
 --> tmp/quickie.rs:3:5
3 |>     (0..10).filter(|i| expected)
  |>     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected (), found struct `std::iter::Filter`
note: expected type `()`
note:    found type `std::iter::Filter<std::ops::Range<_>, [closure@tmp/quickie.rs:3:20: 3:32 expected:_]>`

error: aborting due to previous error

error: mismatched types [--explain E0308]
 --> tmp/quickie2.rs:3:5
3 |>     (0..10).filter(|i| found)
  |>     ^^^^^^^^^^^^^^^^^^^^^^^^^ expected (), found struct `std::iter::Filter`
note: expected type `()`
note:    found type `std::iter::Filter<std::ops::Range<_>, [closure@tmp/quickie2.rs:3:20: 3:29 found:_]>`

error: aborting due to previous error

error: unresolved name `unfound`. Did you mean `found`? [--explain E0425]
 --> tmp/quickie2.rs:3:24
3 |>     (0..10).filter(|i| unfound)
  |>                        ^^^^^^^

error: mismatched types [--explain E0308]
 --> tmp/quickie2.rs:3:5
3 |>     (0..10).filter(|i| unfound)
  |>     ^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected (), found struct `std::iter::Filter`
note: expected type `()`
note:    found type `std::iter::Filter<std::ops::Range<_>, [closure@tmp/quickie2.rs:3:20: 3:31]>`

[cuviper]

For the unfound case, it should have been changed everywhere so you don't get that "unresolved name". Anyway, it sure looks like they're all splitting correctly - yay!