Stabilize the #[diagnostic] namespace and #[diagnostic::on_unimplemented] attribute

[weiznich]

This PR stabilizes the #[diagnostic] attribute namespace and a minimal option of the #[diagnostic::on_unimplemented] attribute.

The #[diagnostic] attribute namespace is meant to provide a home for attributes that allow users to influence error messages emitted by the compiler. The compiler is not guaranteed to use any of this hints, however it should accept any (non-)existing attribute in this namespace and potentially emit lint-warnings for unused attributes and options. This is meant to allow discarding certain attributes/options in the future to allow fundamental changes to the compiler without the need to keep then non-meaningful options working.

The #[diagnostic::on_unimplemented] attribute is allowed to appear on a trait definition. This allows crate authors to hint the compiler to emit a specific error message if a certain trait is not implemented. For the #[diagnostic::on_unimplemented] attribute the following options are implemented:

• message which provides the text for the top level error message
• label which provides the text for the label shown inline in the broken code in the error message
• note which provides additional notes.

The note option can appear several times, which results in several note messages being emitted. If any of the other options appears several times the first occurrence of the relevant option specifies the actually used value. Any other occurrence generates an lint warning. For any other non-existing option a lint-warning is generated.

All three options accept a text as argument. This text is allowed to contain format parameters referring to generic argument or Self by name via the {Self} or {NameOfGenericArgument} syntax. For any non-existing argument a lint warning is generated.

This allows to have a trait definition like:

#[diagnostic::on_unimplemented(
    message = "My Message for `ImportantTrait<{A}>` is not implemented for `{Self}`",
    label = "My Label",
    note = "Note 1",
    note = "Note 2"
)]
trait ImportantTrait<A> {}

which then generates for the following code

fn use_my_trait(_: impl ImportantTrait<i32>) {}

fn main() {
    use_my_trait(String::new());
}

this error message:

error[E0277]: My Message for `ImportantTrait<i32>` is not implemented for `String`
  --> src/main.rs:14:18
   |
14 |     use_my_trait(String::new());
   |     ------------ ^^^^^^^^^^^^^ My Label
   |     |
   |     required by a bound introduced by this call
   |
   = help: the trait `ImportantTrait<i32>` is not implemented for `String`
   = note: Note 1
   = note: Note 2

Playground with the unstable feature

Fixes #111996

[rustbot]

r? @b-naber

(rustbot has picked a reviewer for you, use r? to override)

[ehuss]

Before stabilization, can we make sure a reference PR is opened and reviewed?

[bors]

☔ The latest upstream changes (presumably #119088) made this pull request unmergeable. Please resolve the merge conflicts.

[weiznich]

Before stabilization, can we make sure a reference PR is opened and reviewed?

Do you have any suggestions where exactly the relevant documentation should be placed in the reference?

[ehuss]

There are a few places that need to be updated:

• Add it to the list of built-in attributes under the diagnostic section.
• Create a section in the diagnostics chapter that details the syntax and behavior of the attribute. There are templates for the format that attributes use (see deprecated and must_use on that page for examples).

Is this implemented as a Tool attribute? If so, that section should probably be updated. If not, then it might be OK in the diagnostics chapter to have a The `diagnostic` namespace section and then a subsection under that The `diagnostic::on_unimplemented` attribute.

Thanks!

[weiznich]

Thanks for the pointers 👍

I've opened a PR here: rust-lang/reference#1449

[bors]

☔ The latest upstream changes (presumably #119911) made this pull request unmergeable. Please resolve the merge conflicts.

[rustbot]

There are merge commits (commits with multiple parents) in your changes. We have a no merge policy so these commits will need to be removed for this pull request to be merged.

You can start a rebase with the following commands:

$ # rebase
$ git rebase -i master
$ # delete any merge commits in the editor that appears
$ git push --force-with-lease

The following commits are merge commits:

• 2528825

[weiznich]

@rustbot label -S-waiting-on-author -has-merge-commits

[weiznich]

@rustbot label +T-lang

(As this was decided in the discussion around the RFC rust-lang/rfcs#3368)

[bors]

☔ The latest upstream changes (presumably #120620) made this pull request unmergeable. Please resolve the merge conflicts.

[ehuss]

Is it intentional that the format parameters support the general syntax of std's format strings? There seems to be a variety of situations that don't seem to be handled like I would expect:

• Strings such as {} or {1:} generate an error instead of a warning. Doesn't that go against the intention that the diagnostic attributes are flexible in what they accept to support forwards compatibility?
• Strings such as {A:1} don't generate a warning or an error, even though it includes a nonsense width argument. This is true for all format spec things (precision, ? debug, etc.)
• Format specifiers allow flexible whitespace, like {A }.
• Braces are escaped by doubling them with {{ or }}.
• Incorrect braces seem to be ignored, and simply stop parsing, and are ignored (no warning). For example "foo }"

Is it intended that these format parameters are simple string replacements (like {Self} gets replaced with the type name) or is it intended that it uses the full formatting machinery of std's format strings (allowing things like width specifiers)? In either case, it seems like there are several situations that are not handled as I would expect. Additionally, none of this was documented.

[weiznich]

@ehuss I'm not sure I can follow you here. Do you mean that this was not mentioned at all? (In that case, both the Reference PR and the description explicitly mentions it). Or do you mean that it contains more features as described there? Or that these 4 specific cases result in different results than you personally would expect? For the later case I would argue that this is merely a bug that can be fixed independently from stabilization. Given a proper report with an explicit list of test cases I can try working on that.

[ehuss]

Do you mean that this was not mentioned at all? (In that case, both the Reference PR and the description explicitly mentions it).

The description says "This text is allowed to contain format parameters referring to ..." but doesn't define what a "format parameter" is other than {Self} or {NameOfGenericArgument}. Unless I missed something, I don't see where it says that the entire string is a std::fmt style format string. If the string is to be interpreted using some syntax system, then that needs to be explicitly stated and documented.

Given a proper report with an explicit list of test cases I can try working on that.

Sure, I'll file an issue.
EDIT: Filed #122391

[ehuss]

To put my concerns in a different way, this now requires that compiler implementers be able to interpret std format strings at runtime, which is not a requirement I think has been made until now. It is very unlikely to be an issue, but seems like a significant requirement that was not stated.