Skip to content

Commit

Permalink
Add suggestion to THIR unsafe_op_in_unsafe_fn lint
Browse files Browse the repository at this point in the history
  • Loading branch information
matthewjasper committed Nov 6, 2023
1 parent 2b2c0f9 commit 2b59992
Show file tree
Hide file tree
Showing 13 changed files with 367 additions and 40 deletions.
9 changes: 9 additions & 0 deletions compiler/rustc_hir/src/hir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3566,6 +3566,15 @@ impl<'hir> OwnerNode<'hir> {
}
}

pub fn fn_sig(self) -> Option<&'hir FnSig<'hir>> {
match self {
OwnerNode::TraitItem(TraitItem { kind: TraitItemKind::Fn(fn_sig, _), .. })
| OwnerNode::ImplItem(ImplItem { kind: ImplItemKind::Fn(fn_sig, _), .. })
| OwnerNode::Item(Item { kind: ItemKind::Fn(fn_sig, _, _), .. }) => Some(fn_sig),
_ => None,
}
}

pub fn fn_decl(self) -> Option<&'hir FnDecl<'hir>> {
match self {
OwnerNode::TraitItem(TraitItem { kind: TraitItemKind::Fn(fn_sig, _), .. })
Expand Down
3 changes: 3 additions & 0 deletions compiler/rustc_mir_build/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -320,6 +320,7 @@ mir_build_unreachable_pattern = unreachable pattern
.label = unreachable pattern
.catchall_label = matches any value
mir_build_unsafe_fn_safe_body = an unsafe function restricts its caller, but its body is safe by default
mir_build_unsafe_not_inherited = items do not inherit unsafety from separate enclosing items
mir_build_unsafe_op_in_unsafe_fn_borrow_of_layout_constrained_field_requires_unsafe =
Expand Down Expand Up @@ -386,3 +387,5 @@ mir_build_unused_unsafe = unnecessary `unsafe` block
mir_build_unused_unsafe_enclosing_block_label = because it's nested under this `unsafe` block
mir_build_variant_defined_here = not covered
mir_build_wrap_suggestion = consider wrapping the function body in an unsafe block
76 changes: 66 additions & 10 deletions compiler/rustc_mir_build/src/check_unsafety.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,10 @@ struct UnsafetyVisitor<'a, 'tcx> {
param_env: ParamEnv<'tcx>,
inside_adt: bool,
warnings: &'a mut Vec<UnusedUnsafeWarning>,

/// Flag to ensure that we only suggest wrapping the entire function body in
/// an unsafe block once.
suggest_unsafe_block: bool,
}

impl<'tcx> UnsafetyVisitor<'_, 'tcx> {
Expand Down Expand Up @@ -95,7 +99,13 @@ impl<'tcx> UnsafetyVisitor<'_, 'tcx> {
SafetyContext::UnsafeFn if unsafe_op_in_unsafe_fn_allowed => {}
SafetyContext::UnsafeFn => {
// unsafe_op_in_unsafe_fn is disallowed
kind.emit_unsafe_op_in_unsafe_fn_lint(self.tcx, self.hir_context, span);
kind.emit_unsafe_op_in_unsafe_fn_lint(
self.tcx,
self.hir_context,
span,
self.suggest_unsafe_block,
);
self.suggest_unsafe_block = false;
}
SafetyContext::Safe => {
kind.emit_requires_unsafe_err(
Expand Down Expand Up @@ -297,6 +307,7 @@ impl<'a, 'tcx> Visitor<'a, 'tcx> for UnsafetyVisitor<'a, 'tcx> {
}
PatKind::InlineConstant { def, .. } => {
self.visit_inner_body(*def);
visit::walk_pat(self, pat);
}
_ => {
visit::walk_pat(self, pat);
Expand Down Expand Up @@ -545,7 +556,22 @@ impl UnsafeOpKind {
tcx: TyCtxt<'_>,
hir_id: hir::HirId,
span: Span,
suggest_unsafe_block: bool,
) {
let parent_id = tcx.hir().get_parent_item(hir_id);
let parent_owner = tcx.hir().owner(parent_id);
let should_suggest = parent_owner.fn_sig().map_or(false, |sig| sig.header.is_unsafe());
let unsafe_not_inherited_note = if should_suggest {
suggest_unsafe_block.then(|| {
let body_span = tcx.hir().body(parent_owner.body_id().unwrap()).value.span;
UnsafeNotInheritedLintNote {
signature_span: tcx.def_span(parent_id.def_id),
body_span,
}
})
} else {
None
};
// FIXME: ideally we would want to trim the def paths, but this is not
// feasible with the current lint emission API (see issue #106126).
match self {
Expand All @@ -556,61 +582,89 @@ impl UnsafeOpKind {
UnsafeOpInUnsafeFnCallToUnsafeFunctionRequiresUnsafe {
span,
function: &with_no_trimmed_paths!(tcx.def_path_str(*did)),
unsafe_not_inherited_note,
},
),
CallToUnsafeFunction(None) => tcx.emit_spanned_lint(
UNSAFE_OP_IN_UNSAFE_FN,
hir_id,
span,
UnsafeOpInUnsafeFnCallToUnsafeFunctionRequiresUnsafeNameless { span },
UnsafeOpInUnsafeFnCallToUnsafeFunctionRequiresUnsafeNameless {
span,
unsafe_not_inherited_note,
},
),
UseOfInlineAssembly => tcx.emit_spanned_lint(
UNSAFE_OP_IN_UNSAFE_FN,
hir_id,
span,
UnsafeOpInUnsafeFnUseOfInlineAssemblyRequiresUnsafe { span },
UnsafeOpInUnsafeFnUseOfInlineAssemblyRequiresUnsafe {
span,
unsafe_not_inherited_note,
},
),
InitializingTypeWith => tcx.emit_spanned_lint(
UNSAFE_OP_IN_UNSAFE_FN,
hir_id,
span,
UnsafeOpInUnsafeFnInitializingTypeWithRequiresUnsafe { span },
UnsafeOpInUnsafeFnInitializingTypeWithRequiresUnsafe {
span,
unsafe_not_inherited_note,
},
),
UseOfMutableStatic => tcx.emit_spanned_lint(
UNSAFE_OP_IN_UNSAFE_FN,
hir_id,
span,
UnsafeOpInUnsafeFnUseOfMutableStaticRequiresUnsafe { span },
UnsafeOpInUnsafeFnUseOfMutableStaticRequiresUnsafe {
span,
unsafe_not_inherited_note,
},
),
UseOfExternStatic => tcx.emit_spanned_lint(
UNSAFE_OP_IN_UNSAFE_FN,
hir_id,
span,
UnsafeOpInUnsafeFnUseOfExternStaticRequiresUnsafe { span },
UnsafeOpInUnsafeFnUseOfExternStaticRequiresUnsafe {
span,
unsafe_not_inherited_note,
},
),
DerefOfRawPointer => tcx.emit_spanned_lint(
UNSAFE_OP_IN_UNSAFE_FN,
hir_id,
span,
UnsafeOpInUnsafeFnDerefOfRawPointerRequiresUnsafe { span },
UnsafeOpInUnsafeFnDerefOfRawPointerRequiresUnsafe {
span,
unsafe_not_inherited_note,
},
),
AccessToUnionField => tcx.emit_spanned_lint(
UNSAFE_OP_IN_UNSAFE_FN,
hir_id,
span,
UnsafeOpInUnsafeFnAccessToUnionFieldRequiresUnsafe { span },
UnsafeOpInUnsafeFnAccessToUnionFieldRequiresUnsafe {
span,
unsafe_not_inherited_note,
},
),
MutationOfLayoutConstrainedField => tcx.emit_spanned_lint(
UNSAFE_OP_IN_UNSAFE_FN,
hir_id,
span,
UnsafeOpInUnsafeFnMutationOfLayoutConstrainedFieldRequiresUnsafe { span },
UnsafeOpInUnsafeFnMutationOfLayoutConstrainedFieldRequiresUnsafe {
span,
unsafe_not_inherited_note,
},
),
BorrowOfLayoutConstrainedField => tcx.emit_spanned_lint(
UNSAFE_OP_IN_UNSAFE_FN,
hir_id,
span,
UnsafeOpInUnsafeFnBorrowOfLayoutConstrainedFieldRequiresUnsafe { span },
UnsafeOpInUnsafeFnBorrowOfLayoutConstrainedFieldRequiresUnsafe {
span,
unsafe_not_inherited_note,
},
),
CallToFunctionWith(did) => tcx.emit_spanned_lint(
UNSAFE_OP_IN_UNSAFE_FN,
Expand All @@ -619,6 +673,7 @@ impl UnsafeOpKind {
UnsafeOpInUnsafeFnCallToFunctionWithRequiresUnsafe {
span,
function: &with_no_trimmed_paths!(tcx.def_path_str(*did)),
unsafe_not_inherited_note,
},
),
}
Expand Down Expand Up @@ -833,6 +888,7 @@ pub fn thir_check_unsafety(tcx: TyCtxt<'_>, def: LocalDefId) {
param_env: tcx.param_env(def),
inside_adt: false,
warnings: &mut warnings,
suggest_unsafe_block: true,
};
visitor.visit_expr(&thir[expr]);

Expand Down
43 changes: 43 additions & 0 deletions compiler/rustc_mir_build/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@ pub struct UnsafeOpInUnsafeFnCallToUnsafeFunctionRequiresUnsafe<'a> {
#[label]
pub span: Span,
pub function: &'a str,
#[subdiagnostic]
pub unsafe_not_inherited_note: Option<UnsafeNotInheritedLintNote>,
}

#[derive(LintDiagnostic)]
Expand All @@ -37,6 +39,8 @@ pub struct UnsafeOpInUnsafeFnCallToUnsafeFunctionRequiresUnsafe<'a> {
pub struct UnsafeOpInUnsafeFnCallToUnsafeFunctionRequiresUnsafeNameless {
#[label]
pub span: Span,
#[subdiagnostic]
pub unsafe_not_inherited_note: Option<UnsafeNotInheritedLintNote>,
}

#[derive(LintDiagnostic)]
Expand All @@ -45,6 +49,8 @@ pub struct UnsafeOpInUnsafeFnCallToUnsafeFunctionRequiresUnsafeNameless {
pub struct UnsafeOpInUnsafeFnUseOfInlineAssemblyRequiresUnsafe {
#[label]
pub span: Span,
#[subdiagnostic]
pub unsafe_not_inherited_note: Option<UnsafeNotInheritedLintNote>,
}

#[derive(LintDiagnostic)]
Expand All @@ -53,6 +59,8 @@ pub struct UnsafeOpInUnsafeFnUseOfInlineAssemblyRequiresUnsafe {
pub struct UnsafeOpInUnsafeFnInitializingTypeWithRequiresUnsafe {
#[label]
pub span: Span,
#[subdiagnostic]
pub unsafe_not_inherited_note: Option<UnsafeNotInheritedLintNote>,
}

#[derive(LintDiagnostic)]
Expand All @@ -61,6 +69,8 @@ pub struct UnsafeOpInUnsafeFnInitializingTypeWithRequiresUnsafe {
pub struct UnsafeOpInUnsafeFnUseOfMutableStaticRequiresUnsafe {
#[label]
pub span: Span,
#[subdiagnostic]
pub unsafe_not_inherited_note: Option<UnsafeNotInheritedLintNote>,
}

#[derive(LintDiagnostic)]
Expand All @@ -69,6 +79,8 @@ pub struct UnsafeOpInUnsafeFnUseOfMutableStaticRequiresUnsafe {
pub struct UnsafeOpInUnsafeFnUseOfExternStaticRequiresUnsafe {
#[label]
pub span: Span,
#[subdiagnostic]
pub unsafe_not_inherited_note: Option<UnsafeNotInheritedLintNote>,
}

#[derive(LintDiagnostic)]
Expand All @@ -77,6 +89,8 @@ pub struct UnsafeOpInUnsafeFnUseOfExternStaticRequiresUnsafe {
pub struct UnsafeOpInUnsafeFnDerefOfRawPointerRequiresUnsafe {
#[label]
pub span: Span,
#[subdiagnostic]
pub unsafe_not_inherited_note: Option<UnsafeNotInheritedLintNote>,
}

#[derive(LintDiagnostic)]
Expand All @@ -85,6 +99,8 @@ pub struct UnsafeOpInUnsafeFnDerefOfRawPointerRequiresUnsafe {
pub struct UnsafeOpInUnsafeFnAccessToUnionFieldRequiresUnsafe {
#[label]
pub span: Span,
#[subdiagnostic]
pub unsafe_not_inherited_note: Option<UnsafeNotInheritedLintNote>,
}

#[derive(LintDiagnostic)]
Expand All @@ -93,13 +109,17 @@ pub struct UnsafeOpInUnsafeFnAccessToUnionFieldRequiresUnsafe {
pub struct UnsafeOpInUnsafeFnMutationOfLayoutConstrainedFieldRequiresUnsafe {
#[label]
pub span: Span,
#[subdiagnostic]
pub unsafe_not_inherited_note: Option<UnsafeNotInheritedLintNote>,
}

#[derive(LintDiagnostic)]
#[diag(mir_build_unsafe_op_in_unsafe_fn_borrow_of_layout_constrained_field_requires_unsafe)]
pub struct UnsafeOpInUnsafeFnBorrowOfLayoutConstrainedFieldRequiresUnsafe {
#[label]
pub span: Span,
#[subdiagnostic]
pub unsafe_not_inherited_note: Option<UnsafeNotInheritedLintNote>,
}

#[derive(LintDiagnostic)]
Expand All @@ -109,6 +129,8 @@ pub struct UnsafeOpInUnsafeFnCallToFunctionWithRequiresUnsafe<'a> {
#[label]
pub span: Span,
pub function: &'a str,
#[subdiagnostic]
pub unsafe_not_inherited_note: Option<UnsafeNotInheritedLintNote>,
}

#[derive(Diagnostic)]
Expand Down Expand Up @@ -376,6 +398,27 @@ pub struct UnsafeNotInheritedNote {
pub span: Span,
}

pub struct UnsafeNotInheritedLintNote {
pub signature_span: Span,
pub body_span: Span,
}

impl AddToDiagnostic for UnsafeNotInheritedLintNote {
fn add_to_diagnostic_with<F>(self, diag: &mut Diagnostic, _: F)
where
F: Fn(&mut Diagnostic, SubdiagnosticMessage) -> SubdiagnosticMessage,
{
diag.span_note(self.signature_span, fluent::mir_build_unsafe_fn_safe_body);
let body_start = self.body_span.shrink_to_lo();
let body_end = self.body_span.shrink_to_hi();
diag.tool_only_multipart_suggestion(
fluent::mir_build_wrap_suggestion,
vec![(body_start, "{ unsafe ".into()), (body_end, "}".into())],
Applicability::MaybeIncorrect,
);
}
}

#[derive(LintDiagnostic)]
#[diag(mir_build_unused_unsafe)]
pub struct UnusedUnsafe {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
warning: call to unsafe function is unsafe and requires unsafe block (error E0133)
--> $DIR/edition-2024-unsafe_op_in_unsafe_fn.rs:12:5
--> $DIR/edition-2024-unsafe_op_in_unsafe_fn.rs:13: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/edition-2024-unsafe_op_in_unsafe_fn.rs:11:1
--> $DIR/edition-2024-unsafe_op_in_unsafe_fn.rs:12:1
|
LL | unsafe fn foo() {
| ^^^^^^^^^^^^^^^
Expand Down
10 changes: 7 additions & 3 deletions tests/ui/unsafe/edition-2024-unsafe_op_in_unsafe_fn.rs
Original file line number Diff line number Diff line change
@@ -1,17 +1,21 @@
// edition: 2024
// compile-flags: -Zunstable-options
// check-pass
// revisions: mir thir
// [thir]compile-flags: -Zthir-unsafeck

#![crate_type = "lib"]

#![deny(unused_unsafe)]

unsafe fn unsf() {}

unsafe fn foo() {
unsf();
//~^ WARN call to unsafe function is unsafe and requires unsafe block
//[mir]~^ WARN call to unsafe function is unsafe and requires unsafe block
//[thir]~^^ WARN call to unsafe function `unsf` is unsafe and requires unsafe block

// no unused_unsafe
unsafe { unsf(); }
unsafe {
unsf();
}
}
16 changes: 16 additions & 0 deletions tests/ui/unsafe/edition-2024-unsafe_op_in_unsafe_fn.thir.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
warning: call to unsafe function `unsf` is unsafe and requires unsafe block (error E0133)
--> $DIR/edition-2024-unsafe_op_in_unsafe_fn.rs:13: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/edition-2024-unsafe_op_in_unsafe_fn.rs:12:1
|
LL | unsafe fn foo() {
| ^^^^^^^^^^^^^^^
= note: `#[warn(unsafe_op_in_unsafe_fn)]` on by default

warning: 1 warning emitted

Loading

0 comments on commit 2b59992

Please sign in to comment.