Pre-RFC: Nicer Types In Diagnostics

compiler
21

One more thing I'd like to add: we should also account for private definitions re-exported as public elsewhere. A common-enough pattern is to have some private mod foo::bar that defines Baz and reexport it from mod foo { pub use self::bar::Baz; }. This means that the qualified path we currently present the user is foo::bar::Baz, but the one that they can use is foo::Baz. We should also keep track of these and present the accessible path in errors and disambiguated output, either by presenting

expected type `foo::bar::Baz` (through `foo::Baz`)

or

expected type `foo::Baz` (defined at `foo::bar::Baz`)

I think these four items represent 90% of the complaints with the current output.

19 Likes
22

This is likely out of scope for this RFC, but I wanted to bring it up as a fairly egregious example of how bad this can look. There's some context for this in my stackoverflow question here.

The error message in this case is:

error[E0599]: no method named `into_actor` found for type `Pin<Box<dyn Future<Output = Result<Vec<ResultType>, Box<dyn Error>>> + Send>>` in the current scope
   --> src/app_socket.rs:194:26
    |
194 |                         .into_actor(&self)
    |                          ^^^^^^^^^^ method not found in `Pin<Box<dyn Future<Output = Result<Vec<ResultType>, Box<dyn Error>>> + Send>>`
    |
    = note: the method `into_actor` exists but the following trait bounds were not satisfied:
            `&dyn Future<Output = Result<std::vec::Vec<ResultType>, std::boxed::Box<dyn std::error::Error>>> + Send : WrapFuture<_>`
            `&dyn Future<Output = Result<std::vec::Vec<ResultType>, std::boxed::Box<dyn std::error::Error>>> + Send : WrapStream<_>`
            `&mut dyn Future<Output = Result<std::vec::Vec<ResultType>, std::boxed::Box<dyn std::error::Error>>> + Send : WrapFuture<_>`
            `&mut dyn Future<Output = Result<std::vec::Vec<ResultType>, std::boxed::Box<dyn std::error::Error>>> + Send : WrapStream<_>`
            `&mut Pin<std::boxed::Box<dyn Future<Output = Result<std::vec::Vec<ResultType>, std::boxed::Box<dyn std::error::Error>>> + Send>> : WrapFuture<_>`
            `&mut Pin<std::boxed::Box<dyn Future<Output = Result<std::vec::Vec<ResultType>, std::boxed::Box<dyn std::error::Error>>> + Send>> : WrapStream<_>`
            `&Pin<std::boxed::Box<dyn Future<Output = Result<std::vec::Vec<ResultType>, std::boxed::Box<dyn std::error::Error>>> + Send>> : WrapFuture<_>`
            `&Pin<std::boxed::Box<dyn Future<Output = Result<std::vec::Vec<ResultType>, std::boxed::Box<dyn std::error::Error>>> + Send>> : WrapStream<_>`
            `dyn Future<Output = Result<std::vec::Vec<ResultType>, std::boxed::Box<dyn std::error::Error>>> + Send : WrapFuture<_>`
            `dyn Future<Output = Result<std::vec::Vec<ResultType>, std::boxed::Box<dyn std::error::Error>>> + Send : WrapStream<_>`
            `Pin<std::boxed::Box<dyn Future<Output = Result<std::vec::Vec<ResultType>, std::boxed::Box<dyn std::error::Error>>> + Send>> : WrapFuture<_>`
            `Pin<std::boxed::Box<dyn Future<Output = Result<std::vec::Vec<ResultType>, std::boxed::Box<dyn std::error::Error>>> + Send>> : WrapStream<_>`

I believe I already removed some of the paths here. Your proposal would help a lot, reducing it to something like:

error[E0599]: no method named `into_actor` found for type `Pin<Box<dyn Future<Output = Result<Vec<ResultType>, Box<dyn Error>>> + Send>>` in the current scope
    --> src/app_socket.rs:194:26
    |
194 |                         .into_actor(&self)
    |                          ^^^^^^^^^^ method not found in `Pin<Box<dyn Future<Output = Result<Vec<ResultType>, Box<dyn Error>>> + Send>>`
    |
    = note: the method `into_actor` exists but the following trait bounds were not satisfied:
            `&dyn Future<Output = Result<Vec<ResultType>, Box<dyn Error>>> + Send : WrapFuture<_>`
            `&dyn Future<Output = Result<Vec<ResultType>, Box<dyn Error>>> + Send : WrapStream<_>`
            `&mut dyn Future<Output = Result<Vec<ResultType>, Box<dyn Error>>> + Send : WrapFuture<_>`
            `&mut dyn Future<Output = Result<Vec<ResultType>, Box<dyn Error>>> + Send : WrapStream<_>`
            `&mut Pin<Box<dyn Future<Output = Result<Vec<ResultType>, Box<dyn Error>>> + Send>> : WrapFuture<_>`
            `&mut Pin<Box<dyn Future<Output = Result<Vec<ResultType>, Box<dyn Error>>> + Send>> : WrapStream<_>`
            `&Pin<Box<dyn Future<Output = Result<Vec<ResultType>, Box<dyn Error>>> + Send>> : WrapFuture<_>`
            `&Pin<Box<dyn Future<Output = Result<Vec<ResultType>, Box<dyn Error>>> + Send>> : WrapStream<_>`
            `dyn Future<Output = Result<Vec<ResultType>, Box<dyn Error>>> + Send : WrapFuture<_>`
            `dyn Future<Output = Result<Vec<ResultType>, Box<dyn Error>>> + Send : WrapStream<_>`
            `Pin<Box<dyn Future<Output = Result<Vec<ResultType>, Box<dyn Error>>> + Send>> : WrapFuture<_>`
            `Pin<Box<dyn Future<Output = Result<Vec<ResultType>, Box<dyn Error>>> + Send>> : WrapStream<_>`

