Skip to content

Commit 02977cb

Browse files
JarchoJarcho
committed
Extract util functions from redundant_pattern_match 
1 parent 650a0e5 commit 02977cb

File tree

3 files changed

+188
-141
lines changed

3 files changed

+188
-141
lines changed

‎clippy_lints/src/matches/redundant_pattern_match.rs

Lines changed: 5 additions & 135 deletions
Original file line numberDiff line numberDiff line change
@@ -2,18 +2,14 @@ use super::REDUNDANT_PATTERN_MATCHING;
22
use clippy_utils::diagnostics::span_lint_and_then;
33
use clippy_utils::source::snippet;
44
use clippy_utils::sugg::Sugg;
5-
use clippy_utils::ty::{implements_trait, is_type_diagnostic_item, is_type_lang_item, match_type};
6-
use clippy_utils::{higher, match_def_path};
7-
use clippy_utils::{is_lang_ctor, is_trait_method, paths};
5+
use clippy_utils::ty::type_needs_ordered_drop;
6+
use clippy_utils::visitors::any_temporaries_need_ordered_drop;
7+
use clippy_utils::{higher, is_lang_ctor, is_trait_method, match_def_path, paths};
88
use if_chain::if_chain;
99
use rustc_ast::ast::LitKind;
10-
use rustc_data_structures::fx::FxHashSet;
1110
use rustc_errors::Applicability;
1211
use rustc_hir::LangItem::{OptionNone, PollPending};
13-
use rustc_hir::{
14-
intravisit::{walk_expr, Visitor},
15-
Arm, Block, Expr, ExprKind, LangItem, Node, Pat, PatKind, QPath, UnOp,
16-
};
12+
use rustc_hir::{Arm, Expr, ExprKind, Node, Pat, PatKind, QPath, UnOp};
1713
use rustc_lint::LateContext;
1814
use rustc_middle::ty::{self, subst::GenericArgKind, DefIdTree, Ty};
1915
use rustc_span::sym;
@@ -32,59 +28,6 @@ pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
3228
}
3329
}
3430

