From 975152ce30b8c0aa3cb7a3dca0a17c6c20e9db5b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C3=A9o=20Lanteri=20Thauvin?= Date: Thu, 28 Jul 2022 00:03:16 +0200 Subject: [PATCH] Add MVP suggestion for `unsafe_op_in_unsafe_fn` Nemo157 rebase notes: Migrated the changes to the lint into fluent --- compiler/rustc_mir_transform/messages.ftl | 1 + .../rustc_mir_transform/src/check_unsafety.rs | 26 ++++++++++++---- compiler/rustc_mir_transform/src/errors.rs | 19 ++++++++---- ...rfc-2585-unsafe_op_in_unsafe_fn.mir.stderr | 16 ++++++++++ .../unsafe/wrapping-unsafe-block-sugg.fixed | 12 ++++++++ tests/ui/unsafe/wrapping-unsafe-block-sugg.rs | 12 ++++++++ .../unsafe/wrapping-unsafe-block-sugg.stderr | 30 +++++++++++++++++++ 7 files changed, 105 insertions(+), 11 deletions(-) create mode 100644 tests/ui/unsafe/wrapping-unsafe-block-sugg.fixed create mode 100644 tests/ui/unsafe/wrapping-unsafe-block-sugg.rs create mode 100644 tests/ui/unsafe/wrapping-unsafe-block-sugg.stderr diff --git a/compiler/rustc_mir_transform/messages.ftl b/compiler/rustc_mir_transform/messages.ftl index b13429d121d..300e592a6b4 100644 --- a/compiler/rustc_mir_transform/messages.ftl +++ b/compiler/rustc_mir_transform/messages.ftl @@ -55,6 +55,7 @@ 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 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 069514d8a3b..fd8e02ebeab 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,24 @@ 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 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) + }), + }, + ); + suggest_unsafe_block = false; + } } } diff --git a/compiler/rustc_mir_transform/src/errors.rs b/compiler/rustc_mir_transform/src/errors.rs index 602e40d5131..b546e34f037 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,7 @@ impl RequiresUnsafeDetail { pub(crate) struct UnsafeOpInUnsafeFn { pub details: RequiresUnsafeDetail, + pub suggest_unsafe_block: Option<(Span, Span)>, } impl<'a> DecorateLint<'a, ()> for UnsafeOpInUnsafeFn { @@ -138,13 +140,20 @@ 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)) = self.suggest_unsafe_block { + diag.multipart_suggestion_verbose( + crate::fluent_generated::mir_transform_suggestion, + vec![(start, " unsafe {".into()), (end, "}".into())], + Applicability::MaybeIncorrect, + ); + } + diag } 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 6f005fe8958..2333f710c40 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 @@ -10,6 +10,14 @@ note: the lint level is defined here | LL | #![deny(unsafe_op_in_unsafe_fn)] | ^^^^^^^^^^^^^^^^^^^^^^ +help: consider wrapping the function body in an unsafe block + | +LL ~ unsafe fn deny_level() { unsafe { +LL | unsf(); + ... +LL | +LL ~ }} + | error: dereference of raw pointer is unsafe and requires unsafe block (error E0133) --> $DIR/rfc-2585-unsafe_op_in_unsafe_fn.rs:15:5 @@ -52,6 +60,14 @@ note: the lint level is defined here LL | #[deny(warnings)] | ^^^^^^^^ = note: `#[deny(unsafe_op_in_unsafe_fn)]` implied by `#[deny(warnings)]` +help: consider wrapping the function body in an unsafe block + | +LL ~ unsafe fn warning_level() { unsafe { +LL | unsf(); + ... +LL | +LL ~ }} + | error: dereference of raw pointer is unsafe and requires unsafe block (error E0133) --> $DIR/rfc-2585-unsafe_op_in_unsafe_fn.rs:31:5 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 00000000000..81b7d68bc4a --- /dev/null +++ b/tests/ui/unsafe/wrapping-unsafe-block-sugg.fixed @@ -0,0 +1,12 @@ +// run-rustfix + +#![deny(unsafe_op_in_unsafe_fn)] + +unsafe fn unsf() {} + +pub unsafe fn foo() { unsafe { + unsf(); //~ ERROR call to unsafe function is unsafe + unsf(); //~ ERROR call to unsafe function is unsafe +}} + +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 00000000000..ef6936d91d1 --- /dev/null +++ b/tests/ui/unsafe/wrapping-unsafe-block-sugg.rs @@ -0,0 +1,12 @@ +// run-rustfix + +#![deny(unsafe_op_in_unsafe_fn)] + +unsafe fn unsf() {} + +pub unsafe fn foo() { + unsf(); //~ ERROR call to unsafe function is unsafe + unsf(); //~ ERROR call to unsafe function is unsafe +} + +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 00000000000..7283ee08bdf --- /dev/null +++ b/tests/ui/unsafe/wrapping-unsafe-block-sugg.stderr @@ -0,0 +1,30 @@ +error: call to unsafe function is unsafe and requires unsafe block (error E0133) + --> $DIR/wrapping-unsafe-block-sugg.rs:8:5 + | +LL | unsf(); + | ^^^^^^ call to unsafe function + | + = note: consult the function's documentation for information on how to avoid undefined behavior +note: the lint level is defined here + --> $DIR/wrapping-unsafe-block-sugg.rs:3:9 + | +LL | #![deny(unsafe_op_in_unsafe_fn)] + | ^^^^^^^^^^^^^^^^^^^^^^ +help: consider wrapping the function body in an unsafe block + | +LL ~ pub unsafe fn foo() { unsafe { +LL | unsf(); +LL | unsf(); +LL ~ }} + | + +error: call to unsafe function is unsafe and requires unsafe block (error E0133) + --> $DIR/wrapping-unsafe-block-sugg.rs:9:5 + | +LL | unsf(); + | ^^^^^^ call to unsafe function + | + = note: consult the function's documentation for information on how to avoid undefined behavior + +error: aborting due to 2 previous errors +