Skip to content

Commit 9752fd7

Browse files
matthewjaspermatthewjasper
committed
Add suggestion to THIR unsafe_op_in_unsafe_fn lint
1 parent 1af5e60 commit 9752fd7

12 files changed

+358
-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

+66-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,25 @@ impl UnsafeOpKind {
545553
tcx: TyCtxt<'_>,
546554
hir_id: hir::HirId,
547555
span: Span,
556+
suggest_unsafe_block: bool,
548557
) {
558+
let parent_id = tcx.hir().get_parent_item(hir_id);
559+
let parent_owner = tcx.hir().owner(parent_id);
560+
let parent_node: hir::Node<'_> = parent_owner.into();
561+
let should_suggest = parent_node.fn_sig().map_or(false, |sig| sig.header.is_unsafe());
562+
let unsafe_not_inherited_note = if should_suggest {
563+
suggest_unsafe_block.then(|| {
564+
let body_span = tcx.hir().body(parent_owner.body_id().unwrap()).value.span;
565+
UnsafeNotInheritedLintNote {
566+
signature_span: tcx
567+
.hir()
568+
.span(tcx.hir().local_def_id_to_hir_id(parent_id.def_id)),
569+
body_span,
570+
}
571+
})
572+
} else {
573+
None
574+
};
549575
// FIXME: ideally we would want to trim the def paths, but this is not
550576
// feasible with the current lint emission API (see issue #106126).
551577
match self {
@@ -556,61 +582,89 @@ impl UnsafeOpKind {
556582
UnsafeOpInUnsafeFnCallToUnsafeFunctionRequiresUnsafe {
557583
span,
558584
function: &with_no_trimmed_paths!(tcx.def_path_str(*did)),
585+
unsafe_not_inherited_note,
559586
},
560587
),
561588
CallToUnsafeFunction(None) => tcx.emit_spanned_lint(
562589
UNSAFE_OP_IN_UNSAFE_FN,
563590
hir_id,
564591
span,
565-
UnsafeOpInUnsafeFnCallToUnsafeFunctionRequiresUnsafeNameless { span },
592+
UnsafeOpInUnsafeFnCallToUnsafeFunctionRequiresUnsafeNameless {
593+
span,
594+
unsafe_not_inherited_note,
595+
},
566596
),
567597
UseOfInlineAssembly => tcx.emit_spanned_lint(
568598
UNSAFE_OP_IN_UNSAFE_FN,
569599
hir_id,
570600
span,
571-
UnsafeOpInUnsafeFnUseOfInlineAssemblyRequiresUnsafe { span },
601+
UnsafeOpInUnsafeFnUseOfInlineAssemblyRequiresUnsafe {
602+
span,
603+
unsafe_not_inherited_note,
604+
},
572605
),
573606
InitializingTypeWith => tcx.emit_spanned_lint(
574607
UNSAFE_OP_IN_UNSAFE_FN,
575608
hir_id,
576609
span,
577-
UnsafeOpInUnsafeFnInitializingTypeWithRequiresUnsafe { span },
610+
UnsafeOpInUnsafeFnInitializingTypeWithRequiresUnsafe {
611+
span,
612+
unsafe_not_inherited_note,
613+
},
578614
),
579615
UseOfMutableStatic => tcx.emit_spanned_lint(
580616
UNSAFE_OP_IN_UNSAFE_FN,
581617
hir_id,
582618
span,
583-
UnsafeOpInUnsafeFnUseOfMutableStaticRequiresUnsafe { span },
619+
UnsafeOpInUnsafeFnUseOfMutableStaticRequiresUnsafe {
620+
span,
621+
unsafe_not_inherited_note,
622+
},
584623
),
585624
UseOfExternStatic => tcx.emit_spanned_lint(
586625
UNSAFE_OP_IN_UNSAFE_FN,
587626
hir_id,
588627
span,
589-
UnsafeOpInUnsafeFnUseOfExternStaticRequiresUnsafe { span },
628+
UnsafeOpInUnsafeFnUseOfExternStaticRequiresUnsafe {
629+
span,
630+
unsafe_not_inherited_note,
631+
},
590632
),
591633
DerefOfRawPointer => tcx.emit_spanned_lint(
592634
UNSAFE_OP_IN_UNSAFE_FN,
593635
hir_id,
594636
span,
595-
UnsafeOpInUnsafeFnDerefOfRawPointerRequiresUnsafe { span },
637+
UnsafeOpInUnsafeFnDerefOfRawPointerRequiresUnsafe {
638+
span,
639+
unsafe_not_inherited_note,
640+
},
596641
),
597642
AccessToUnionField => tcx.emit_spanned_lint(
598643
UNSAFE_OP_IN_UNSAFE_FN,
599644
hir_id,
600645
span,
601-
UnsafeOpInUnsafeFnAccessToUnionFieldRequiresUnsafe { span },
646+
UnsafeOpInUnsafeFnAccessToUnionFieldRequiresUnsafe {
647+
span,
648+
unsafe_not_inherited_note,
649+
},
602650
),
603651
MutationOfLayoutConstrainedField => tcx.emit_spanned_lint(
604652
UNSAFE_OP_IN_UNSAFE_FN,
605653
hir_id,
606654
span,
607-
UnsafeOpInUnsafeFnMutationOfLayoutConstrainedFieldRequiresUnsafe { span },
655+
UnsafeOpInUnsafeFnMutationOfLayoutConstrainedFieldRequiresUnsafe {
656+
span,
657+
unsafe_not_inherited_note,
658+
},
608659
),
609660
BorrowOfLayoutConstrainedField => tcx.emit_spanned_lint(
610661
UNSAFE_OP_IN_UNSAFE_FN,
611662
hir_id,
612663
span,
613-
UnsafeOpInUnsafeFnBorrowOfLayoutConstrainedFieldRequiresUnsafe { span },
664+
UnsafeOpInUnsafeFnBorrowOfLayoutConstrainedFieldRequiresUnsafe {
665+
span,
666+
unsafe_not_inherited_note,
667+
},
614668
),
615669
CallToFunctionWith(did) => tcx.emit_spanned_lint(
616670
UNSAFE_OP_IN_UNSAFE_FN,
@@ -619,6 +673,7 @@ impl UnsafeOpKind {
619673
UnsafeOpInUnsafeFnCallToFunctionWithRequiresUnsafe {
620674
span,
621675
function: &with_no_trimmed_paths!(tcx.def_path_str(*did)),
676+
unsafe_not_inherited_note,
622677
},
623678
),
624679
}
@@ -833,6 +888,7 @@ pub fn thir_check_unsafety(tcx: TyCtxt<'_>, def: LocalDefId) {
833888
param_env: tcx.param_env(def),
834889
inside_adt: false,
835890
warnings: &mut warnings,
891+
suggest_unsafe_block: true,
836892
};
837893
visitor.visit_expr(&thir[expr]);
838894

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

+43
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,27 @@ pub struct UnsafeNotInheritedNote {
376398
pub span: Span,
377399
}
378400

401+
pub struct UnsafeNotInheritedLintNote {
402+
pub signature_span: Span,
403+
pub body_span: Span,
404+
}
405+
406+
impl AddToDiagnostic for UnsafeNotInheritedLintNote {
407+
fn add_to_diagnostic_with<F>(self, diag: &mut Diagnostic, _: F)
408+
where
409+
F: Fn(&mut Diagnostic, SubdiagnosticMessage) -> SubdiagnosticMessage,
410+
{
411+
diag.span_note(self.signature_span, fluent::mir_build_unsafe_fn_safe_body);
412+
let body_start = self.body_span.shrink_to_lo();
413+
let body_end = self.body_span.shrink_to_hi();
414+
diag.tool_only_multipart_suggestion(
415+
fluent::mir_build_wrap_suggestion,
416+
vec![(body_start, "{ unsafe ".into()), (body_end, "}".into())],
417+
Applicability::MaybeIncorrect,
418+
);
419+
}
420+
}
421+
379422
#[derive(LintDiagnostic)]
380423
#[diag(mir_build_unused_unsafe)]
381424
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+

Diff for: ‎tests/ui/unsafe/rfc-2585-unsafe_op_in_unsafe_fn.thir.stderr

+10
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,11 @@ LL | unsf();
55
| ^^^^^^ call to unsafe function
66
|
77
= 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/rfc-2585-unsafe_op_in_unsafe_fn.rs:11:1
10+
|
11+
LL | unsafe fn deny_level() {
12+
| ^^^^^^^^^^^^^^^^^^^^^^
813
note: the lint level is defined here
914
--> $DIR/rfc-2585-unsafe_op_in_unsafe_fn.rs:4:9
1015
|
@@ -46,6 +51,11 @@ LL | unsf();
4651
| ^^^^^^ call to unsafe function
4752
|
4853
= note: consult the function's documentation for information on how to avoid undefined behavior
54+
note: an unsafe function restricts its caller, but its body is safe by default
55+
--> $DIR/rfc-2585-unsafe_op_in_unsafe_fn.rs:27:1
56+
|
57+
LL | unsafe fn warning_level() {
58+
| ^^^^^^^^^^^^^^^^^^^^^^^^^
4959
note: the lint level is defined here
5060
--> $DIR/rfc-2585-unsafe_op_in_unsafe_fn.rs:26:8
5161
|

0 commit comments

Comments
 (0)