Add ability to attach custom #[on_unimplemented] error messages for unimplemented traits

[Manishearth]

fixes #20783

r? @nikomatsakis

[nikomatsakis]

👯

[freebroccolo]

Awesome :] This should be very useful.

[sfackler]

Cool!

[flaper87]

looks better now, I think we should have an get_attr in ty but it can be left for a separate PR.

@nikomatsakis your call now :)

[huonw]

This pair of nested matchs can be if let Some(ref i) = item.value_str() { ... }.

[huonw]

What's the name i meant to represent?

[Manishearth]

An interned string. Updated.

[huonw]

Error messages conventionally start with a lower-case letter.

(They're printed like error: There is no type ..., and that looks a bit peculiar.)

[Manishearth]

Fixed

[huonw]

Yay!

I wonder if we need to have a definition-site verification as well as the use-site behaviour, as it stands one will only get validation that the on_unimplemented message is valid when actually triggering an unimplemented error; that is, downstream users will get error messages about the invalidity of the format string.

E.g. AFAICT, this will compile fine in isolation

#[on_unimplemented = "foo {X}, {Y}, {unclosed"]
pub trait Foo {}

[Manishearth]

@huonw fixed the other issues

[P1start]

It's great to see this! But I think the error message should still mention the specific trait (FromIterator in this case) somewhere, to show the actual reason why it doesn't work (and thus help towards resolving the error if the user is able to implement FromIterator for that type). That could either be done manually inside the specific #[on_unimplemented] attribute invocation or the attribute's behaviour could be changed in the compiler to add a help message instead of replacing the original error message. I personally don't have much of a preference either way.

[Manishearth]

@P1start It is done manually, a {Self} placeholder can be used (see the fake "FromIterator" in the test)

[Manishearth]

Oh, wait, I see, you want a placeholder for the trait name. I guess that can be done manually (or I can add a {trait} format arg, though that's just equivalent to directly writing path::to::Trait<{A}, {B}, {C}>). Got any idea for what the error for FromIterator should look like?

[Manishearth]

@huon I made it a note, and added a lint/test.

[huonw]

This is probably best run after the main error message, just in case the span_errs inside it are triggered, so that the output has a slightly more sensible order.

[huonw]

Hm, I suspect these spans are useless for the most part (I imagine they are usually cross-crate, and so unable to point to the actual attribute in question). It may be better to either use obligation.cause.span always, or detect if item.meta().span is syntax::codemap::DUMMY_SP or not and default to obligation.cause.span when it is DUMMY_SP.

[huonw]

(In fact, thinking about it now, it may also make sense to make these error messages clearer that they are talking about the on_unimplemented message, not the code you're calling, something like invalid #[on_unimplemented] format: there is no....)

[Ms2ger]

Indentation

[nikomatsakis]

Maybe add a test with an illegal thing, like {}?

[nrc]

r=nikomatsakis

[bors]

saw approval from nikomatsakis
at Manishearth@02d0a8b