Auto merge of #112017 - Nemo157:unsafe-block-rustfix, r=eholk

Add MVP suggestion for `unsafe_op_in_unsafe_fn`

Rebase of https://github.com/rust-lang/rust/pull/99827

cc tracking issue https://github.com/rust-lang/rust/issues/71668

No real changes since the original PR, just migrated the new suggestion to use fluent messages and added a couple more testcases, AFAICT from the discussion there were no outstanding changes requested.

compiler/rustc_mir_transform/messages.ftl

@@ -55,6 +55,8 @@ mir_transform_unaligned_packed_ref = reference to packed field is unaligned
 mir_transform_union_access_label = access to union field
 mir_transform_union_access_note = the field may not be properly initialized: using uninitialized data will cause undefined behavior
 mir_transform_unsafe_op_in_unsafe_fn = {$details} is unsafe and requires unsafe block (error E0133)
+    .suggestion = consider wrapping the function body in an unsafe block
+    .note = an unsafe function restricts its caller, but its body is safe by default
 
 mir_transform_unused_unsafe = unnecessary `unsafe` block
     .label = because it's nested under this `unsafe` block

compiler/rustc_mir_transform/src/check_unsafety.rs

@@ -525,6 +525,8 @@ pub fn check_unsafety(tcx: TyCtxt<'_>, def_id: LocalDefId) {
     }
 
     let UnsafetyCheckResult { violations, unused_unsafes, .. } = tcx.unsafety_check_result(def_id);
+    // Only suggest wrapping the entire function body in an unsafe block once
+    let mut suggest_unsafe_block = true;
 
     for &UnsafetyViolation { source_info, lint_root, kind, details } in violations.iter() {
         let details = errors::RequiresUnsafeDetail { violation: details, span: source_info.span };
@@ -559,12 +561,29 @@ pub fn check_unsafety(tcx: TyCtxt<'_>, def_id: LocalDefId) {
                     op_in_unsafe_fn_allowed,
                 });
             }
-            UnsafetyViolationKind::UnsafeFn => tcx.emit_spanned_lint(
-                UNSAFE_OP_IN_UNSAFE_FN,
-                lint_root,
-                source_info.span,
-                errors::UnsafeOpInUnsafeFn { details },
-            ),
+            UnsafetyViolationKind::UnsafeFn => {
+                tcx.emit_spanned_lint(
+                    UNSAFE_OP_IN_UNSAFE_FN,
+                    lint_root,
+                    source_info.span,
+                    errors::UnsafeOpInUnsafeFn {
+                        details,
+                        suggest_unsafe_block: suggest_unsafe_block.then(|| {
+                            let hir_id = tcx.hir().local_def_id_to_hir_id(def_id);
+                            let fn_sig = tcx
+                                .hir()
+                                .fn_sig_by_hir_id(hir_id)
+                                .expect("this violation only occurs in fn");
+                            let body = tcx.hir().body_owned_by(def_id);
+                            let body_span = tcx.hir().body(body).value.span;
+                            let start = tcx.sess.source_map().start_point(body_span).shrink_to_hi();
+                            let end = tcx.sess.source_map().end_point(body_span).shrink_to_lo();
+                            (start, end, fn_sig.span)
+                        }),
+                    },
+                );
+                suggest_unsafe_block = false;
+            }
         }
     }
 

compiler/rustc_mir_transform/src/errors.rs

@@ -1,5 +1,6 @@
 use rustc_errors::{
-    DecorateLint, DiagnosticBuilder, DiagnosticMessage, EmissionGuarantee, Handler, IntoDiagnostic,
+    Applicability, DecorateLint, DiagnosticBuilder, DiagnosticMessage, EmissionGuarantee, Handler,
+    IntoDiagnostic,
 };
 use rustc_macros::{Diagnostic, LintDiagnostic, Subdiagnostic};
 use rustc_middle::mir::{AssertKind, UnsafetyViolationDetails};
