1
Fork 0

Auto merge of #117229 - matthewjasper:thir-unsafeck-fixes, r=cjgillot

Thir unsafeck fixes

- Recognise thread local statics in THIR unsafeck
- Add suggestion for unsafe_op_in_unsafe_fn
- Fix unsafe checking of let expressions
This commit is contained in:
bors 2023-11-07 10:41:10 +00:00
commit 61a3eea804
24 changed files with 503 additions and 99 deletions

View file

@ -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> {
@ -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(
@ -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);
@ -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) {
@ -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);
@ -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 {
@ -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,
@ -617,6 +665,7 @@ impl UnsafeOpKind {
UnsafeOpInUnsafeFnCallToFunctionWithRequiresUnsafe {
span,
function: &with_no_trimmed_paths!(tcx.def_path_str(*did)),
unsafe_not_inherited_note,
},
),
}
@ -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]);

View file

@ -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)]
@ -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)]
@ -45,6 +49,8 @@ pub struct UnsafeOpInUnsafeFnCallToUnsafeFunctionRequiresUnsafeNameless {
pub struct UnsafeOpInUnsafeFnUseOfInlineAssemblyRequiresUnsafe {
#[label]
pub span: Span,
#[subdiagnostic]
pub unsafe_not_inherited_note: Option<UnsafeNotInheritedLintNote>,
}
#[derive(LintDiagnostic)]
@ -53,6 +59,8 @@ pub struct UnsafeOpInUnsafeFnUseOfInlineAssemblyRequiresUnsafe {
pub struct UnsafeOpInUnsafeFnInitializingTypeWithRequiresUnsafe {
#[label]
pub span: Span,
#[subdiagnostic]
pub unsafe_not_inherited_note: Option<UnsafeNotInheritedLintNote>,
}
#[derive(LintDiagnostic)]
@ -61,6 +69,8 @@ pub struct UnsafeOpInUnsafeFnInitializingTypeWithRequiresUnsafe {
pub struct UnsafeOpInUnsafeFnUseOfMutableStaticRequiresUnsafe {
#[label]
pub span: Span,
#[subdiagnostic]
pub unsafe_not_inherited_note: Option<UnsafeNotInheritedLintNote>,
}
#[derive(LintDiagnostic)]
@ -69,6 +79,8 @@ pub struct UnsafeOpInUnsafeFnUseOfMutableStaticRequiresUnsafe {
pub struct UnsafeOpInUnsafeFnUseOfExternStaticRequiresUnsafe {
#[label]
pub span: Span,
#[subdiagnostic]
pub unsafe_not_inherited_note: Option<UnsafeNotInheritedLintNote>,
}
#[derive(LintDiagnostic)]
@ -77,6 +89,8 @@ pub struct UnsafeOpInUnsafeFnUseOfExternStaticRequiresUnsafe {
pub struct UnsafeOpInUnsafeFnDerefOfRawPointerRequiresUnsafe {
#[label]
pub span: Span,
#[subdiagnostic]
pub unsafe_not_inherited_note: Option<UnsafeNotInheritedLintNote>,
}
#[derive(LintDiagnostic)]
@ -85,6 +99,8 @@ pub struct UnsafeOpInUnsafeFnDerefOfRawPointerRequiresUnsafe {
pub struct UnsafeOpInUnsafeFnAccessToUnionFieldRequiresUnsafe {
#[label]
pub span: Span,
#[subdiagnostic]
pub unsafe_not_inherited_note: Option<UnsafeNotInheritedLintNote>,
}
#[derive(LintDiagnostic)]
@ -93,6 +109,8 @@ pub struct UnsafeOpInUnsafeFnAccessToUnionFieldRequiresUnsafe {
pub struct UnsafeOpInUnsafeFnMutationOfLayoutConstrainedFieldRequiresUnsafe {
#[label]
pub span: Span,
#[subdiagnostic]
pub unsafe_not_inherited_note: Option<UnsafeNotInheritedLintNote>,
}
#[derive(LintDiagnostic)]
@ -100,6 +118,8 @@ pub struct UnsafeOpInUnsafeFnMutationOfLayoutConstrainedFieldRequiresUnsafe {
pub struct UnsafeOpInUnsafeFnBorrowOfLayoutConstrainedFieldRequiresUnsafe {
#[label]
pub span: Span,
#[subdiagnostic]
pub unsafe_not_inherited_note: Option<UnsafeNotInheritedLintNote>,
}
#[derive(LintDiagnostic)]
@ -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)]
@ -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 {