Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Thir unsafeck fixes #117229

Merged
merged 4 commits into from
Nov 7, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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: 2 additions & 1 deletion compiler/rustc_middle/src/thir/visit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -66,8 +66,9 @@ pub fn walk_expr<'a, 'tcx: 'a, V: Visitor<'a, 'tcx>>(visitor: &mut V, expr: &Exp
Use { source } => visitor.visit_expr(&visitor.thir()[source]),
NeverToAny { source } => visitor.visit_expr(&visitor.thir()[source]),
PointerCoercion { source, cast: _ } => visitor.visit_expr(&visitor.thir()[source]),
Let { expr, .. } => {
Let { expr, ref pat } => {
visitor.visit_expr(&visitor.thir()[expr]);
visitor.visit_pat(pat);
}
Loop { body } => visitor.visit_expr(&visitor.thir()[body]),
Match { scrutinee, ref arms, .. } => {
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
88 changes: 69 additions & 19 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,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add a doc-comment on this field?

}

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 @@ -394,7 +405,9 @@ impl<'a, 'tcx> Visitor<'a, 'tcx> for UnsafetyVisitor<'a, 'tcx> {
}
}
ExprKind::Deref { arg } => {
if let ExprKind::StaticRef { def_id, .. } = self.thir[arg].kind {
if let ExprKind::StaticRef { def_id, .. } | ExprKind::ThreadLocalRef(def_id) =
self.thir[arg].kind
{
if self.tcx.is_mutable_static(def_id) {
self.requires_unsafe(expr.span, UseOfMutableStatic);
} else if self.tcx.is_foreign_item(def_id) {
Expand Down Expand Up @@ -482,14 +495,6 @@ impl<'a, 'tcx> Visitor<'a, 'tcx> for UnsafetyVisitor<'a, 'tcx> {
}
}
}
ExprKind::Let { expr: expr_id, .. } => {
let let_expr = &self.thir[expr_id];
if let ty::Adt(adt_def, _) = let_expr.ty.kind()
&& adt_def.is_union()
{
self.requires_unsafe(expr.span, AccessToUnionField);
}
}
_ => {}
}
visit::walk_expr(self, expr);
Expand Down Expand Up @@ -543,7 +548,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 @@ -554,61 +574,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 @@ -617,6 +665,7 @@ impl UnsafeOpKind {
UnsafeOpInUnsafeFnCallToFunctionWithRequiresUnsafe {
span,
function: &with_no_trimmed_paths!(tcx.def_path_str(*did)),
unsafe_not_inherited_note,
},
),
}
Expand Down Expand Up @@ -831,6 +880,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
2 changes: 1 addition & 1 deletion src/tools/compiletest/src/runtest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3962,7 +3962,7 @@ impl<'test> TestCx<'test> {
// And finally, compile the fixed code and make sure it both
// succeeds and has no diagnostics.
let rustc = self.make_compile_args(
&self.testpaths.file.with_extension(UI_FIXED),
&self.expected_output_path(UI_FIXED),
TargetLocation::ThisFile(self.make_exe_name()),
emit_metadata,
AllowUnused::No,
Expand Down
Loading