[RFC] On_unimplemented_trait_use

[B-2U]

Summary

Add [diagnostic::on_unimplemented_trait_use] in #[diagnostic] on structs that will influence error messages emitted by unsatisfied traits bounds.

Motivation

The idea came about when I was trying to print out a PathBuf, there's a custom message that said:

in format strings you may be able to use {:?} (or {:#?} for pretty-print) instead
call .display() or .to_string_lossy() to safely print paths, as they may contain non-Unicode data

And found out its hardcoded in trait Display

#[rustc_on_unimplemented(
    on(
        any(_Self = "std::path::Path", _Self = "std::path::PathBuf"),
        label = "`{Self}` cannot be formatted with the default formatter; call `.display()` on it",
        note = "call `.display()` or `.to_string_lossy()` to safely print paths, \
                as they may contain non-Unicode data"
    ),
    message = "`{Self}` doesn't implement `{Display}`",
    label = "`{Self}` cannot be formatted with the default formatter",
    note = "in format strings you may be able to use `{{:?}}` (or {{:#?}} for pretty-print) instead"
)]
pub trait Display {...}

It would be nice if this functionality is exposed to libraries as well, so that when the user tries to use an unimplemented trait (e.g. maybe Display isn't implemented because it's insufficient to clearly express intentions) the author can explain why via this diagnostic and offer a recommendation/alternative.

For example:

#[diagnostic::on_unimplemented_trait_use(
    trait = Display,
    message = "`{Self}` doesn't implement `{Display}`",
    label = "`{Self}` cannot be formatted with the default formatter; call `.display()` on it",
    note = "call `.display()` or `.to_string_lossy()` to safely print paths, \
                as they may contain non-Unicode data"
)]
struct PathBuf;

Unresolved questions

• syntax for generic traits

[B-2U]

Unresolved question: If the struct declares this diagnostic attribute for trait Foo but also implements Foo, what should be done?

[kennytm]

If the struct declares this diagnostic attribute for trait Foo but also implements Foo, what should be done?

Do nothing? The diagnostic is only consulted "on unimplemented", so it is no-op "on implemented".

[kennytm]

what is the syntax when the trait is generic? e.g. std::ops::Index<T> for str.

[B-2U]

Tbh, I'm not sure. Let me add it into the Unresolved questions

[cyqsimon]

If the struct declares this diagnostic attribute for trait Foo but also implements Foo, what should be done?

Do nothing? The diagnostic is only consulted "on unimplemented", so it is no-op "on implemented".

If I understand this proposal correctly, this attribute is only intended to be used on types not traits. So it would be a logical error to both explain why a trait isn't implemented but also implement said trait no?

[kennytm]

@cyqsimon When the type is generic e.g. struct Map<K, V> it is perfectly logical to implement a trait for a subset of that type e.g. T: Hash+Eq. But you can only attach the attribute to the entire struct Map declaration.

[cyqsimon]

@cyqsimon When the type is generic e.g. struct Map<K, V> it is perfectly logical to implement a trait for a subset of that type e.g. T: Hash+Eq. But you can only attach the attribute to the entire struct Map declaration.

Yikes that is annoying.

Would it be a good idea to emit a warning/error only when there exists a blanket implementation that is a superset of the unimplemented trait declared with this diagnostic item? So for example:

#[diagnostic::on_unimplemented_trait_use(
    trait = Bar,
    where(T: Clone), // new syntax
    ...
)]
struct Foo<T>(T);
trait Bar {}

// this is fine because `Copy + Clone` is stricter than `Clone`
impl<T: Copy + Clone> Bar for Foo<T> {}

// but these are not
impl<T: Clone> Bar for Foo<T> {}
impl<T> Bar for Foo<T> {}

[cyqsimon]

Also what do you think about the where() syntax in the attribute? My intention is to use it to address the problem of generic traits too (although for that we probably want to add a functional equivalent to the impl<T> syntax too).

[kennytm]

@cyqsimon

Would it be a good idea to emit a warning/error …

Please note that the #[diagnostic] namespace RFC #3368 specifically mentioned:

Any attribute in this namespace is not allowed to: Change the result of the compilation, which means applying such an attribute should never cause a compilation error as long as they are syntactically valid

[cyqsimon]

@cyqsimon

Would it be a good idea to emit a warning/error …

Please note that the #[diagnostic] namespace RFC #3368 specifically mentioned:

Any attribute in this namespace is not allowed to:

• Change the result of the compilation, which means applying such an attribute should never cause a compilation error as long as they are syntactically valid

Okay good to know, so no errors. Are we allowed to emit additional warnings though?

[kennytm]

@cyqsimon They can (in the same way #[diagnostic::something_invalid] will generate a warning in place), but I don't think this is a blocking issue of the RFC. In fact, considering needing to support generics I'd prefer the compiler not to do anything when the trait is actually implemented to keep both the implementation and usage simple.

where(T: Clone), // new syntax

If you checked #3368 the proposed syntax was if(T = Foo). The condition is evaluated using HIR-level information though (by comparing DefId), I don't think one should assume the compiler should solve trait bounds to prove T: Clone at this point.

[weiznich]

I would like to raise the same semi related note as in #3639: As far as I remember one of the main motivations for the specification of diagnostic attribute namespaces in #3368
was to outline a set of common rules for all attributes in this namespace, so that adding a new attribute to this namespace does not need a new RFC anymore.

That does not mean that there should be no discussion about new attributes, just that this likely doesn't need a full RFC.