Rollup merge of #103283 - nbarrios1337:unsafe-impl-suggestions, r=cjgillot

matthiaskrgr · web-flow · commit 270e0c53574b · 2022-10-28T07:06:42.000+02:00
Add suggestions for unsafe impl error codes

Adds suggestions for users to add `unsafe` to trait impls that should be `unsafe`, and remove `unsafe` from trait impls that do not require `unsafe`

With the folllowing code:

```rust
struct Foo {}

struct Bar {}

trait Safe {}

unsafe trait Unsafe {}

impl Safe for Foo {} // ok

impl Unsafe for Foo {} // E0200

unsafe impl Safe for Bar {} // E0199

unsafe impl Unsafe for Bar {} // ok

// omitted empty main fn
```

The current rustc output is:
```
error[E0199]: implementing the trait `Safe` is not unsafe
  --&gt; e0200.rs:13:1
   |
13 | unsafe impl Safe for Bar {} // E0199
   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^

error[E0200]: the trait `Unsafe` requires an `unsafe impl` declaration
  --&gt; e0200.rs:11:1
   |
11 | impl Unsafe for Foo {} // E0200
   | ^^^^^^^^^^^^^^^^^^^^^^

error: aborting due to 2 previous errors

Some errors have detailed explanations: E0199, E0200.
For more information about an error, try `rustc --explain E0199`.
```

With this PR, the future rustc output would be:
```
error[E0199]: implementing the trait `Safe` is not unsafe
  --&gt; ../../temp/e0200.rs:13:1
   |
13 | unsafe impl Safe for Bar {} // E0199
   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^
   |
help: remove `unsafe` from this trait implementation
   |
13 - unsafe impl Safe for Bar {} // E0199
13 + impl Safe for Bar {} // E0199
   |

error[E0200]: the trait `Unsafe` requires an `unsafe impl` declaration
  --&gt; ../../temp/e0200.rs:11:1
   |
11 | impl Unsafe for Foo {} // E0200
   | ^^^^^^^^^^^^^^^^^^^^^^
   |
   = note: the trait `Unsafe` enforces invariants that the compiler can't check. Review the trait documentation and make sure this implementation upholds those invariants before adding the `unsafe` keyword
help: add `unsafe` to this trait implementation
   |
11 | unsafe impl Unsafe for Foo {} // E0200
   | ++++++

error: aborting due to 2 previous errors

Some errors have detailed explanations: E0199, E0200.
For more information about an error, try `rustc --explain E0199`.
```

``@rustbot`` label +T-compiler +A-diagnostics +A-suggestion-diagnostics
diff --git a/compiler/rustc_hir_analysis/src/coherence/unsafety.rs b/compiler/rustc_hir_analysis/src/coherence/unsafety.rs
@@ -26,6 +26,12 @@ pub(super) fn check_item(tcx: TyCtxt<'_>, def_id: LocalDefId) {
                     "implementing the trait `{}` is not unsafe",
                     trait_ref.print_only_trait_path()
                 )
+                .span_suggestion_verbose(
+                    item.span.with_hi(item.span.lo() + rustc_span::BytePos(7)),
+                    "remove `unsafe` from this trait implementation",
+                    "",
+                    rustc_errors::Applicability::MachineApplicable,
+                )
                 .emit();
             }
 
@@ -37,6 +43,18 @@ pub(super) fn check_item(tcx: TyCtxt<'_>, def_id: LocalDefId) {
                     "the trait `{}` requires an `unsafe impl` declaration",
                     trait_ref.print_only_trait_path()
                 )
+                .note(format!(
+                    "the trait `{}` enforces invariants that the compiler can't check. \
+                    Review the trait documentation and make sure this implementation \
+                    upholds those invariants before adding the `unsafe` keyword",
+                    trait_ref.print_only_trait_path()
+                ))
+                .span_suggestion_verbose(
+                    item.span.shrink_to_lo(),
+                    "add `unsafe` to this trait implementation",
+                    "unsafe ",
+                    rustc_errors::Applicability::MaybeIncorrect,
+                )
                 .emit();
             }
 
@@ -48,6 +66,18 @@ pub(super) fn check_item(tcx: TyCtxt<'_>, def_id: LocalDefId) {
                     "requires an `unsafe impl` declaration due to `#[{}]` attribute",
                     attr_name
                 )
+                .note(format!(
+                    "the trait `{}` enforces invariants that the compiler can't check. \
+                    Review the trait documentation and make sure this implementation \
+                    upholds those invariants before adding the `unsafe` keyword",
+                    trait_ref.print_only_trait_path()
+                ))
+                .span_suggestion_verbose(
+                    item.span.shrink_to_lo(),
+                    "add `unsafe` to this trait implementation",
+                    "unsafe ",
+                    rustc_errors::Applicability::MaybeIncorrect,
+                )
                 .emit();
             }
 
diff --git a/src/test/ui/coherence/coherence-default-trait-impl.stderr b/src/test/ui/coherence/coherence-default-trait-impl.stderr
@@ -3,12 +3,24 @@ error[E0199]: implementing the trait `MySafeTrait` is not unsafe
    |
 LL | unsafe impl MySafeTrait for Foo {}
    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+help: remove `unsafe` from this trait implementation
