From 09de1df96f0a08ce0d4b987432852461a0c8dcb6 Mon Sep 17 00:00:00 2001 From: Georg Semmler Date: Thu, 24 Oct 2024 07:40:30 +0200 Subject: [PATCH 1/7] Add documentation for `#[diagnostic::do_not_recommend]` --- src/attributes.md | 3 +++ src/attributes/diagnostics.md | 41 +++++++++++++++++++++++++++++++++++ 2 files changed, 44 insertions(+) diff --git a/src/attributes.md b/src/attributes.md index 1a316a902..5a387a0af 100644 --- a/src/attributes.md +++ b/src/attributes.md @@ -239,6 +239,8 @@ The following is an index of all built-in attributes. - [`must_use`] --- Generates a lint for unused values. - [`diagnostic::on_unimplemented`] --- Hints the compiler to emit a certain error message if a trait is not implemented. + - [`diagnostic::do_not_recommend`] --- Hints the compiler to not show a certain trait + impl in error messages - ABI, linking, symbols, and FFI - [`link`] --- Specifies a native library to link with an `extern` block. - [`link_name`] --- Specifies the name of the symbol for functions or statics @@ -371,3 +373,4 @@ The following is an index of all built-in attributes. [function pointer]: types/function-pointer.md [variadic functions]: items/external-blocks.html#variadic-functions [`diagnostic::on_unimplemented`]: attributes/diagnostics.md#the-diagnosticon_unimplemented-attribute +[`diagnostic::do_not_recommend`]: attributes/diagnostics.md#the-diagnosticdo_not_recommend-attribute diff --git a/src/attributes/diagnostics.md b/src/attributes/diagnostics.md index 6d21a5adf..1d45574a3 100644 --- a/src/attributes/diagnostics.md +++ b/src/attributes/diagnostics.md @@ -492,6 +492,47 @@ error[E0277]: My Message for `ImportantTrait` implemented for `String` = note: Note 2 ``` +### The `diagnostic::do_not_recommend` attribute + +The `#[diagnostic::on_unimplemented]` attribute is a hint to the compiler to not show a certain trait implementation as part of the error message. +The attribute should be placed on a [trait implementation items]. It does not accept any arguments. If the attribute is placed in a wrong location or contains an unexpected argument the compiler might emit a warning and otherwise ignore the malformed part. + +In this example: + +```rust,compile_fail,E0277 +trait Foo {} + +#[diagnostic::do_not_recommend] +impl Foo for T where T: Send {} + +fn needs_foo() {} + +fn main() { + needs_foo::<*mut ()>(); +} + +``` + +the compiler may generate an error message which looks like this: + +```text +error[E0277]: the trait bound `*mut (): Foo` is not satisfied + --> $DIR/simple.rs:15:17 + | +LL | needs_foo::<*mut ()>(); + | ^^^^^^^ the trait `Foo` is not implemented for `*mut ()` + | +note: required by a bound in `needs_foo` + --> $DIR/simple.rs:10:17 + | +LL | fn needs_foo() {} + | ^^^ required by this bound in `needs_foo` + +error: aborting due to 1 previous error +``` + +Without the attribute the compiler would complain about `*mut (): Send` instead. + [Clippy]: https://github.com/rust-lang/rust-clippy [_MetaListNameValueStr_]: ../attributes.md#meta-item-attribute-syntax [_MetaListPaths_]: ../attributes.md#meta-item-attribute-syntax From 3465b25c8e5d19a45a321715adbcc0f9a86f4050 Mon Sep 17 00:00:00 2001 From: Eric Huss Date: Mon, 9 Dec 2024 07:00:11 -0800 Subject: [PATCH 2/7] Update do_not_recommend docs A few updates for the docs here: - Fix mistake using wrong attribute name - Add annotations - Try to clarify what kind of behavior the attribute has - Separate allowed-positions, and syntax as separate concerns. --- src/attributes/diagnostics.md | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/src/attributes/diagnostics.md b/src/attributes/diagnostics.md index 5beedb4c9..694ce9fac 100644 --- a/src/attributes/diagnostics.md +++ b/src/attributes/diagnostics.md @@ -329,7 +329,7 @@ The `deprecated` attribute has several forms: r[attributes.diagnostic.deprecated.allowed-positions] The `deprecated` attribute may be applied to any [item], [trait item], [enum variant], [struct field], [external block item], or [macro definition]. It -cannot be applied to [trait implementation items]. When applied to an item +cannot be applied to [trait implementation items][trait-impl]. When applied to an item containing other items, such as a [module] or [implementation], all child items inherit the deprecation attribute. src/main.rs:9:17 + | +9 | needs_foo::<*mut ()>(); + | ^^^^^^^ the trait `Send` is not implemented for `*mut ()` + | +note: required for `*mut ()` to implement `Foo` + --> src/main.rs:3:9 + | +3 | impl Foo for T where T: Send {} + | ^^^ ^ ---- unsatisfied trait bound introduced here +note: required by a bound in `needs_foo` + --> src/main.rs:5:17 + | +5 | fn needs_foo() {} + | ^^^ required by this bound in `needs_foo` ``` -the compiler may generate an error message which looks like this: +By adding the `#[diagnostic::do_no_recommend]` attribute to the `impl`, the message would no longer suggest it: ```text error[E0277]: the trait bound `*mut (): Foo` is not satisfied - --> $DIR/simple.rs:15:17 + --> src/main.rs:11:17 | -LL | needs_foo::<*mut ()>(); +11 | needs_foo::<*mut ()>(); | ^^^^^^^ the trait `Foo` is not implemented for `*mut ()` | note: required by a bound in `needs_foo` - --> $DIR/simple.rs:10:17 + --> src/main.rs:7:17 | -LL | fn needs_foo() {} +7 | fn needs_foo() {} | ^^^ required by this bound in `needs_foo` - -error: aborting due to 1 previous error ``` -Without the attribute the compiler would complain about `Send` not being implemented for `*mut ()` due to the required bounds in the implementation. - [Clippy]: https://github.com/rust-lang/rust-clippy [_MetaListNameValueStr_]: ../attributes.md#meta-item-attribute-syntax [_MetaListPaths_]: ../attributes.md#meta-item-attribute-syntax From 341cc837141f912a13749b2bf338493be8e1340f Mon Sep 17 00:00:00 2001 From: Eric Huss Date: Sun, 15 Dec 2024 15:09:04 -0800 Subject: [PATCH 5/7] Soften the language to say "usually" There may be times when the recommendation is useful, but the intent is to use the attribute when it usually is not. --- src/attributes/diagnostics.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/attributes/diagnostics.md b/src/attributes/diagnostics.md index fec23e09c..5e0c95b6e 100644 --- a/src/attributes/diagnostics.md +++ b/src/attributes/diagnostics.md @@ -566,7 +566,7 @@ r[attributes.diagnostic.do_not_recommend] r[attributes.diagnostic.do_not_recommend.intro] The `#[diagnostic::do_not_recommend]` attribute is a hint to the compiler to not show the annotated trait implementation as part of a diagnostic message. -> **Note**: Suppressing the recommendation can be useful if you know that the recommendation would not be useful to the programmer. This often occurs with broad, blanket impls. The recommendation may send the programmer down the wrong path, or the trait implementation may be an internal detail that you don't want to expose, or the bounds may not be able to be satisfied by the programmer. +> **Note**: Suppressing the recommendation can be useful if you know that the recommendation would usually not be useful to the programmer. This often occurs with broad, blanket impls. The recommendation may send the programmer down the wrong path, or the trait implementation may be an internal detail that you don't want to expose, or the bounds may not be able to be satisfied by the programmer. > > For example, in an error message about a type not implementing a required trait, the compiler may find a trait implementation that would satisfy the requirements if it weren't for specific bounds in the trait implementation. The compiler may tell the user that there is an impl, but the problem is the bounds in the trait implementation. The `#[diagnostic::do_not_recommend]` attribute can be used to tell the compiler to *not* tell the user about the trait implementation, and instead simply tell the user the type doesn't implement the required trait. From de346adfa3bf1d65e4a41ad627dcf293dd1d21d0 Mon Sep 17 00:00:00 2001 From: Eric Huss Date: Sun, 15 Dec 2024 15:11:35 -0800 Subject: [PATCH 6/7] Rewrite do_not_recommend example This rewrites the example to use a more realistic example that actually demonstrates an improvement in the error message. --- src/attributes/diagnostics.md | 112 +++++++++++++++++++++++----------- 1 file changed, 78 insertions(+), 34 deletions(-) diff --git a/src/attributes/diagnostics.md b/src/attributes/diagnostics.md index 5e0c95b6e..8210b9d48 100644 --- a/src/attributes/diagnostics.md +++ b/src/attributes/diagnostics.md @@ -576,58 +576,102 @@ The attribute should be placed on a [trait implementation item][trait-impl], tho r[attributes.diagnostic.do_not_recommend.syntax] The attribute does not accept any arguments, though unexpected arguments are not considered as an error. -In this example, +In the following example, there is a trait called `AsExpression` which is used for casting arbitrary types to the `Expression` type used in an SQL library. There is a method called `check` which takes an `AsExpression`. ```rust,compile_fail,E0277 -trait Foo {} - -impl Foo for T where T: Send {} +# pub trait Expression { +# type SqlType; +# } +# +# pub trait AsExpression { +# type Expression: Expression; +# } +# +# pub struct Text; +# pub struct Integer; +# +# pub struct Bound(T); +# pub struct SelectInt; +# +# impl Expression for SelectInt { +# type SqlType = Integer; +# } +# +# impl Expression for Bound { +# type SqlType = T; +# } +# +# impl AsExpression for i32 { +# type Expression = Bound; +# } +# +# impl AsExpression for &'_ str { +# type Expression = Bound; +# } +# +# impl Foo for T where T: Expression {} + +// Uncomment this line to change the recommendation. +// #[diagnostic::do_not_recommend] +impl AsExpression for T +where + T: Expression, +{ + type Expression = T; +} -fn needs_foo() {} +trait Foo: Expression + Sized { + fn check(&self, _: T) -> ::SqlType>>::Expression + where + T: AsExpression, + { + todo!() + } +} fn main() { - // Mutable pointers do not implement `Send`, so this will not work. - needs_foo::<*mut ()>(); + SelectInt.check("bar"); } ``` -the compiler may generate an error message about the `Send` bound in the impl which looks like this: +The `SelectInt` type's `check` method is expecting an `Integer` type. Calling it with an i32 type works, as it gets converted to an `Integer` by the `AsExpression` trait. However, calling it with a string does not, and generates a an error that may look like this: ```text -error[E0277]: the trait bound `*mut (): Foo` is not satisfied - --> src/main.rs:9:17 - | -9 | needs_foo::<*mut ()>(); - | ^^^^^^^ the trait `Send` is not implemented for `*mut ()` - | -note: required for `*mut ()` to implement `Foo` - --> src/main.rs:3:9 - | -3 | impl Foo for T where T: Send {} - | ^^^ ^ ---- unsatisfied trait bound introduced here -note: required by a bound in `needs_foo` - --> src/main.rs:5:17 - | -5 | fn needs_foo() {} - | ^^^ required by this bound in `needs_foo` +error[E0277]: the trait bound `&str: Expression` is not satisfied + --> src/main.rs:53:15 + | +53 | SelectInt.check("bar"); + | ^^^^^ the trait `Expression` is not implemented for `&str` + | + = help: the following other types implement trait `Expression`: + Bound + SelectInt +note: required for `&str` to implement `AsExpression` + --> src/main.rs:45:13 + | +45 | impl AsExpression for T + | ^^^^^^^^^^^^^^^^ ^ +46 | where +47 | T: Expression, + | ------------------------ unsatisfied trait bound introduced here ``` -By adding the `#[diagnostic::do_no_recommend]` attribute to the `impl`, the message would no longer suggest it: +By adding the `#[diagnostic::do_no_recommend]` attribute to the blanket `impl` for `AsExpression`, the message changes to: ```text -error[E0277]: the trait bound `*mut (): Foo` is not satisfied - --> src/main.rs:11:17 +error[E0277]: the trait bound `&str: AsExpression` is not satisfied + --> src/main.rs:53:15 | -11 | needs_foo::<*mut ()>(); - | ^^^^^^^ the trait `Foo` is not implemented for `*mut ()` +53 | SelectInt.check("bar"); + | ^^^^^ the trait `AsExpression` is not implemented for `&str` | -note: required by a bound in `needs_foo` - --> src/main.rs:7:17 - | -7 | fn needs_foo() {} - | ^^^ required by this bound in `needs_foo` + = help: the trait `AsExpression` is not implemented for `&str` + but trait `AsExpression` is implemented for it + = help: for that trait implementation, expected `Text`, found `Integer` ``` +The first error message includes a somewhat confusing error message about the relationship of `&str` and `Expression`, as well as the unsatisfied trait bound in the blanket impl. After adding `#[diagnostic::do_no_recommend]`, it no longer considers the blanket impl for the recommendation. The message should be a little clearer, with an indication that a string cannot be converted to an `Integer`. + [Clippy]: https://github.com/rust-lang/rust-clippy [_MetaListNameValueStr_]: ../attributes.md#meta-item-attribute-syntax [_MetaListPaths_]: ../attributes.md#meta-item-attribute-syntax From e4c6263af31fb4e95eae5addf03aceb1bd03cd0a Mon Sep 17 00:00:00 2001 From: Eric Huss Date: Mon, 16 Dec 2024 07:37:21 -0800 Subject: [PATCH 7/7] usually to normally Co-authored-by: Georg Semmler --- src/attributes/diagnostics.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/attributes/diagnostics.md b/src/attributes/diagnostics.md index 8210b9d48..651554c5e 100644 --- a/src/attributes/diagnostics.md +++ b/src/attributes/diagnostics.md @@ -566,7 +566,7 @@ r[attributes.diagnostic.do_not_recommend] r[attributes.diagnostic.do_not_recommend.intro] The `#[diagnostic::do_not_recommend]` attribute is a hint to the compiler to not show the annotated trait implementation as part of a diagnostic message. -> **Note**: Suppressing the recommendation can be useful if you know that the recommendation would usually not be useful to the programmer. This often occurs with broad, blanket impls. The recommendation may send the programmer down the wrong path, or the trait implementation may be an internal detail that you don't want to expose, or the bounds may not be able to be satisfied by the programmer. +> **Note**: Suppressing the recommendation can be useful if you know that the recommendation would normally not be useful to the programmer. This often occurs with broad, blanket impls. The recommendation may send the programmer down the wrong path, or the trait implementation may be an internal detail that you don't want to expose, or the bounds may not be able to be satisfied by the programmer. > > For example, in an error message about a type not implementing a required trait, the compiler may find a trait implementation that would satisfy the requirements if it weren't for specific bounds in the trait implementation. The compiler may tell the user that there is an impl, but the problem is the bounds in the trait implementation. The `#[diagnostic::do_not_recommend]` attribute can be used to tell the compiler to *not* tell the user about the trait implementation, and instead simply tell the user the type doesn't implement the required trait.