Skip to content

Commit ddd3af2

Browse files
JarchoJarcho
committed
Only lint mut_from_ref when unsafe code is used
1 parent cf1e2e9 commit ddd3af2

File tree

4 files changed

+89
-35
lines changed

4 files changed

+89
-35
lines changed

‎clippy_lints/src/ptr.rs

Lines changed: 29 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -3,16 +3,17 @@
33
use clippy_utils::diagnostics::{span_lint, span_lint_and_sugg, span_lint_and_then};
44
use clippy_utils::source::snippet_opt;
55
use clippy_utils::ty::expr_sig;
6+
use clippy_utils::visitors::contains_unsafe_block;
67
use clippy_utils::{get_expr_use_or_unification_node, is_lint_allowed, path_def_id, path_to_local, paths};
78
use if_chain::if_chain;
89
use rustc_errors::Applicability;
910
use rustc_hir::def_id::DefId;
1011
use rustc_hir::hir_id::HirIdMap;
1112
use rustc_hir::intravisit::{walk_expr, Visitor};
1213
use rustc_hir::{
13-
self as hir, AnonConst, BinOpKind, BindingAnnotation, Body, Expr, ExprKind, FnDecl, FnRetTy, GenericArg,
14+
self as hir, AnonConst, BinOpKind, BindingAnnotation, Body, Expr, ExprKind, FnRetTy, FnSig, GenericArg,
1415
ImplItemKind, ItemKind, Lifetime, LifetimeName, Mutability, Node, Param, ParamName, PatKind, QPath, TraitFn,
15-
TraitItem, TraitItemKind, TyKind,
16+
TraitItem, TraitItemKind, TyKind, Unsafety,
1617
};
1718
use rustc_lint::{LateContext, LateLintPass};
1819
use rustc_middle::hir::nested_filter;
@@ -145,7 +146,7 @@ impl<'tcx> LateLintPass<'tcx> for Ptr {
145146
return;
146147
}
147148

