Skip to content

Commit

Permalink
Code cleanup and additional std types checked
Browse files Browse the repository at this point in the history
  • Loading branch information
@Jarcho
Jarcho committed Apr 3, 2021
1 parent 6813466 commit 25d211a
Showing 1 changed file with 110 additions and 127 deletions.
237 changes: 110 additions & 127 deletions clippy_lints/src/matches.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1707,14 +1707,17 @@ mod redundant_pattern_match {
use super::REDUNDANT_PATTERN_MATCHING;
use clippy_utils::diagnostics::span_lint_and_then;
use clippy_utils::source::{snippet, snippet_with_applicability};
use clippy_utils::ty::{is_type_diagnostic_item, is_type_lang_item};
use clippy_utils::ty::{is_type_diagnostic_item, is_type_lang_item, match_type};
use clippy_utils::{implements_trait, is_trait_method, match_qpath, match_trait_method, paths};
use if_chain::if_chain;
use rustc_ast::ast::LitKind;
use rustc_errors::Applicability;
use rustc_hir::{Arm, Expr, ExprKind, LangItem, MatchSource, Node, PatKind, QPath, UnOp};
use rustc_hir::{
intravisit::{walk_expr, ErasedMap, NestedVisitorMap, Visitor},
Arm, Block, Expr, ExprKind, LangItem, MatchSource, Node, PatKind, QPath,
};
use rustc_lint::LateContext;
use rustc_middle::ty::{self, subst::GenericArgKind, Ty, TyCtxt, TypeckResults};
use rustc_middle::ty::{self, subst::GenericArgKind, Ty};
use rustc_span::sym;

pub fn check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
Expand All @@ -1733,34 +1736,42 @@ mod redundant_pattern_match {
// Check if the drop order for a type matters
fn type_needs_ordered_drop(cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> bool {
if !ty.needs_drop(cx.tcx, cx.param_env) {
return false;
}
if cx
false
} else if cx
.tcx
.lang_items()
.drop_trait()
.map_or(false, |id| !implements_trait(cx, ty, id, &[]))
{
// This type doesn't implement drop, so no side effects here.
// Check if any component type has any.
return match ty.kind() {
match ty.kind() {
ty::Tuple(_) => ty.tuple_fields().any(|ty| type_needs_ordered_drop(cx, ty)),
ty::Array(ty, _) => type_needs_ordered_drop(cx, ty),
ty::Adt(adt, subs) => adt
.all_fields()
.map(|f| f.ty(cx.tcx, subs))
.any(|ty| type_needs_ordered_drop(cx, ty)),
_ => true,
};
}
}

// Check for std types which implement drop, but only for memory allocation.
if is_type_diagnostic_item(cx, ty, sym::vec_type)
else if is_type_diagnostic_item(cx, ty, sym::vec_type)
|| is_type_lang_item(cx, ty, LangItem::OwnedBox)
|| is_type_diagnostic_item(cx, ty, sym::Rc)
|| is_type_diagnostic_item(cx, ty, sym::Arc)
|| is_type_diagnostic_item(cx, ty, sym::cstring_type)
|| match_type(cx, ty, &paths::BTREEMAP)
|| match_type(cx, ty, &paths::LINKED_LIST)
|| match_type(cx, ty, &paths::WEAK_RC)
|| match_type(cx, ty, &paths::WEAK_ARC)
{
try_get_generic_ty(ty, 0).map_or(true, |ty| type_needs_ordered_drop(cx, ty))
// Check all of the generic arguments.
if let ty::Adt(_, subs) = ty.kind() {
subs.types().any(|ty| type_needs_ordered_drop(cx, ty))
} else {
true
}
} else {
true
}
Expand All @@ -1780,113 +1791,84 @@ mod redundant_pattern_match {
}
}

// Gets all the the temporaries in an expression which need to be dropped afterwards.
#[allow(clippy::too_many_lines)]
fn get_temporary_tys(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) -> Vec<Ty<'tcx>> {
fn f(
addr_of: bool,
tcx: TyCtxt<'tcx>,
typeck: &TypeckResults<'tcx>,
expr: &'tcx Expr<'tcx>,
result: &mut Vec<Ty<'tcx>>,
) {
match expr.kind {
ExprKind::ConstBlock(_)
| ExprKind::Tup([])
| ExprKind::Array([])
| ExprKind::Lit(_)
| ExprKind::Path(_)
| ExprKind::Assign(..)
| ExprKind::AssignOp(..)
| ExprKind::Break(..)
| ExprKind::Continue(_)
| ExprKind::Ret(_)
| ExprKind::InlineAsm(_)
| ExprKind::LlvmInlineAsm(_)
| ExprKind::Yield(..)
| ExprKind::Err
| ExprKind::MethodCall(_, _, [], _) => (),
ExprKind::MethodCall(_, _, [self_arg, args @ ..], _) => {
if addr_of {
result.push(typeck.expr_ty(expr));
}
let ref_self = typeck
.type_dependent_def_id(expr.hir_id)
.map_or(false, |id| tcx.fn_sig(id).skip_binder().inputs()[0].is_ref());
f(ref_self, tcx, typeck, self_arg, result);
for arg in args {
f(false, tcx, typeck, arg, result);
}
},
ExprKind::AddrOf(_, _, expr) | ExprKind::Field(expr, _) => f(true, tcx, typeck, expr, result),
ExprKind::Box(expr) | ExprKind::Cast(expr, _) => f(false, tcx, typeck, expr, result),
ExprKind::Array(exprs @ [_, ..]) | ExprKind::Tup(exprs @ [_, ..]) | ExprKind::Call(_, exprs) => {
if addr_of {
result.push(typeck.expr_ty(expr));
}
for expr in exprs {
f(false, tcx, typeck, expr, result);
}
},
ExprKind::Binary(_, lhs, rhs) => {
if addr_of {
result.push(typeck.expr_ty(expr));
}
f(false, tcx, typeck, lhs, result);
f(false, tcx, typeck, rhs, result);
},
ExprKind::Unary(UnOp::UnDeref, e) => {
f(typeck.expr_ty(e).is_ref(), tcx, typeck, e, result);
},
ExprKind::Type(e, _) | ExprKind::Unary(_, e) => {
if addr_of {
result.push(typeck.expr_ty(expr));
}
f(false, tcx, typeck, e, result);
},
ExprKind::Index(base, index) => {
f(true, tcx, typeck, base, result);
f(false, tcx, typeck, index, result);
},
ExprKind::DropTemps(_) | ExprKind::Closure(..) => {
if addr_of {
result.push(typeck.expr_ty(expr));
}
},
ExprKind::Match(e, _, _) => {
if addr_of {
result.push(typeck.expr_ty(expr));
}
f(true, tcx, typeck, e, result);
},
ExprKind::Block(b, _) | ExprKind::Loop(b, _, _) => {
if addr_of {
result.push(typeck.expr_ty(expr));
}
if let Some(expr) = b.expr {
f(false, tcx, typeck, expr, result);
}
},
ExprKind::Struct(_, fields, _) => {
if addr_of {
result.push(typeck.expr_ty(expr));
}
for field in fields {
f(false, tcx, typeck, field.expr, result);
}
},
ExprKind::Repeat(expr, _) => {
if addr_of {
result.push(typeck.expr_ty(expr));
}
f(false, tcx, typeck, expr, result);
},
// Checks if there are any temporaries created in the given expression for which drop order
// matters.
fn temporaries_need_ordered_drop(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) -> bool {
struct V<'a, 'tcx> {
cx: &'a LateContext<'tcx>,
res: bool,
}
impl<'a, 'tcx> Visitor<'tcx> for V<'a, 'tcx> {
type Map = ErasedMap<'tcx>;
fn nested_visit_map(&mut self) -> NestedVisitorMap<Self::Map> {
NestedVisitorMap::None
}

fn visit_expr(&mut self, expr: &'tcx Expr<'tcx>) {
match expr.kind {
// Taking the reference of a value leaves a temporary
// e.g. In `&String::new()` the string is a temporary value.
// Remaining fields are temporary values
// e.g. In `(String::new(), 0).1` the string is a temporary value.
ExprKind::AddrOf(_, _, expr) | ExprKind::Field(expr, _) => {
if !matches!(expr.kind, ExprKind::Path(_)) {
if type_needs_ordered_drop(self.cx, self.cx.typeck_results().expr_ty(expr)) {
self.res = true;
} else {
self.visit_expr(expr);
}
}
},
// the base type is alway taken by reference.
// e.g. In `(vec![0])[0]` the vector is a temporary value.
ExprKind::Index(base, index) => {
if !matches!(base.kind, ExprKind::Path(_)) {
if type_needs_ordered_drop(self.cx, self.cx.typeck_results().expr_ty(base)) {
self.res = true;
} else {
self.visit_expr(base);
}
}
self.visit_expr(index);
},
// Method calls can take self by reference.
// e.g. In `String::new().len()` the string is a temporary value.
ExprKind::MethodCall(_, _, [self_arg, args @ ..], _) => {
if !matches!(self_arg.kind, ExprKind::Path(_)) {
let self_by_ref = self
.cx
.typeck_results()
.type_dependent_def_id(expr.hir_id)
.map_or(false, |id| self.cx.tcx.fn_sig(id).skip_binder().inputs()[0].is_ref());
if self_by_ref
&& type_needs_ordered_drop(self.cx, self.cx.typeck_results().expr_ty(self_arg))
{
self.res = true;
} else {
self.visit_expr(self_arg)
}
}
args.iter().for_each(|arg| self.visit_expr(arg));
},
// Either explicitly drops values, or changes control flow.
ExprKind::DropTemps(_)
| ExprKind::Ret(_)
| ExprKind::Break(..)
| ExprKind::Yield(..)
| ExprKind::Block(Block { expr: None, .. }, _)
| ExprKind::Loop(..) => (),

// Only consider the final expression.
ExprKind::Block(Block { expr: Some(expr), .. }, _) => self.visit_expr(expr),

_ => walk_expr(self, expr),
}
}
}

let mut res = Vec::new();
f(false, cx.tcx, cx.typeck_results(), expr, &mut res);
res
let mut v = V { cx, res: false };
v.visit_expr(expr);
v.res
}

fn find_sugg_for_if_let<'tcx>(
Expand All @@ -1904,6 +1886,8 @@ mod redundant_pattern_match {
kind = &inner.kind;
}
let op_ty = cx.typeck_results().expr_ty(op);
// Determine which function should be used, and the type contained by the corresponding
// variant.
let (good_method, inner_ty) = match kind {
PatKind::TupleStruct(ref path, ref patterns, _) if patterns.len() == 1 => {
if let PatKind::Wild = patterns[0].kind {
Expand Down Expand Up @@ -1934,29 +1918,28 @@ mod redundant_pattern_match {
} else {
return;
};
// drop for `None` and `Pending` do nothing
// `None` and `Pending` don't have an inner type.
(method, cx.tcx.types.unit)
},
_ => return,
};

// if this is the last expression in a block then the lifetime of the expression is extended
// past the end of the block
// If this is the last expression in a block or there is an else clause then the whole
// type needs to be considered, not just the inner type of the branch being matched on.
// Note the last expression in a block is dropped after all local bindings.
let check_ty = if has_else
|| (keyword == "if" && {
let map = cx.tcx.hir();
let id = map.get_parent_node(expr.hir_id);
id != expr.hir_id && matches!(map.find(id), Some(Node::Block(..)))
}) {
|| (keyword == "if" && matches!(cx.tcx.hir().parent_iter(expr.hir_id).next(), Some((_, Node::Block(..)))))
{
op_ty
} else {
inner_ty
};

let needs_drop = type_needs_ordered_drop(cx, check_ty)
|| get_temporary_tys(cx, op)
.iter()
.any(|ty| type_needs_ordered_drop(cx, ty));
// All temporaries created in the scrutinee expression are dropped at the same time as the
// scrutinee would be, so they have to be considered as well.
// e.g. in `if let Some(x) = foo.lock().unwrap().baz.as_ref() { .. }` the lock will be held
// for the duration if body.
let needs_drop = type_needs_ordered_drop(cx, check_ty) || temporaries_need_ordered_drop(cx, op);

// check that `while_let_on_iterator` lint does not trigger
if_chain! {
Expand Down

0 comments on commit 25d211a

Please sign in to comment.