But I think this could be even better. There is a large type:

Future<Output = Result<Vec<ResultType>, Box<dyn Error>>>

Which is repeated on every line of the implementation list. If this could be transformed into a named type it could be written something like:

error[E0599]: no method named `into_actor` found for type `Pin<Box<dyn Future<Output = Result<Vec<ResultType>, Box<dyn Error>>> + Send>>` in the current scope
    --> src/app_socket.rs:194:26
    |
194 |                         .into_actor(&self)
    |                          ^^^^^^^^^^ method not found in `Pin<Box<dyn Future<Output = Result<Vec<ResultType>, Box<dyn Error>>> + Send>>`
    |
    = note: the method `into_actor` exists but the following trait bounds were not satisfied:
            `&dyn F + Send : WrapFuture<_>`
            `&dyn F + Send : WrapStream<_>`
            `&mut dyn F + Send : WrapFuture<_>`
            `&mut dyn F + Send : WrapStream<_>`
            `&mut Pin<Box<dyn F + Send>> : WrapFuture<_>`
            `&mut Pin<Box<dyn F + Send>> : WrapStream<_>`
            `&Pin<Box<dyn F + Send>> : WrapFuture<_>`
            `&Pin<Box<dyn F + Send>> : WrapStream<_>`
            `dyn F + Send : WrapFuture<_>`
            `dyn F + Send : WrapStream<_>`
            `Pin<Box<dyn F + Send>> : WrapFuture<_>`
            `Pin<Box<dyn F + Send>> : WrapStream<_>`
      where F is the type: 
            `Future<Output = Result<Vec<ResultType>, Box<dyn Error>>>`

Then you end up with, again, a significantly more readable error. I imagine this could be implemented based on a cost function. When the size of some subtype in the error expressions is greater than some weight introduce a named type, starting with the same letter if possible, and rewrite the error expressions using it. I must admit I'm not that confident this would be feasible however.

7 Likes
23

In that specific case, it might be even clearer if the &mut Pin<Box<T>>, &Pin<Box<T>>, Pin<Box<T>>, &mut T, &T, T list could be compressed somehow. It's good that deref coersions are called out as potential resolutions in the error, but perhaps the self receiver types at least could be elided?

    = note: the method `into_actor` exists but the following trait bounds were not satisfied:
            `&mut Pin<Box<dyn F + Send>> : WrapFuture<_>` (or any compatible method receiver)
            `&mut Pin<Box<dyn F + Send>> : WrapStream<_>` (or any compatible method receiver)
      where:
          F = `Future<Output = Result<Vec<ResultType>, Box<dyn Error>>>`
1 Like
24

On the topic of making types easier to read, I'm also wondering if we might be able to do some sort of 'type diffing' in type mismatch error messages too - I often find it's pretty challenging to pick out the exact part of a long type that is different, and end up having to copy it into an editor to find the difference. This proposal would help, but perhaps it could be even better this way!

As an example, Scala 3 has type diffs:

vlmwrmD
vlmwrmD.png1676×245 30.9 KB

Note how the A and B are highlighted. Not sure how to make this accessible for non-coloured terminals though.

14 Likes
25

I like the idea of keeping the 'via imports' as footnotes - they're a bit like the reference section of a wikipedia article, you don't want it to cloud the main focus of the error message.

@ekuber is bang on the money that the locally available type name should be the short version (and showing crate version numbers when you have two versions of the same type would be F.A.B.).

Less scary compile errors are going to reduce the learning curve for new rustacians and us humans will spend less time parsing these compiler errors. Win-win. :crab:

1 Like
26

Note that we already do this. Try assigning am option to a field of type result in your local machine and you will see the highlighting through bolding. It is not present in the playground because it uses json output which doesn't expose the highlighting.

2 Likes
27

This is great, please cc me on the RFC and any rustc PRs :D.

However, I was under the impression that this didn't need an RFC, just a lot of implementation work (or less work if you follow one of my hacky ideas on one of the relevant issues, I can dig it up later if you're not sure what I mean).

4 Likes
28

@eddyb maybe we want to just roll this into a design meeting?

2 Likes
29

This topic was automatically closed 90 days after the last reply. New replies are no longer allowed.