148-
check_mut_from_ref(cx, sig.decl);
149+
check_mut_from_ref(cx, sig, None);
149150
for arg in check_fn_args(
150151
cx,
151152
cx.tcx.fn_sig(item.def_id).skip_binder().inputs(),
@@ -170,10 +171,10 @@ impl<'tcx> LateLintPass<'tcx> for Ptr {
170171
fn check_body(&mut self, cx: &LateContext<'tcx>, body: &'tcx Body<'_>) {
171172
let hir = cx.tcx.hir();
172173
let mut parents = hir.parent_iter(body.value.hir_id);
173-
let (item_id, decl, is_trait_item) = match parents.next() {
174+
let (item_id, sig, is_trait_item) = match parents.next() {
174175
Some((_, Node::Item(i))) => {
175176
if let ItemKind::Fn(sig, ..) = &i.kind {
176-
(i.def_id, sig.decl, false)
177+
(i.def_id, sig, false)
177178
} else {
178179
return;
179180
}
@@ -185,22 +186,23 @@ impl<'tcx> LateLintPass<'tcx> for Ptr {
185186
return;
186187
}
187188
if let ImplItemKind::Fn(sig, _) = &i.kind {
188-
(i.def_id, sig.decl, false)
189+
(i.def_id, sig, false)
189190
} else {
190191
return;
191192
}
192193
},
193194
Some((_, Node::TraitItem(i))) => {
194195
if let TraitItemKind::Fn(sig, _) = &i.kind {
195-
(i.def_id, sig.decl, true)
196+
(i.def_id, sig, true)
196197
} else {
197198
return;
198199
}
199200
},
200201
_ => return,
201202
};
202203

203-
check_mut_from_ref(cx, decl);
204+
check_mut_from_ref(cx, sig, Some(body));
205+
let decl = sig.decl;
204206
let sig = cx.tcx.fn_sig(item_id).skip_binder();
205207
let lint_args: Vec<_> = check_fn_args(cx, sig.inputs(), decl.inputs, body.params)
206208
.filter(|arg| !is_trait_item || arg.mutability() == Mutability::Not)
@@ -478,31 +480,31 @@ fn check_fn_args<'cx, 'tcx: 'cx>(
478480
})
479481
}
480482

481-
fn check_mut_from_ref(cx: &LateContext<'_>, decl: &FnDecl<'_>) {
482-
if let FnRetTy::Return(ty) = decl.output {
483-
if let Some((out, Mutability::Mut, _)) = get_rptr_lm(ty) {
484-
let mut immutables = vec![];
485-
for (_, mutbl, argspan) in decl
486-
.inputs
487-
.iter()
488-
.filter_map(get_rptr_lm)
489-
.filter(|&(lt, _, _)| lt.name == out.name)
490-
{
491-
if mutbl == Mutability::Mut {
492-
return;
493-
}
494-
immutables.push(argspan);
495-
}
496-
if immutables.is_empty() {
497-
return;
498-
}
483+
fn check_mut_from_ref<'tcx>(cx: &LateContext<'tcx>, sig: &FnSig<'_>, body: Option<&'tcx Body<'_>>) {
484+
if let FnRetTy::Return(ty) = sig.decl.output
485+
&& let Some((out, Mutability::Mut, _)) = get_rptr_lm(ty)
486+
{
487+
let args: Option<Vec<_>> = sig
488+
.decl
489+
.inputs
490+
.iter()
491+
.filter_map(get_rptr_lm)
492+
.filter(|&(lt, _, _)| lt.name == out.name)
493+
.map(|(_, mutability, span)| (mutability == Mutability::Not).then(|| span))
494+
.collect();
495+
if let Some(args) = args
496+
&& !args.is_empty()
497+
&& body.map_or(true, |body| {
498+
sig.header.unsafety == Unsafety::Unsafe || contains_unsafe_block(cx, &body.value)
499+
})
500+
{
499501
span_lint_and_then(
500502
cx,
501503
MUT_FROM_REF,
502504
ty.span,
503505
"mutable borrow from immutable input(s)",
504506
|diag| {
505-
let ms = MultiSpan::from_spans(immutables);
507+
let ms = MultiSpan::from_spans(args);
506508
diag.span_note(ms, "immutable borrow here");
507509
},
508510
);

‎clippy_utils/src/visitors.rs

Lines changed: 33 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,8 @@ use rustc_hir as hir;
33
use rustc_hir::def::{DefKind, Res};
44
use rustc_hir::intravisit::{self, walk_block, walk_expr, Visitor};
55
use rustc_hir::{
6-
Arm, Block, BlockCheckMode, Body, BodyId, Expr, ExprKind, HirId, ItemId, ItemKind, Stmt, UnOp, Unsafety,
6+
Arm, Block, BlockCheckMode, Body, BodyId, Expr, ExprKind, HirId, ItemId, ItemKind, Stmt, UnOp, UnsafeSource,
7+
Unsafety,
78
};
89
use rustc_lint::LateContext;
910
use rustc_middle::hir::map::Map;
@@ -370,3 +371,34 @@ pub fn is_expr_unsafe<'tcx>(cx: &LateContext<'tcx>, e: &'tcx Expr<'_>) -> bool {
370371
v.visit_expr(e);
371372
v.is_unsafe
372373
}
374+
375+
/// Checks if the given expression contains an unsafe block
376+
pub fn contains_unsafe_block<'tcx>(cx: &LateContext<'tcx>, e: &'tcx Expr<'tcx>) -> bool {
377+
struct V<'cx, 'tcx> {
378+
cx: &'cx LateContext<'tcx>,
379+
found_unsafe: bool,
380+
}
381+
impl<'tcx> Visitor<'tcx> for V<'_, 'tcx> {
382+
type NestedFilter = nested_filter::OnlyBodies;
383+
fn nested_visit_map(&mut self) -> Self::Map {
384+
self.cx.tcx.hir()
385+
}
386+
387+
fn visit_block(&mut self, b: &'tcx Block<'_>) {
388+
if self.found_unsafe {
389+
return;
390+
}
391+
if b.rules == BlockCheckMode::UnsafeBlock(UnsafeSource::UserProvided) {
392+
self.found_unsafe = true;
393+
return;
394+
}
395+
walk_block(self, b);
396+
}
397+
}
398+
let mut v = V {
399+
cx,
400+
found_unsafe: false,
401+
};
402+
v.visit_expr(e);
403+
v.found_unsafe
404+
}

‎tests/ui/mut_from_ref.rs

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ struct Foo;
55

66
impl Foo {
77
fn this_wont_hurt_a_bit(&self) -> &mut Foo {
8-
unimplemented!()
8+
unsafe { unimplemented!() }
99
}
1010
}
1111

@@ -15,29 +15,37 @@ trait Ouch {
1515

1616
impl Ouch for Foo {
1717
fn ouch(x: &Foo) -> &mut Foo {
18-
unimplemented!()
18+
unsafe { unimplemented!() }
1919
}
2020
}
2121

2222
fn fail(x: &u32) -> &mut u16 {
23-
unimplemented!()
23+
unsafe { unimplemented!() }
2424
}
2525

2626
fn fail_lifetime<'a>(x: &'a u32, y: &mut u32) -> &'a mut u32 {
27-
unimplemented!()
27+
unsafe { unimplemented!() }
2828
}
2929

3030
fn fail_double<'a, 'b>(x: &'a u32, y: &'a u32, z: &'b mut u32) -> &'a mut u32 {
31-
unimplemented!()
31+
unsafe { unimplemented!() }
3232
}
3333

3434
// this is OK, because the result borrows y
3535
fn works<'a>(x: &u32, y: &'a mut u32) -> &'a mut u32 {
36-
unimplemented!()
36+
unsafe { unimplemented!() }
3737
}
3838

3939
// this is also OK, because the result could borrow y
4040
fn also_works<'a>(x: &'a u32, y: &'a mut u32) -> &'a mut u32 {
41+
unsafe { unimplemented!() }
42+
}
43+
44+
unsafe fn also_broken(x: &u32) -> &mut u32 {
45+
unimplemented!()
46+
}
47+
48+
fn without_unsafe(x: &u32) -> &mut u32 {
4149
unimplemented!()
4250
}
4351

‎tests/ui/mut_from_ref.stderr

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,5 +59,17 @@ note: immutable borrow here
5959
LL | fn fail_double<'a, 'b>(x: &'a u32, y: &'a u32, z: &'b mut u32) -> &'a mut u32 {
6060
| ^^^^^^^ ^^^^^^^
6161

62-
error: aborting due to 5 previous errors
62+
error: mutable borrow from immutable input(s)
63+
--> $DIR/mut_from_ref.rs:44:35
64+
|
65+
LL | unsafe fn also_broken(x: &u32) -> &mut u32 {
66+
| ^^^^^^^^
67+
|
68+
note: immutable borrow here
69+
--> $DIR/mut_from_ref.rs:44:26
70+
|
71+
LL | unsafe fn also_broken(x: &u32) -> &mut u32 {
72+
| ^^^^
73+
74+
error: aborting due to 6 previous errors
6375

0 commit comments

Comments
 (0)