Custom errors for well-known traits

[nikomatsakis]

Consider these comments by the one and only @steveklabnik:

[17:20:10] <steveklabnik> ugh
[17:20:19] <steveklabnik> so close to getting this to compile
[17:20:21] <steveklabnik> and i am rewarded with
[17:20:27] <steveklabnik>  error: the trait `core::iter::FromIterator<core::result::Result<(collections::vec::Vec<usize>, collections::string::String), std::sync::mpsc::RecvError>>` is not implemented
                          for the type `collections::vec::Vec<(collections::vec::Vec<usize>, collections::string::String)>`
[17:20:31] <steveklabnik> O_O
[17:21:01] <steveklabnik> that one is easy i guess
[17:21:04] <steveklabnik> just a scary message

wouldn't it be nicer if the compiler saw that error involved FromIterator and printed something like "the type you are iterating over doesn't match the expected type of the collection you are creating"?

Nominating for 1.0 (not beta), because this could be a big usability benefit. I'm sure there are other "well-known" traits we could do this for.

[ghost]

It would probably be a lot more work but it would be nice to do this in an extensible way so that the functionality could be exposed to library authors somehow.

[reem]

I've long wanted this but have been hesitant to ask for special casing. Perhaps a #[on_unimplemented] attribute could be used to specify an additional message to display when a type is expected to but does not implement this trait or something like that.

[nikomatsakis]

Yes, I had the same thought regarding extensibilty. At first I wasn't sure how to do it, but after thinking on it more, I bet something really simple, like the following, would work:

#[on_unimplemented="a collection of type `{Self}` cannot be built from an iterator over elements of type `{A}`"]
trait FromIterator<A>

basically you supply a format string, and we will give it arguments named after your type parameters with the stringified versions of each type.

[nikomatsakis]

It'd be helpful to have more examples where this could be applied.

[nikomatsakis]

I'd be happy to mentor this. Shouldn't be too hard.

[steveklabnik]

If anyone gets this error, it's that I had a Result where there previously was a value, so I just added an unwrap() because I'm lazy ;)

[ftxqxd]

I've thought about a slightly more general version of an on_unimplemented attribute before. Something like #[on_err(E1234, "a collection of type {Self}cannot be built from an iterator over elements of type{A}"], where E1234 is the error code for the 'unimplemented' error message (it doesn't seem to have one yet). With a more general attribute like that, error messages for things like using String instead of &str or vice-versa could give a better message as well.

[nikomatsakis]

Some notes on how you might do this. This is specific to my original proposal.

[04:04:59] <nmatsakis> Manishearth: so the place to start is librustc/middle/traits/error_reporting.rs
[04:05:23] <nmatsakis> Manishearth: when it finds that a TraitPredicate is Unimplemented, I would then go and fetch the attributes from the trait's def-id
[04:05:37] <nmatsakis> Manishearth: (`ty::has_attr` or something similar)
[04:05:42] <nmatsakis> Manishearth: and then extract the format string
[04:05:53] <nmatsakis> Manishearth: prepare the arguments by converting each type argument in the trait ref to a string
[04:06:04] <nmatsakis> Manishearth: and invoke the format_args! logic (you can't use the macro itself)
[04:06:25] <nmatsakis> Manishearth: at least that was my plan, when I write it out doesn't sound QUITE so trivial, in that there's a bit of familiarity with compiler data structures involved

[nikomatsakis]

@P1start makes sense. In any case, seems like we should start with something and grow as we go. This wouldn't a stable attribute anyway (at least not for some time).

[eddyb]

I have simplified format_args! recently (and everything involved is safe) but I think only the format string parser (in libfmt_macros) should be used, the actual invocation is much easier to do as a concatenation.

[ghost]

It'd be helpful to have more examples where this could be applied.

@nikomatsakis Perhaps not the common case but I'd love to have this for a library for generic programming that I'm working on. For example, the shapeless folks use something similar.

[reem]

@Manishearth if my understanding of the code is correct, this currently replaces the current error message with the custom one. I think this is incorrect, and instead the on_unimplemented message should be printed in addition to the more verbose and specific error. There may be cases where a simplified on_unimplemented message hides some important information that is needed to fix the error.

[Manishearth]

Yeah, I've fixed it to a note in my local branch