@@ -130,6 +131,12 @@ impl RequiresUnsafeDetail {
 
 pub(crate) struct UnsafeOpInUnsafeFn {
     pub details: RequiresUnsafeDetail,
+
+    /// These spans point to:
+    ///  1. the start of the function body
+    ///  2. the end of the function body
+    ///  3. the function signature
+    pub suggest_unsafe_block: Option<(Span, Span, Span)>,
 }
 
 impl<'a> DecorateLint<'a, ()> for UnsafeOpInUnsafeFn {
@@ -138,13 +145,21 @@ impl<'a> DecorateLint<'a, ()> for UnsafeOpInUnsafeFn {
         self,
         diag: &'b mut DiagnosticBuilder<'a, ()>,
     ) -> &'b mut DiagnosticBuilder<'a, ()> {
-        let desc = diag
-            .handler()
-            .expect("lint should not yet be emitted")
-            .eagerly_translate_to_string(self.details.label(), [].into_iter());
+        let handler = diag.handler().expect("lint should not yet be emitted");
+        let desc = handler.eagerly_translate_to_string(self.details.label(), [].into_iter());
         diag.set_arg("details", desc);
         diag.span_label(self.details.span, self.details.label());
         diag.note(self.details.note());
+
+        if let Some((start, end, fn_sig)) = self.suggest_unsafe_block {
+            diag.span_note(fn_sig, crate::fluent_generated::mir_transform_note);
+            diag.tool_only_multipart_suggestion(
+                crate::fluent_generated::mir_transform_suggestion,
+                vec![(start, " unsafe {".into()), (end, "}".into())],
+                Applicability::MaybeIncorrect,
+            );
+        }
+
         diag
     }
 

tests/ui/unsafe/auxiliary/external_unsafe_macro.rs

@@ -0,0 +1,4 @@
+pub unsafe fn unsf() {}
+
+#[macro_export]
+macro_rules! unsafe_macro { () => ($crate::unsf()) }

tests/ui/unsafe/rfc-2585-unsafe_op_in_unsafe_fn.mir.stderr

@@ -5,6 +5,11 @@ LL |     unsf();
    |     ^^^^^^ call to unsafe function
    |
    = note: consult the function's documentation for information on how to avoid undefined behavior
+note: an unsafe function restricts its caller, but its body is safe by default
+  --> $DIR/rfc-2585-unsafe_op_in_unsafe_fn.rs:11:1
+   |
+LL | unsafe fn deny_level() {
+   | ^^^^^^^^^^^^^^^^^^^^^^
 note: the lint level is defined here
   --> $DIR/rfc-2585-unsafe_op_in_unsafe_fn.rs:4:9
    |
@@ -46,6 +51,11 @@ LL |     unsf();
    |     ^^^^^^ call to unsafe function
    |
    = note: consult the function's documentation for information on how to avoid undefined behavior
+note: an unsafe function restricts its caller, but its body is safe by default
+  --> $DIR/rfc-2585-unsafe_op_in_unsafe_fn.rs:27:1
+   |
+LL | unsafe fn warning_level() {
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^
 note: the lint level is defined here
   --> $DIR/rfc-2585-unsafe_op_in_unsafe_fn.rs:26:8
    |

tests/ui/unsafe/wrapping-unsafe-block-sugg.fixed

@@ -0,0 +1,66 @@
+// run-rustfix
+// aux-build:external_unsafe_macro.rs
+
+#![deny(unsafe_op_in_unsafe_fn)] //~ NOTE
+
+extern crate external_unsafe_macro;
+
+unsafe fn unsf() {}
+
+pub unsafe fn foo() { unsafe {
+    //~^ NOTE an unsafe function restricts its caller, but its body is safe by default
+    unsf(); //~ ERROR call to unsafe function is unsafe
+    //~^ NOTE
+    //~| NOTE
+    unsf(); //~ ERROR call to unsafe function is unsafe
+    //~^ NOTE
+    //~| NOTE
+}}
+
+pub unsafe fn bar(x: *const i32) -> i32 { unsafe {
+    //~^ NOTE an unsafe function restricts its caller, but its body is safe by default
+    let y = *x; //~ ERROR dereference of raw pointer is unsafe and requires unsafe block
+    //~^ NOTE
+    //~| NOTE
+    y + *x //~ ERROR dereference of raw pointer is unsafe and requires unsafe block
+    //~^ NOTE
+    //~| NOTE
+}}
+
+static mut BAZ: i32 = 0;
+pub unsafe fn baz() -> i32 { unsafe {
+    //~^ NOTE an unsafe function restricts its caller, but its body is safe by default
+    let y = BAZ; //~ ERROR use of mutable static is unsafe and requires unsafe block
+    //~^ NOTE
+    //~| NOTE
+    y + BAZ //~ ERROR use of mutable static is unsafe and requires unsafe block
+    //~^ NOTE
+    //~| NOTE
+}}
+
+macro_rules! unsafe_macro { () => (unsf()) }
+//~^ ERROR call to unsafe function is unsafe
+//~| NOTE
+//~| NOTE
+//~| ERROR call to unsafe function is unsafe
+//~| NOTE
+//~| NOTE
+
+pub unsafe fn unsafe_in_macro() { unsafe {
+    //~^ NOTE an unsafe function restricts its caller, but its body is safe by default
+    unsafe_macro!();
+    //~^ NOTE
+    //~| NOTE
+    unsafe_macro!();
+    //~^ NOTE
+    //~| NOTE
+}}
+
+pub unsafe fn unsafe_in_external_macro() {
+    // FIXME: https://github.com/rust-lang/rust/issues/112504
+    // FIXME: ~^ NOTE an unsafe function restricts its caller, but its body is safe by default
+    external_unsafe_macro::unsafe_macro!();
+    external_unsafe_macro::unsafe_macro!();
+}
+
+fn main() {}

tests/ui/unsafe/wrapping-unsafe-block-sugg.rs

@@ -0,0 +1,66 @@
+// run-rustfix
+// aux-build:external_unsafe_macro.rs
+
+#![deny(unsafe_op_in_unsafe_fn)] //~ NOTE
+
+extern crate external_unsafe_macro;
+
+unsafe fn unsf() {}
+
+pub unsafe fn foo() {
+    //~^ NOTE an unsafe function restricts its caller, but its body is safe by default
+    unsf(); //~ ERROR call to unsafe function is unsafe
+    //~^ NOTE
+    //~| NOTE
+    unsf(); //~ ERROR call to unsafe function is unsafe
+    //~^ NOTE
+    //~| NOTE
+}
+
+pub unsafe fn bar(x: *const i32) -> i32 {
+    //~^ NOTE an unsafe function restricts its caller, but its body is safe by default
+    let y = *x; //~ ERROR dereference of raw pointer is unsafe and requires unsafe block
+    //~^ NOTE
+    //~| NOTE
+    y + *x //~ ERROR dereference of raw pointer is unsafe and requires unsafe block
+    //~^ NOTE
+    //~| NOTE
+}
+
+static mut BAZ: i32 = 0;
+pub unsafe fn baz() -> i32 {
+    //~^ NOTE an unsafe function restricts its caller, but its body is safe by default
+    let y = BAZ; //~ ERROR use of mutable static is unsafe and requires unsafe block
+    //~^ NOTE
+    //~| NOTE
+    y + BAZ //~ ERROR use of mutable static is unsafe and requires unsafe block
+    //~^ NOTE
+    //~| NOTE
+}
+
+macro_rules! unsafe_macro { () => (unsf()) }
+//~^ ERROR call to unsafe function is unsafe
+//~| NOTE
+//~| NOTE
+//~| ERROR call to unsafe function is unsafe
+//~| NOTE
+//~| NOTE
+
+pub unsafe fn unsafe_in_macro() {
+    //~^ NOTE an unsafe function restricts its caller, but its body is safe by default
+    unsafe_macro!();
+    //~^ NOTE
+    //~| NOTE
+    unsafe_macro!();
+    //~^ NOTE
+    //~| NOTE
+}
+
+pub unsafe fn unsafe_in_external_macro() {
+    // FIXME: https://github.com/rust-lang/rust/issues/112504
+    // FIXME: ~^ NOTE an unsafe function restricts its caller, but its body is safe by default
+    external_unsafe_macro::unsafe_macro!();
+    external_unsafe_macro::unsafe_macro!();
+}
+
+fn main() {}

tests/ui/unsafe/wrapping-unsafe-block-sugg.stderr

@@ -0,0 +1,99 @@
+error: call to unsafe function is unsafe and requires unsafe block (error E0133)
+  --> $DIR/wrapping-unsafe-block-sugg.rs:12:5
+   |
+LL |     unsf();
+   |     ^^^^^^ call to unsafe function
+   |
+   = note: consult the function's documentation for information on how to avoid undefined behavior
+note: an unsafe function restricts its caller, but its body is safe by default
+  --> $DIR/wrapping-unsafe-block-sugg.rs:10:1
+   |
+LL | pub unsafe fn foo() {
+   | ^^^^^^^^^^^^^^^^^^^
+note: the lint level is defined here
+  --> $DIR/wrapping-unsafe-block-sugg.rs:4:9
+   |
+LL | #![deny(unsafe_op_in_unsafe_fn)]
+   |         ^^^^^^^^^^^^^^^^^^^^^^
+
+error: call to unsafe function is unsafe and requires unsafe block (error E0133)
+  --> $DIR/wrapping-unsafe-block-sugg.rs:15:5
+   |
+LL |     unsf();
+   |     ^^^^^^ call to unsafe function
+   |
+   = note: consult the function's documentation for information on how to avoid undefined behavior
+
+error: dereference of raw pointer is unsafe and requires unsafe block (error E0133)
+  --> $DIR/wrapping-unsafe-block-sugg.rs:22:13
+   |
+LL |     let y = *x;
+   |             ^^ dereference of raw pointer
+   |
+   = note: raw pointers may be null, dangling or unaligned; they can violate aliasing rules and cause data races: all of these are undefined behavior
+note: an unsafe function restricts its caller, but its body is safe by default
+  --> $DIR/wrapping-unsafe-block-sugg.rs:20:1
+   |
+LL | pub unsafe fn bar(x: *const i32) -> i32 {
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+error: dereference of raw pointer is unsafe and requires unsafe block (error E0133)
+  --> $DIR/wrapping-unsafe-block-sugg.rs:25:9
+   |
+LL |     y + *x
+   |         ^^ dereference of raw pointer
+   |
+   = note: raw pointers may be null, dangling or unaligned; they can violate aliasing rules and cause data races: all of these are undefined behavior
+
+error: use of mutable static is unsafe and requires unsafe block (error E0133)
+  --> $DIR/wrapping-unsafe-block-sugg.rs:33:13
+   |
+LL |     let y = BAZ;
+   |             ^^^ use of mutable static
+   |
+   = note: mutable statics can be mutated by multiple threads: aliasing violations or data races will cause undefined behavior
+note: an unsafe function restricts its caller, but its body is safe by default
+  --> $DIR/wrapping-unsafe-block-sugg.rs:31:1
+   |
+LL | pub unsafe fn baz() -> i32 {
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+error: use of mutable static is unsafe and requires unsafe block (error E0133)
+  --> $DIR/wrapping-unsafe-block-sugg.rs:36:9
+   |
+LL |     y + BAZ
+   |         ^^^ use of mutable static
+   |
+   = note: mutable statics can be mutated by multiple threads: aliasing violations or data races will cause undefined behavior
+
+error: call to unsafe function is unsafe and requires unsafe block (error E0133)
+  --> $DIR/wrapping-unsafe-block-sugg.rs:41:36
+   |
+LL | macro_rules! unsafe_macro { () => (unsf()) }
+   |                                    ^^^^^^ call to unsafe function
+...
+LL |     unsafe_macro!();
+   |     --------------- in this macro invocation
+   |
+   = note: consult the function's documentation for information on how to avoid undefined behavior
+note: an unsafe function restricts its caller, but its body is safe by default
+  --> $DIR/wrapping-unsafe-block-sugg.rs:49:1
+   |
+LL | pub unsafe fn unsafe_in_macro() {
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   = note: this error originates in the macro `unsafe_macro` (in Nightly builds, run with -Z macro-backtrace for more info)
+
+error: call to unsafe function is unsafe and requires unsafe block (error E0133)
+  --> $DIR/wrapping-unsafe-block-sugg.rs:41:36
+   |
+LL | macro_rules! unsafe_macro { () => (unsf()) }
+   |                                    ^^^^^^ call to unsafe function
+...
+LL |     unsafe_macro!();
+   |     --------------- in this macro invocation
+   |
+   = note: consult the function's documentation for information on how to avoid undefined behavior
+   = note: this error originates in the macro `unsafe_macro` (in Nightly builds, run with -Z macro-backtrace for more info)
+
+error: aborting due to 8 previous errors
+