+   |
+LL - unsafe impl MySafeTrait for Foo {}
+LL + impl MySafeTrait for Foo {}
+   |
 
 error[E0200]: the trait `MyUnsafeTrait` requires an `unsafe impl` declaration
   --> $DIR/coherence-default-trait-impl.rs:13:1
    |
 LL | impl MyUnsafeTrait for Foo {}
    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = note: the trait `MyUnsafeTrait` enforces invariants that the compiler can't check. Review the trait documentation and make sure this implementation upholds those invariants before adding the `unsafe` keyword
+help: add `unsafe` to this trait implementation
+   |
+LL | unsafe impl MyUnsafeTrait for Foo {}
+   | ++++++
 
 error: aborting due to 2 previous errors
 
diff --git a/src/test/ui/dropck/dropck-eyepatch-implies-unsafe-impl.stderr b/src/test/ui/dropck/dropck-eyepatch-implies-unsafe-impl.stderr
@@ -8,6 +8,12 @@ LL | |     // (unsafe to access self.1  due to #[may_dangle] on A)
 LL | |     fn drop(&mut self) { println!("drop {} {:?}", self.0, self.2); }
 LL | | }
    | |_^
+   |
+   = note: the trait `Drop` enforces invariants that the compiler can't check. Review the trait documentation and make sure this implementation upholds those invariants before adding the `unsafe` keyword
+help: add `unsafe` to this trait implementation
+   |
+LL | unsafe impl<#[may_dangle] A, B: fmt::Debug> Drop for Pt<A, B> {
+   | ++++++
 
 error[E0569]: requires an `unsafe impl` declaration due to `#[may_dangle]` attribute
   --> $DIR/dropck-eyepatch-implies-unsafe-impl.rs:27:1
@@ -19,6 +25,12 @@ LL | |     // (unsafe to access self.1 due to #[may_dangle] on 'a)
 LL | |     fn drop(&mut self) { println!("drop {} {:?}", self.0, self.2); }
 LL | | }
    | |_^
+   |
+   = note: the trait `Drop` enforces invariants that the compiler can't check. Review the trait documentation and make sure this implementation upholds those invariants before adding the `unsafe` keyword
+help: add `unsafe` to this trait implementation
+   |
+LL | unsafe impl<#[may_dangle] 'a, 'b, B: fmt::Debug> Drop for Pr<'a, 'b, B> {
+   | ++++++
 
 error: aborting due to 2 previous errors
 
diff --git a/src/test/ui/error-codes/E0199.stderr b/src/test/ui/error-codes/E0199.stderr
@@ -3,6 +3,12 @@ error[E0199]: implementing the trait `Bar` is not unsafe
    |
 LL | unsafe impl Bar for Foo { }
    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+help: remove `unsafe` from this trait implementation
+   |
+LL - unsafe impl Bar for Foo { }
+LL + impl Bar for Foo { }
+   |
 
 error: aborting due to previous error
 
diff --git a/src/test/ui/error-codes/E0200.stderr b/src/test/ui/error-codes/E0200.stderr
@@ -3,6 +3,12 @@ error[E0200]: the trait `Bar` requires an `unsafe impl` declaration
    |
 LL | impl Bar for Foo { }
    | ^^^^^^^^^^^^^^^^^^^^
+   |
+   = note: the trait `Bar` enforces invariants that the compiler can't check. Review the trait documentation and make sure this implementation upholds those invariants before adding the `unsafe` keyword
+help: add `unsafe` to this trait implementation
+   |
+LL | unsafe impl Bar for Foo { }
+   | ++++++
 
 error: aborting due to previous error
 
diff --git a/src/test/ui/traits/safety-trait-impl-cc.stderr b/src/test/ui/traits/safety-trait-impl-cc.stderr
@@ -7,6 +7,12 @@ LL | |         panic!();
 LL | |     }
 LL | | }
    | |_^
+   |
+   = note: the trait `Foo` enforces invariants that the compiler can't check. Review the trait documentation and make sure this implementation upholds those invariants before adding the `unsafe` keyword
+help: add `unsafe` to this trait implementation
+   |
+LL | unsafe impl lib::Foo for Bar {
+   | ++++++
 
 error: aborting due to previous error
 
diff --git a/src/test/ui/traits/safety-trait-impl.stderr b/src/test/ui/traits/safety-trait-impl.stderr
@@ -3,12 +3,24 @@ error[E0200]: the trait `UnsafeTrait` requires an `unsafe impl` declaration
    |
 LL | impl UnsafeTrait for u16 { }
    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = note: the trait `UnsafeTrait` enforces invariants that the compiler can't check. Review the trait documentation and make sure this implementation upholds those invariants before adding the `unsafe` keyword
+help: add `unsafe` to this trait implementation
+   |
+LL | unsafe impl UnsafeTrait for u16 { }
+   | ++++++
 
 error[E0199]: implementing the trait `SafeTrait` is not unsafe
   --> $DIR/safety-trait-impl.rs:16:1
    |
 LL | unsafe impl SafeTrait for u32 { }
    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+help: remove `unsafe` from this trait implementation
+   |
+LL - unsafe impl SafeTrait for u32 { }
+LL + impl SafeTrait for u32 { }
+   |
 
 error: aborting due to 2 previous errors
 
