diff --git a/compiler/rustc_mir_transform/messages.ftl b/compiler/rustc_mir_transform/messages.ftl index b13429d121d10..2598eb2ed0968 100644 --- a/compiler/rustc_mir_transform/messages.ftl +++ b/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 diff --git a/compiler/rustc_mir_transform/src/check_unsafety.rs b/compiler/rustc_mir_transform/src/check_unsafety.rs index 069514d8a3bf3..41e31df77cb92 100644 --- a/compiler/rustc_mir_transform/src/check_unsafety.rs +++ b/compiler/rustc_mir_transform/src/check_unsafety.rs @@ -527,6 +527,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 }; @@ -561,12 +563,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; + } } } diff --git a/compiler/rustc_mir_transform/src/errors.rs b/compiler/rustc_mir_transform/src/errors.rs index 602e40d513104..05a27f853b875 100644 --- a/compiler/rustc_mir_transform/src/errors.rs +++ b/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 } diff --git a/tests/ui/unsafe/auxiliary/external_unsafe_macro.rs b/tests/ui/unsafe/auxiliary/external_unsafe_macro.rs new file mode 100644 index 0000000000000..b3837338b331e --- /dev/null +++ b/tests/ui/unsafe/auxiliary/external_unsafe_macro.rs @@ -0,0 +1,4 @@ +pub unsafe fn unsf() {} + +#[macro_export] +macro_rules! unsafe_macro { () => ($crate::unsf()) } diff --git a/tests/ui/unsafe/rfc-2585-unsafe_op_in_unsafe_fn.mir.stderr b/tests/ui/unsafe/rfc-2585-unsafe_op_in_unsafe_fn.mir.stderr index 6f005fe895848..0c0826c1cfb39 100644 --- a/tests/ui/unsafe/rfc-2585-unsafe_op_in_unsafe_fn.mir.stderr +++ b/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 | diff --git a/tests/ui/unsafe/wrapping-unsafe-block-sugg.fixed b/tests/ui/unsafe/wrapping-unsafe-block-sugg.fixed new file mode 100644 index 0000000000000..db1c102210e3b --- /dev/null +++ b/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() {} diff --git a/tests/ui/unsafe/wrapping-unsafe-block-sugg.rs b/tests/ui/unsafe/wrapping-unsafe-block-sugg.rs new file mode 100644 index 0000000000000..9c6be45896e25 --- /dev/null +++ b/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() {} diff --git a/tests/ui/unsafe/wrapping-unsafe-block-sugg.stderr b/tests/ui/unsafe/wrapping-unsafe-block-sugg.stderr new file mode 100644 index 0000000000000..43f619c27d274 --- /dev/null +++ b/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 +