Coherence error messages can be inscrutable

Opened by Niko Matsakis at 2023-08-03 16:28:23

In particular, when the overlap check fails, it doesn't give much clarity, particularly when the overlap is between impls in different crates. It should tell you:

Precisely what type overlaps
For each case of negative reasoning it was not able to make, why it was not able to make it. #2 might be a bit tricky to do, not sure, but we can certainly improve on current state of affairs.

triage: P-high (1.0)
Niko Matsakis at 2015-04-02 20:48:44
@FlaPer87 is going to look into this as well. As a concrete first step, we want to print out the overlapping types, and not just cite impls. For example:
```
// crate A
trait Foo { }
impl<T> Foo for &T { }

// crate B
struct MyType;
impl Foo for &MyType { } // X
```
This would ideally print something like:
- Multiple impls can be applied to &MyType
  - First candidate is impl X, on line 123
  - Second candidate is in crate A: impl<T> Foo for &T
However, the last part is a bit tricky since the impl must be reconstructed from metadata, but we can start with even just printing &MyType. It's "obvious" from the impl decl (sort of...) once you know what's going on, but helpful to see it printed when you don't.
Niko Matsakis at 2015-04-03 10:29:43
I'm encouraging @aturon to take this on :)
Niko Matsakis at 2015-11-06 20:25:26
I still consider the error messages fairly inscrutable, even with @aturon's commit.
Niko Matsakis at 2015-12-22 18:05:17
In particular, I think we should tell you where we failed to use negative reasoning because of orphan rules.
Niko Matsakis at 2015-12-22 18:05:51
@nikomatsakis Whoops, sorry for the premature closure. I'd be interested in continuing to work in this space, but we should talk about the end goal.
Aaron Turon at 2015-12-22 18:07:58
@aturon basically I think we should report each piece of negative reasoning that we were not able to use because of orphan rules. Bonus points if we limit it to negative reasoning that would have helped.

I think the "motivating example" form this thread is interesting -- the thread opens with an explanation of the error, it seems like coherence could probably have produced something much like that.

Right now the example gives you a counter-example like:

conflicting implementations of trait FromSqlRow<(_, _), _> for type (_, _)

which is definitely a good start. But I think it'd be great if it could add:
the danger is that another crate might write an impl like:
```
impl FromSql<(i32, i32), MyLocalType> for (i32, i32)
```
which would induce overlap.
Niko Matsakis at 2016-01-25 16:31:19
http://stackoverflow.com/q/37347311/1256624 is a "real world" example where confusion was caused by being overly definite in the error message rather than explaining that it was a possibility/what the actual risk is.
Huon Wilson at 2016-05-20 13:21:57

Current output:

error[E0119]: conflicting implementations of trait `FromSqlRow<(_, _), _>` for type `(_, _)`
 --> src/main.rs:9:1
  |
5 | impl<T, ST, DB> FromSqlRow<ST, DB> for T where
  | ---------------------------------------- first implementation here
...
9 | impl<A, B, TA, TB, DB> FromSqlRow<(TA, TB), DB> for (A, B) where
  | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ conflicting implementation for `(_, _)`
  |
  = note: downstream crates may implement trait `FromSql<(_, _), _>` for type `(_, _)`

error[E0119]: conflicting implementations of trait `From<MyError<_>>` for type `MyError<_>`
 --> src/main.rs:9:1
  |
9 | impl<S: Storage> From<S::Error> for MyError<S> {
  | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  |
  = note: conflicting implementation in crate `core`:
          - impl<T> From<T> for T;
          - ```

Esteban Kuber at 2023-08-03 16:28:23