35-
/// Checks if the drop order for a type matters. Some std types implement drop solely to
36-
/// deallocate memory. For these types, and composites containing them, changing the drop order
37-
/// won't result in any observable side effects.
38-
fn type_needs_ordered_drop<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> bool {
39-
type_needs_ordered_drop_inner(cx, ty, &mut FxHashSet::default())
40-
}
41-
42-
fn type_needs_ordered_drop_inner<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>, seen: &mut FxHashSet<Ty<'tcx>>) -> bool {
43-
if !seen.insert(ty) {
44-
return false;
45-
}
46-
if !ty.needs_drop(cx.tcx, cx.param_env) {
47-
false
48-
} else if !cx
49-
.tcx
50-
.lang_items()
51-
.drop_trait()
52-
.map_or(false, |id| implements_trait(cx, ty, id, &[]))
53-
{
54-
// This type doesn't implement drop, so no side effects here.
55-
// Check if any component type has any.
56-
match ty.kind() {
57-
ty::Tuple(fields) => fields.iter().any(|ty| type_needs_ordered_drop_inner(cx, ty, seen)),
58-
ty::Array(ty, _) => type_needs_ordered_drop_inner(cx, *ty, seen),
59-
ty::Adt(adt, subs) => adt
60-
.all_fields()
61-
.map(|f| f.ty(cx.tcx, subs))
62-
.any(|ty| type_needs_ordered_drop_inner(cx, ty, seen)),
63-
_ => true,
64-
}
65-
}
66-
// Check for std types which implement drop, but only for memory allocation.
67-
else if is_type_diagnostic_item(cx, ty, sym::Vec)
68-
|| is_type_lang_item(cx, ty, LangItem::OwnedBox)
69-
|| is_type_diagnostic_item(cx, ty, sym::Rc)
70-
|| is_type_diagnostic_item(cx, ty, sym::Arc)
71-
|| is_type_diagnostic_item(cx, ty, sym::cstring_type)
72-
|| is_type_diagnostic_item(cx, ty, sym::BTreeMap)
73-
|| is_type_diagnostic_item(cx, ty, sym::LinkedList)
74-
|| match_type(cx, ty, &paths::WEAK_RC)
75-
|| match_type(cx, ty, &paths::WEAK_ARC)
76-
{
77-
// Check all of the generic arguments.
78-
if let ty::Adt(_, subs) = ty.kind() {
79-
subs.types().any(|ty| type_needs_ordered_drop_inner(cx, ty, seen))
80-
} else {
81-
true
82-
}
83-
} else {
84-
true
85-
}
86-
}
87-
8831
// Extract the generic arguments out of a type
8932
fn try_get_generic_ty(ty: Ty<'_>, index: usize) -> Option<Ty<'_>> {
9033
if_chain! {
@@ -99,79 +42,6 @@ fn try_get_generic_ty(ty: Ty<'_>, index: usize) -> Option<Ty<'_>> {
9942
}
10043
}
10144

102-
// Checks if there are any temporaries created in the given expression for which drop order
103-
// matters.
104-
fn temporaries_need_ordered_drop<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) -> bool {
105-
struct V<'a, 'tcx> {
106-
cx: &'a LateContext<'tcx>,
107-
res: bool,
108-
}
109-
impl<'a, 'tcx> Visitor<'tcx> for V<'a, 'tcx> {
110-
fn visit_expr(&mut self, expr: &'tcx Expr<'tcx>) {
111-
match expr.kind {
112-
// Taking the reference of a value leaves a temporary
113-
// e.g. In `&String::new()` the string is a temporary value.
114-
// Remaining fields are temporary values
115-
// e.g. In `(String::new(), 0).1` the string is a temporary value.
116-
ExprKind::AddrOf(_, _, expr) | ExprKind::Field(expr, _) => {
117-
if !matches!(expr.kind, ExprKind::Path(_)) {
118-
if type_needs_ordered_drop(self.cx, self.cx.typeck_results().expr_ty(expr)) {
119-
self.res = true;
120-
} else {
121-
self.visit_expr(expr);
122-
}
123-
}
124-
},
125-
// the base type is alway taken by reference.
126-
// e.g. In `(vec![0])[0]` the vector is a temporary value.
127-
ExprKind::Index(base, index) => {
128-
if !matches!(base.kind, ExprKind::Path(_)) {
129-
if type_needs_ordered_drop(self.cx, self.cx.typeck_results().expr_ty(base)) {
130-
self.res = true;
131-
} else {
132-
self.visit_expr(base);
133-
}
134-
}
135-
self.visit_expr(index);
136-
},
137-
// Method calls can take self by reference.
138-
// e.g. In `String::new().len()` the string is a temporary value.
139-
ExprKind::MethodCall(_, [self_arg, args @ ..], _) => {
140-
if !matches!(self_arg.kind, ExprKind::Path(_)) {
141-
let self_by_ref = self
142-
.cx
143-
.typeck_results()
144-
.type_dependent_def_id(expr.hir_id)
145-
.map_or(false, |id| self.cx.tcx.fn_sig(id).skip_binder().inputs()[0].is_ref());
146-
if self_by_ref && type_needs_ordered_drop(self.cx, self.cx.typeck_results().expr_ty(self_arg)) {
147-
self.res = true;
148-
} else {
149-
self.visit_expr(self_arg);
150-
}
151-
}
152-
args.iter().for_each(|arg| self.visit_expr(arg));
153-
},
154-
// Either explicitly drops values, or changes control flow.
155-
ExprKind::DropTemps(_)
156-
| ExprKind::Ret(_)
157-
| ExprKind::Break(..)
158-
| ExprKind::Yield(..)
159-
| ExprKind::Block(Block { expr: None, .. }, _)
160-
| ExprKind::Loop(..) => (),
161-
162-
// Only consider the final expression.
163-
ExprKind::Block(Block { expr: Some(expr), .. }, _) => self.visit_expr(expr),
164-
165-
_ => walk_expr(self, expr),
166-
}
167-
}
168-
}
169-
170-
let mut v = V { cx, res: false };
171-
v.visit_expr(expr);
172-
v.res
173-
}
174-
17545
fn find_sugg_for_if_let<'tcx>(
17646
cx: &LateContext<'tcx>,
17747
expr: &'tcx Expr<'_>,
@@ -243,7 +113,7 @@ fn find_sugg_for_if_let<'tcx>(
243113
// scrutinee would be, so they have to be considered as well.
244114
// e.g. in `if let Some(x) = foo.lock().unwrap().baz.as_ref() { .. }` the lock will be held
245115
// for the duration if body.
246-
let needs_drop = type_needs_ordered_drop(cx, check_ty) || temporaries_need_ordered_drop(cx, let_expr);
116+
let needs_drop = type_needs_ordered_drop(cx, check_ty) || any_temporaries_need_ordered_drop(cx, let_expr);
247117

248118
// check that `while_let_on_iterator` lint does not trigger
249119
if_chain! {

‎clippy_utils/src/ty.rs

Lines changed: 58 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,11 @@
33
#![allow(clippy::module_name_repetitions)]
44

55
use rustc_ast::ast::Mutability;
6-
use rustc_data_structures::fx::FxHashMap;
6+
use rustc_data_structures::fx::{FxHashMap, FxHashSet};
77
use rustc_hir as hir;
88
use rustc_hir::def::{CtorKind, DefKind, Res};
99
use rustc_hir::def_id::DefId;
10-
use rustc_hir::{Expr, TyKind, Unsafety};
10+
use rustc_hir::{Expr, LangItem, TyKind, Unsafety};
1111
use rustc_infer::infer::TyCtxtInferExt;
1212
use rustc_lint::LateContext;
1313
use rustc_middle::mir::interpret::{ConstValue, Scalar};
@@ -22,7 +22,7 @@ use rustc_trait_selection::infer::InferCtxtExt;
2222
use rustc_trait_selection::traits::query::normalize::AtExt;
2323
use std::iter;
2424

25-
use crate::{match_def_path, must_use_attr, path_res};
25+
use crate::{match_def_path, must_use_attr, path_res, paths};
2626

2727
// Checks if the given type implements copy.
2828
pub fn is_copy<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> bool {
@@ -590,3 +590,58 @@ pub fn is_c_void(cx: &LateContext<'_>, ty: Ty<'_>) -> bool {
590590
false
591591
}
592592
}
593+
594+
/// Checks if the drop order for a type matters. Some std types implement drop solely to
595+
/// deallocate memory. For these types, and composites containing them, changing the drop order
596+
/// won't result in any observable side effects.
597+
pub fn type_needs_ordered_drop<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> bool {
598+
type_needs_ordered_drop_inner(cx, ty, &mut FxHashSet::default())
599+
}
600+
601+
fn type_needs_ordered_drop_inner<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>, seen: &mut FxHashSet<Ty<'tcx>>) -> bool {
602+
if !seen.insert(ty) {
603+
return false;
604+
}
605+
if !ty.needs_drop(cx.tcx, cx.param_env) {
606+
false
607+
} else if !cx
608+
.tcx
609+
.lang_items()
610+
.drop_trait()
611+
.map_or(false, |id| implements_trait(cx, ty, id, &[]))
612+
{
613+
// This type doesn't implement drop, so no side effects here.
614+
// Check if any component type has any.
615+
match ty.kind() {
616+
ty::Tuple(fields) => fields.iter().any(|ty| type_needs_ordered_drop_inner(cx, ty, seen)),
617+
ty::Array(ty, _) => type_needs_ordered_drop_inner(cx, *ty, seen),
618+
ty::Adt(adt, subs) => adt
619+
.all_fields()
620+
.map(|f| f.ty(cx.tcx, subs))
621+
.any(|ty| type_needs_ordered_drop_inner(cx, ty, seen)),
622+
_ => true,
623+
}
624+
}
625+
// Check for std types which implement drop, but only for memory allocation.
626+
else if is_type_diagnostic_item(cx, ty, sym::Vec)
627+
|| is_type_lang_item(cx, ty, LangItem::OwnedBox)
628+
|| is_type_diagnostic_item(cx, ty, sym::Rc)
629+
|| is_type_diagnostic_item(cx, ty, sym::Arc)
630+
|| is_type_diagnostic_item(cx, ty, sym::cstring_type)
631+
|| is_type_diagnostic_item(cx, ty, sym::BTreeMap)
632+
|| is_type_diagnostic_item(cx, ty, sym::LinkedList)
633+
|| is_type_diagnostic_item(cx, ty, sym::HashSet)
634+
|| is_type_diagnostic_item(cx, ty, sym::HashMap)
635+
|| match_type(cx, ty, &paths::WEAK_RC)
636+
|| match_type(cx, ty, &paths::WEAK_ARC)
637+
{
638+
// Check all of the generic arguments.
639+
if let ty::Adt(_, subs) = ty.kind() {
640+
subs.types().any(|ty| type_needs_ordered_drop_inner(cx, ty, seen))
641+
} else {
642+
true
643+
}
644+
} else {
645+
true
646+
}
647+
}

‎clippy_utils/src/visitors.rs

Lines changed: 125 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,17 @@
11
use crate::path_to_local_id;
2+
use crate::ty::type_needs_ordered_drop;
3+
use core::ops::ControlFlow;
24
use rustc_hir as hir;
3-
use rustc_hir::def::{DefKind, Res};
5+
use rustc_hir::def::{CtorKind, DefKind, Res};
46
use rustc_hir::intravisit::{self, walk_block, walk_expr, Visitor};
57
use rustc_hir::{
6-
Arm, Block, BlockCheckMode, Body, BodyId, Expr, ExprKind, HirId, ItemId, ItemKind, Stmt, UnOp, Unsafety,
8+
Arm, Block, BlockCheckMode, Body, BodyId, Expr, ExprKind, HirId, ItemId, ItemKind, Let, QPath, Stmt, UnOp, Unsafety,
79
};
810
use rustc_lint::LateContext;
911
use rustc_middle::hir::map::Map;
1012
use rustc_middle::hir::nested_filter;
11-
use rustc_middle::ty;
13+
use rustc_middle::ty::adjustment::Adjust;
14+
use rustc_middle::ty::{self, Ty, TypeckResults};
1215

1316
/// Convenience method for creating a `Visitor` with just `visit_expr` overridden and nested
1417
/// bodies (i.e. closures) are visited.
@@ -370,3 +373,122 @@ pub fn is_expr_unsafe<'tcx>(cx: &LateContext<'tcx>, e: &'tcx Expr<'_>) -> bool {
370373
v.visit_expr(e);
371374
v.is_unsafe
372375
}
376+
377+
// Calls the given function for every unconsumed temporary created by the expression. Note the
378+
// function is only guaranteed to be called for types which need to be dropped, but it may be called
379+
// for other types.
380+
pub fn for_each_unconsumed_temporary<'tcx, B>(
381+
cx: &LateContext<'tcx>,
382+
e: &'tcx Expr<'tcx>,
383+
mut f: impl FnMut(Ty<'tcx>) -> ControlFlow<B>,
384+
) -> ControlFlow<B> {
385+
fn helper<'tcx, B>(
386+
typeck: &'tcx TypeckResults<'tcx>,
387+
consume: bool,
388+
e: &'tcx Expr<'tcx>,
389+
f: &mut impl FnMut(Ty<'tcx>) -> ControlFlow<B>,
390+
) -> ControlFlow<B> {
391+
if !consume
392+
|| matches!(
393+
typeck.expr_adjustments(e),
394+
[adjust, ..] if matches!(adjust.kind, Adjust::Borrow(_) | Adjust::Deref(_))
395+
)
396+
{
397+
match e.kind {
398+
ExprKind::Path(QPath::Resolved(None, p))
399+
if matches!(p.res, Res::Def(DefKind::Ctor(_, CtorKind::Const), _)) =>
400+
{
401+
f(typeck.expr_ty(e))?;
402+
},
403+
ExprKind::Path(_)
404+
| ExprKind::Unary(UnOp::Deref, _)
405+
| ExprKind::Index(..)
406+
| ExprKind::Field(..)
407+
| ExprKind::AddrOf(..) => (),
408+
_ => f(typeck.expr_ty(e))?,
409+
}
410+
}
411+
match e.kind {
412+
ExprKind::AddrOf(_, _, e)
413+
| ExprKind::Field(e, _)
414+
| ExprKind::Unary(UnOp::Deref, e)
415+
| ExprKind::Match(e, ..)
416+
| ExprKind::Let(&Let { init: e, .. }) => {
417+
helper(typeck, false, e, f)?;
418+
},
419+
ExprKind::Block(&Block { expr: Some(e), .. }, _)
420+
| ExprKind::Box(e)
421+
| ExprKind::Cast(e, _)
422+
| ExprKind::Unary(_, e) => {
423+
helper(typeck, true, e, f)?;
424+
},
425+
ExprKind::Call(callee, args) => {
426+
helper(typeck, true, callee, f)?;
427+
for arg in args {
428+
helper(typeck, true, arg, f)?;
429+
}
430+
},
431+
ExprKind::MethodCall(_, args, _) | ExprKind::Tup(args) | ExprKind::Array(args) => {
432+
for arg in args {
433+
helper(typeck, true, arg, f)?;
434+
}
435+
},
436+
ExprKind::Index(borrowed, consumed)
437+
| ExprKind::Assign(borrowed, consumed, _)
438+
| ExprKind::AssignOp(_, borrowed, consumed) => {
439+
helper(typeck, false, borrowed, f)?;
440+
helper(typeck, true, consumed, f)?;
441+
},
442+
ExprKind::Binary(_, lhs, rhs) => {
443+
helper(typeck, true, lhs, f)?;
444+
helper(typeck, true, rhs, f)?;
445+
},
446+
ExprKind::Struct(_, fields, default) => {
447+
for field in fields {
448+
helper(typeck, true, field.expr, f)?;
449+
}
450+
if let Some(default) = default {
451+
helper(typeck, true, default, f)?;
452+
}
453+
},
454+
ExprKind::If(_, then, else_expr) => {
455+
helper(typeck, true, then, f)?;
456+
if let Some(else_expr) = else_expr {
457+
helper(typeck, true, else_expr, f)?;
458+
}
459+
},
460+
ExprKind::Type(e, _) => {
461+
helper(typeck, consume, e, f)?;
462+
},
463+
464+
// Either drops temporaries, jumps out of the current expression, or has no sub expression.
465+
ExprKind::DropTemps(_)
466+
| ExprKind::Ret(_)
467+
| ExprKind::Break(..)
468+
| ExprKind::Yield(..)
469+
| ExprKind::Block(..)
470+
| ExprKind::Loop(..)
471+
| ExprKind::Repeat(..)
472+
| ExprKind::Lit(_)
473+
| ExprKind::ConstBlock(_)
474+
| ExprKind::Closure(..)
475+
| ExprKind::Path(_)
476+
| ExprKind::Continue(_)
477+
| ExprKind::InlineAsm(_)
478+
| ExprKind::Err => (),
479+
}
480+
ControlFlow::Continue(())
481+
}
482+
helper(cx.typeck_results(), true, e, &mut f)
483+
}
484+
485+
pub fn any_temporaries_need_ordered_drop<'tcx>(cx: &LateContext<'tcx>, e: &'tcx Expr<'tcx>) -> bool {
486+
for_each_unconsumed_temporary(cx, e, |ty| {
487+
if type_needs_ordered_drop(cx, ty) {
488+
ControlFlow::Break(())
489+
} else {
490+
ControlFlow::Continue(())
491+
}
492+
})
493+
.is_break()
494+
}

0 commit comments

Comments
 (0)