Skip to content

Commit 310f3de

Browse files
matthewjaspermatthewjasper
committed
Add suggestion to THIR unsafe_op_in_unsafe_fn lint
1 parent fd81a38 commit 310f3de

12 files changed

+366
-40
lines changed

Diff for: ‎compiler/rustc_mir_build/messages.ftl

+3
Original file line numberDiff line numberDiff line change
@@ -315,6 +315,7 @@ mir_build_unreachable_pattern = unreachable pattern
315315
.label = unreachable pattern
316316
.catchall_label = matches any value
317317
318+
mir_build_unsafe_fn_safe_body = an unsafe function restricts its caller, but its body is safe by default
318319
mir_build_unsafe_not_inherited = items do not inherit unsafety from separate enclosing items
319320
320321
mir_build_unsafe_op_in_unsafe_fn_borrow_of_layout_constrained_field_requires_unsafe =
@@ -381,3 +382,5 @@ mir_build_unused_unsafe = unnecessary `unsafe` block
381382
mir_build_unused_unsafe_enclosing_block_label = because it's nested under this `unsafe` block
382383
383384
mir_build_variant_defined_here = not covered
385+
386+
mir_build_wrap_suggestion = consider wrapping the function body in an unsafe block

Diff for: ‎compiler/rustc_mir_build/src/check_unsafety.rs

+73-10
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ struct UnsafetyVisitor<'a, 'tcx> {
3535
param_env: ParamEnv<'tcx>,
3636
inside_adt: bool,
3737
warnings: &'a mut Vec<UnusedUnsafeWarning>,
38+
suggest_unsafe_block: bool,
3839
}
3940

4041
impl<'tcx> UnsafetyVisitor<'_, 'tcx> {
@@ -95,7 +96,13 @@ impl<'tcx> UnsafetyVisitor<'_, 'tcx> {
9596
SafetyContext::UnsafeFn if unsafe_op_in_unsafe_fn_allowed => {}
9697
SafetyContext::UnsafeFn => {
9798
// unsafe_op_in_unsafe_fn is disallowed
98-
kind.emit_unsafe_op_in_unsafe_fn_lint(self.tcx, self.hir_context, span);
99+
kind.emit_unsafe_op_in_unsafe_fn_lint(
100+
self.tcx,
101+
self.hir_context,
102+
span,
103+
self.suggest_unsafe_block,
104+
);
105+
self.suggest_unsafe_block = false;
99106
}
100107
SafetyContext::Safe => {
101108
kind.emit_requires_unsafe_err(
@@ -297,6 +304,7 @@ impl<'a, 'tcx> Visitor<'a, 'tcx> for UnsafetyVisitor<'a, 'tcx> {
297304
}
298305
PatKind::InlineConstant { def, .. } => {
299306
self.visit_inner_body(*def);
307+
visit::walk_pat(self, pat);
300308
}
301309
_ => {
302310
visit::walk_pat(self, pat);
@@ -545,7 +553,32 @@ impl UnsafeOpKind {
545553
tcx: TyCtxt<'_>,
546554
hir_id: hir::HirId,
547555
span: Span,
556+
suggest_unsafe_block: bool,
548557
) {
558+
let note_non_inherited = tcx.hir().parent_iter(hir_id).find(|(id, _)| {
559+
if let Some(sig) = tcx.hir().fn_sig_by_hir_id(*id)
560+
&& sig.header.is_unsafe()
561+
{
562+
true
563+
} else {
564+
false
565+
}
566+
});
567+
let unsafe_not_inherited_note = if let Some((id, _)) = note_non_inherited {
568+
suggest_unsafe_block.then(|| {
569+
let span = tcx.hir().span(id);
570+
let signature_span = tcx.sess.source_map().guess_head_span(span);
571+
let body = tcx.hir().body_owned_by(hir_id.owner.def_id);
572+
let body_span = tcx.hir().body(body).value.span;
573+
UnsafeNotInheritedLintNote {
574+
signature_span,
575+
body_start: tcx.sess.source_map().start_point(body_span).shrink_to_hi(),
576+
body_end: tcx.sess.source_map().end_point(body_span).shrink_to_lo(),
577+
}
578+
})
579+
} else {
580+
None
581+
};
549582
// FIXME: ideally we would want to trim the def paths, but this is not
550583
// feasible with the current lint emission API (see issue #106126).
551584
match self {
@@ -556,61 +589,89 @@ impl UnsafeOpKind {
556589
UnsafeOpInUnsafeFnCallToUnsafeFunctionRequiresUnsafe {
557590
span,
558591
function: &with_no_trimmed_paths!(tcx.def_path_str(*did)),
592+
unsafe_not_inherited_note,
559593
},
560594
),
561595
CallToUnsafeFunction(None) => tcx.emit_spanned_lint(
562596
UNSAFE_OP_IN_UNSAFE_FN,
563597
hir_id,
564598
span,
565-
UnsafeOpInUnsafeFnCallToUnsafeFunctionRequiresUnsafeNameless { span },
599+
UnsafeOpInUnsafeFnCallToUnsafeFunctionRequiresUnsafeNameless {
600+
span,
601+
unsafe_not_inherited_note,
602+
},
566603
),
567604
UseOfInlineAssembly => tcx.emit_spanned_lint(
568605
UNSAFE_OP_IN_UNSAFE_FN,
569606
hir_id,
570607
span,
571-
UnsafeOpInUnsafeFnUseOfInlineAssemblyRequiresUnsafe { span },
608+
UnsafeOpInUnsafeFnUseOfInlineAssemblyRequiresUnsafe {
609+
span,
610+
unsafe_not_inherited_note,
611+
},
572612
),
573613
InitializingTypeWith => tcx.emit_spanned_lint(
574614
UNSAFE_OP_IN_UNSAFE_FN,
575615
hir_id,
576616
span,
577-
UnsafeOpInUnsafeFnInitializingTypeWithRequiresUnsafe { span },
617+
UnsafeOpInUnsafeFnInitializingTypeWithRequiresUnsafe {
618+
span,
619+
unsafe_not_inherited_note,
620+
},
578621
),
579622
UseOfMutableStatic => tcx.emit_spanned_lint(
580623
UNSAFE_OP_IN_UNSAFE_FN,
581624
hir_id,
582625
span,
583-
UnsafeOpInUnsafeFnUseOfMutableStaticRequiresUnsafe { span },
626+
UnsafeOpInUnsafeFnUseOfMutableStaticRequiresUnsafe {
627+
span,
628+
unsafe_not_inherited_note,
629+
},
584630
),
585631
UseOfExternStatic => tcx.emit_spanned_lint(
586632
UNSAFE_OP_IN_UNSAFE_FN,
587633
hir_id,
588634
span,
589-
UnsafeOpInUnsafeFnUseOfExternStaticRequiresUnsafe { span },
635+
UnsafeOpInUnsafeFnUseOfExternStaticRequiresUnsafe {
636+
span,
637+
unsafe_not_inherited_note,
638+
},
590639
),
591640
DerefOfRawPointer => tcx.emit_spanned_lint(
592641
UNSAFE_OP_IN_UNSAFE_FN,
593642
hir_id,
594643
span,
595-
UnsafeOpInUnsafeFnDerefOfRawPointerRequiresUnsafe { span },
644+
UnsafeOpInUnsafeFnDerefOfRawPointerRequiresUnsafe {
645+
span,
646+
unsafe_not_inherited_note,
647+
},
596648
),
597649
AccessToUnionField => tcx.emit_spanned_lint(
598650
UNSAFE_OP_IN_UNSAFE_FN,
599651
hir_id,
600652
span,
601-
UnsafeOpInUnsafeFnAccessToUnionFieldRequiresUnsafe { span },
653+
UnsafeOpInUnsafeFnAccessToUnionFieldRequiresUnsafe {
654+
span,
655+
unsafe_not_inherited_note,
656+
},
602657
),
603658
MutationOfLayoutConstrainedField => tcx.emit_spanned_lint(
604659
UNSAFE_OP_IN_UNSAFE_FN,
605660
hir_id,
606661
span,
607-
UnsafeOpInUnsafeFnMutationOfLayoutConstrainedFieldRequiresUnsafe { span },
662+
UnsafeOpInUnsafeFnMutationOfLayoutConstrainedFieldRequiresUnsafe {
663+
span,
664+
unsafe_not_inherited_note,
665+
},
608666
),
609667
BorrowOfLayoutConstrainedField => tcx.emit_spanned_lint(
610668
UNSAFE_OP_IN_UNSAFE_FN,
611669
hir_id,
612670
span,
613-
UnsafeOpInUnsafeFnBorrowOfLayoutConstrainedFieldRequiresUnsafe { span },
671+
UnsafeOpInUnsafeFnBorrowOfLayoutConstrainedFieldRequiresUnsafe {
672+
span,
673+
unsafe_not_inherited_note,
674+
},
614675
),
615676
CallToFunctionWith(did) => tcx.emit_spanned_lint(
616677
UNSAFE_OP_IN_UNSAFE_FN,
@@ -619,6 +680,7 @@ impl UnsafeOpKind {
619680
UnsafeOpInUnsafeFnCallToFunctionWithRequiresUnsafe {
620681
span,
621682
function: &with_no_trimmed_paths!(tcx.def_path_str(*did)),
683+
unsafe_not_inherited_note,
622684
},
623685
),
624686
}
@@ -833,6 +895,7 @@ pub fn thir_check_unsafety(tcx: TyCtxt<'_>, def: LocalDefId) {
833895
param_env: tcx.param_env(def),
834896
inside_adt: false,
835897
warnings: &mut warnings,
898+
suggest_unsafe_block: true,
836899
};
837900
visitor.visit_expr(&thir[expr]);
838901

Diff for: ‎compiler/rustc_mir_build/src/errors.rs

+44
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,8 @@ pub struct UnsafeOpInUnsafeFnCallToUnsafeFunctionRequiresUnsafe<'a> {
2929
#[label]
3030
pub span: Span,
3131
pub function: &'a str,
32+
#[subdiagnostic]
33+
pub unsafe_not_inherited_note: Option<UnsafeNotInheritedLintNote>,
3234
}
3335

3436
#[derive(LintDiagnostic)]
@@ -37,6 +39,8 @@ pub struct UnsafeOpInUnsafeFnCallToUnsafeFunctionRequiresUnsafe<'a> {
3739
pub struct UnsafeOpInUnsafeFnCallToUnsafeFunctionRequiresUnsafeNameless {
3840
#[label]
3941
pub span: Span,
42+
#[subdiagnostic]
43+
pub unsafe_not_inherited_note: Option<UnsafeNotInheritedLintNote>,
4044
}
4145

4246
#[derive(LintDiagnostic)]
@@ -45,6 +49,8 @@ pub struct UnsafeOpInUnsafeFnCallToUnsafeFunctionRequiresUnsafeNameless {
4549
pub struct UnsafeOpInUnsafeFnUseOfInlineAssemblyRequiresUnsafe {
4650
#[label]
4751
pub span: Span,
52+
#[subdiagnostic]
53+
pub unsafe_not_inherited_note: Option<UnsafeNotInheritedLintNote>,
4854
}
4955

5056
#[derive(LintDiagnostic)]
@@ -53,6 +59,8 @@ pub struct UnsafeOpInUnsafeFnUseOfInlineAssemblyRequiresUnsafe {
5359
pub struct UnsafeOpInUnsafeFnInitializingTypeWithRequiresUnsafe {
5460
#[label]
5561
pub span: Span,
62+
#[subdiagnostic]
63+
pub unsafe_not_inherited_note: Option<UnsafeNotInheritedLintNote>,
5664
}
5765

5866
#[derive(LintDiagnostic)]
@@ -61,6 +69,8 @@ pub struct UnsafeOpInUnsafeFnInitializingTypeWithRequiresUnsafe {
6169
pub struct UnsafeOpInUnsafeFnUseOfMutableStaticRequiresUnsafe {
6270
#[label]
6371
pub span: Span,
72+
#[subdiagnostic]
73+
pub unsafe_not_inherited_note: Option<UnsafeNotInheritedLintNote>,
6474
}
6575

6676
#[derive(LintDiagnostic)]
@@ -69,6 +79,8 @@ pub struct UnsafeOpInUnsafeFnUseOfMutableStaticRequiresUnsafe {
6979
pub struct UnsafeOpInUnsafeFnUseOfExternStaticRequiresUnsafe {
7080
#[label]
7181
pub span: Span,
82+
#[subdiagnostic]
83+
pub unsafe_not_inherited_note: Option<UnsafeNotInheritedLintNote>,
7284
}
7385

7486
#[derive(LintDiagnostic)]
@@ -77,6 +89,8 @@ pub struct UnsafeOpInUnsafeFnUseOfExternStaticRequiresUnsafe {
7789
pub struct UnsafeOpInUnsafeFnDerefOfRawPointerRequiresUnsafe {
7890
#[label]
7991
pub span: Span,
92+
#[subdiagnostic]
93+
pub unsafe_not_inherited_note: Option<UnsafeNotInheritedLintNote>,
8094
}
8195

8296
#[derive(LintDiagnostic)]
@@ -85,6 +99,8 @@ pub struct UnsafeOpInUnsafeFnDerefOfRawPointerRequiresUnsafe {
8599
pub struct UnsafeOpInUnsafeFnAccessToUnionFieldRequiresUnsafe {
86100
#[label]
87101
pub span: Span,
102+
#[subdiagnostic]
103+
pub unsafe_not_inherited_note: Option<UnsafeNotInheritedLintNote>,
88104
}
89105

90106
#[derive(LintDiagnostic)]
@@ -93,13 +109,17 @@ pub struct UnsafeOpInUnsafeFnAccessToUnionFieldRequiresUnsafe {
93109
pub struct UnsafeOpInUnsafeFnMutationOfLayoutConstrainedFieldRequiresUnsafe {
94110
#[label]
95111
pub span: Span,
112+
#[subdiagnostic]
113+
pub unsafe_not_inherited_note: Option<UnsafeNotInheritedLintNote>,
96114
}
97115

98116
#[derive(LintDiagnostic)]
99117
#[diag(mir_build_unsafe_op_in_unsafe_fn_borrow_of_layout_constrained_field_requires_unsafe)]
100118
pub struct UnsafeOpInUnsafeFnBorrowOfLayoutConstrainedFieldRequiresUnsafe {
101119
#[label]
102120
pub span: Span,
121+
#[subdiagnostic]
122+
pub unsafe_not_inherited_note: Option<UnsafeNotInheritedLintNote>,
103123
}
104124

105125
#[derive(LintDiagnostic)]
@@ -109,6 +129,8 @@ pub struct UnsafeOpInUnsafeFnCallToFunctionWithRequiresUnsafe<'a> {
109129
#[label]
110130
pub span: Span,
111131
pub function: &'a str,
132+
#[subdiagnostic]
133+
pub unsafe_not_inherited_note: Option<UnsafeNotInheritedLintNote>,
112134
}
113135

114136
#[derive(Diagnostic)]
@@ -376,6 +398,28 @@ pub struct UnsafeNotInheritedNote {
376398
pub span: Span,
377399
}
378400

401+
pub struct UnsafeNotInheritedLintNote {
402+
pub signature_span: Span,
403+
pub body_start: Span,
404+
pub body_end: Span,
405+
}
406+
407+
impl AddToDiagnostic for UnsafeNotInheritedLintNote {
408+
/// Add a subdiagnostic to an existing diagnostic where `f` is invoked on every message used
409+
/// (to optionally perform eager translation).
410+
fn add_to_diagnostic_with<F>(self, diag: &mut Diagnostic, _: F)
411+
where
412+
F: Fn(&mut Diagnostic, SubdiagnosticMessage) -> SubdiagnosticMessage,
413+
{
414+
diag.span_note(self.signature_span, fluent::mir_build_unsafe_fn_safe_body);
415+
diag.tool_only_multipart_suggestion(
416+
fluent::mir_build_wrap_suggestion,
417+
vec![(self.body_start, " unsafe {".into()), (self.body_end, "}".into())],
418+
Applicability::MaybeIncorrect,
419+
);
420+
}
421+
}
422+
379423
#[derive(LintDiagnostic)]
380424
#[diag(mir_build_unused_unsafe)]
381425
pub struct UnusedUnsafe {

Diff for: ‎tests/ui/unsafe/edition-2024-unsafe_op_in_unsafe_fn.stderr renamed to ‎tests/ui/unsafe/edition-2024-unsafe_op_in_unsafe_fn.mir.stderr

+2-2
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,12 @@
11
warning: call to unsafe function is unsafe and requires unsafe block (error E0133)
2-
--> $DIR/edition-2024-unsafe_op_in_unsafe_fn.rs:12:5
2+
--> $DIR/edition-2024-unsafe_op_in_unsafe_fn.rs:13:5
33
|
44
LL | unsf();
55
| ^^^^^^ call to unsafe function
66
|
77
= note: consult the function's documentation for information on how to avoid undefined behavior
88
note: an unsafe function restricts its caller, but its body is safe by default
9-
--> $DIR/edition-2024-unsafe_op_in_unsafe_fn.rs:11:1
9+
--> $DIR/edition-2024-unsafe_op_in_unsafe_fn.rs:12:1
1010
|
1111
LL | unsafe fn foo() {
1212
| ^^^^^^^^^^^^^^^

Diff for: ‎tests/ui/unsafe/edition-2024-unsafe_op_in_unsafe_fn.rs

+7-3
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,21 @@
11
// edition: 2024
22
// compile-flags: -Zunstable-options
33
// check-pass
4+
// revisions: mir thir
5+
// [thir]compile-flags: -Zthir-unsafeck
46

57
#![crate_type = "lib"]
6-
78
#![deny(unused_unsafe)]
89

910
unsafe fn unsf() {}
1011

1112
unsafe fn foo() {
1213
unsf();
13-
//~^ WARN call to unsafe function is unsafe and requires unsafe block
14+
//[mir]~^ WARN call to unsafe function is unsafe and requires unsafe block
15+
//[thir]~^^ WARN call to unsafe function `unsf` is unsafe and requires unsafe block
1416

1517
// no unused_unsafe
16-
unsafe { unsf(); }
18+
unsafe {
19+
unsf();
20+
}
1721
}

Diff for: ‎tests/ui/unsafe/edition-2024-unsafe_op_in_unsafe_fn.thir.stderr

+16
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
warning: call to unsafe function `unsf` is unsafe and requires unsafe block (error E0133)
2+
--> $DIR/edition-2024-unsafe_op_in_unsafe_fn.rs:13:5
3+
|
4+
LL | unsf();
5+
| ^^^^^^ call to unsafe function
6+
|
7+
= note: consult the function's documentation for information on how to avoid undefined behavior
8+
note: an unsafe function restricts its caller, but its body is safe by default
9+
--> $DIR/edition-2024-unsafe_op_in_unsafe_fn.rs:12:1
10+
|
11+
LL | unsafe fn foo() {
12+
| ^^^^^^^^^^^^^^^
13+
= note: `#[warn(unsafe_op_in_unsafe_fn)]` on by default
14+
15+
warning: 1 warning emitted
16+

0 commit comments

Comments
 (0)