Skip to content

Commit f3d8bfb

Browse files
JarchoJarcho
committed
Code cleanup and additional std types checked
1 parent 946810a commit f3d8bfb

File tree

1 file changed

+109
-126
lines changed

1 file changed

+109
-126
lines changed

‎clippy_lints/src/matches.rs‎

Lines changed: 109 additions & 126 deletions
Original file line numberDiff line numberDiff line change
@@ -1712,9 +1712,12 @@ mod redundant_pattern_match {
17121712
use if_chain::if_chain;
17131713
use rustc_ast::ast::LitKind;
17141714
use rustc_errors::Applicability;
1715-
use rustc_hir::{Arm, Expr, ExprKind, LangItem, MatchSource, Node, PatKind, QPath, UnOp};
1715+
use rustc_hir::{
1716+
intravisit::{walk_expr, ErasedMap, NestedVisitorMap, Visitor},
1717+
Arm, Block, Expr, ExprKind, LangItem, MatchSource, Node, PatKind, QPath,
1718+
};
17161719
use rustc_lint::LateContext;
1717-
use rustc_middle::ty::{self, subst::GenericArgKind, Ty, TyCtxt, TypeckResults};
1720+
use rustc_middle::ty::{self, subst::GenericArgKind, Ty};
17181721
use rustc_span::sym;
17191722

17201723
pub fn check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
@@ -1733,34 +1736,42 @@ mod redundant_pattern_match {
17331736
// Check if the drop order for a type matters
17341737
fn type_needs_ordered_drop(cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> bool {
17351738
if !ty.needs_drop(cx.tcx, cx.param_env) {
1736-
return false;
1737-
}
1738-
if cx
1739+
false
1740+
} else if cx
17391741
.tcx
17401742
.lang_items()
17411743
.drop_trait()
17421744
.map_or(false, |id| !implements_trait(cx, ty, id, &[]))
17431745
{
17441746
// This type doesn't implement drop, so no side effects here.
17451747
// Check if any component type has any.
1746-
return match ty.kind() {
1748+
match ty.kind() {
17471749
ty::Tuple(_) => ty.tuple_fields().any(|ty| type_needs_ordered_drop(cx, ty)),
17481750
ty::Array(ty, _) => type_needs_ordered_drop(cx, ty),
17491751
ty::Adt(adt, subs) => adt
17501752
.all_fields()
17511753
.map(|f| f.ty(cx.tcx, subs))
17521754
.any(|ty| type_needs_ordered_drop(cx, ty)),
17531755
_ => true,
1754-
};
1756+
}
17551757
}
1756-
17571758
// Check for std types which implement drop, but only for memory allocation.
1758-
if is_type_diagnostic_item(cx, ty, sym::vec_type)
1759+
else if is_type_diagnostic_item(cx, ty, sym::vec_type)
17591760
|| is_type_lang_item(cx, ty, LangItem::OwnedBox)
17601761
|| is_type_diagnostic_item(cx, ty, sym::Rc)
17611762
|| is_type_diagnostic_item(cx, ty, sym::Arc)
1763+
|| is_type_diagnostic_item(cx, ty, sym::cstring_type)
1764+
|| match_type(cx, ty, &paths::BTREEMAP)
1765+
|| match_type(cx, ty, &paths::LINKED_LIST)
1766+
|| match_type(cx, ty, &paths::WEAK_RC)
1767+
|| match_type(cx, ty, &paths::WEAK_ARC)
17621768
{
1763-
try_get_generic_ty(ty, 0).map_or(true, |ty| type_needs_ordered_drop(cx, ty))
1769+
// Check all of the generic arguments.
1770+
if let ty::Adt(_, subs) = ty.kind() {
1771+
subs.types().any(|ty| type_needs_ordered_drop(cx, ty))
1772+
} else {
1773+
true
1774+
}
17641775
} else {
17651776
true
17661777
}
@@ -1780,113 +1791,84 @@ mod redundant_pattern_match {
17801791
}
17811792
}
17821793

1783-
// Gets all the the temporaries in an expression which need to be dropped afterwards.
1784-
#[allow(clippy::too_many_lines)]
1785-
fn get_temporary_tys(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) -> Vec<Ty<'tcx>> {
1786-
fn f(
1787-
addr_of: bool,
1788-
tcx: TyCtxt<'tcx>,
1789-
typeck: &TypeckResults<'tcx>,
1790-
expr: &'tcx Expr<'tcx>,
1791-
result: &mut Vec<Ty<'tcx>>,
1792-
) {
1793-
match expr.kind {
1794-
ExprKind::ConstBlock(_)
1795-
| ExprKind::Tup([])
1796-
| ExprKind::Array([])
1797-
| ExprKind::Lit(_)
1798-
| ExprKind::Path(_)
1799-
| ExprKind::Assign(..)
1800-
| ExprKind::AssignOp(..)
1801-
| ExprKind::Break(..)
1802-
| ExprKind::Continue(_)
1803-
| ExprKind::Ret(_)
1804-
| ExprKind::InlineAsm(_)
1805-
| ExprKind::LlvmInlineAsm(_)
1806-
| ExprKind::Yield(..)
1807-
| ExprKind::Err
1808-
| ExprKind::MethodCall(_, _, [], _) => (),
1809-
ExprKind::MethodCall(_, _, [self_arg, args @ ..], _) => {
1810-
if addr_of {
1811-
result.push(typeck.expr_ty(expr));
1812-
}
1813-
let ref_self = typeck
1814-
.type_dependent_def_id(expr.hir_id)
1815-
.map_or(false, |id| tcx.fn_sig(id).skip_binder().inputs()[0].is_ref());
1816-
f(ref_self, tcx, typeck, self_arg, result);
1817-
for arg in args {
1818-
f(false, tcx, typeck, arg, result);
1819-
}
1820-
},
1821-
ExprKind::AddrOf(_, _, expr) | ExprKind::Field(expr, _) => f(true, tcx, typeck, expr, result),
1822-
ExprKind::Box(expr) | ExprKind::Cast(expr, _) => f(false, tcx, typeck, expr, result),
1823-
ExprKind::Array(exprs @ [_, ..]) | ExprKind::Tup(exprs @ [_, ..]) | ExprKind::Call(_, exprs) => {
1824-
if addr_of {
1825-
result.push(typeck.expr_ty(expr));
1826-
}
1827-
for expr in exprs {
1828-
f(false, tcx, typeck, expr, result);
1829-
}
1830-
},
1831-
ExprKind::Binary(_, lhs, rhs) => {
1832-
if addr_of {
1833-
result.push(typeck.expr_ty(expr));
1834-
}
1835-
f(false, tcx, typeck, lhs, result);
1836-
f(false, tcx, typeck, rhs, result);
1837-
},
1838-
ExprKind::Unary(UnOp::UnDeref, e) => {
1839-
f(typeck.expr_ty(e).is_ref(), tcx, typeck, e, result);
1840-
},
1841-
ExprKind::Type(e, _) | ExprKind::Unary(_, e) => {
1842-
if addr_of {
1843-
result.push(typeck.expr_ty(expr));
1844-
}
1845-
f(false, tcx, typeck, e, result);
1846-
},
1847-
ExprKind::Index(base, index) => {
1848-
f(true, tcx, typeck, base, result);
1849-
f(false, tcx, typeck, index, result);
1850-
},
1851-
ExprKind::DropTemps(_) | ExprKind::Closure(..) => {
1852-
if addr_of {
1853-
result.push(typeck.expr_ty(expr));
1854-
}
1855-
},
1856-
ExprKind::Match(e, _, _) => {
1857-
if addr_of {
1858-
result.push(typeck.expr_ty(expr));
1859-
}
1860-
f(true, tcx, typeck, e, result);
1861-
},
1862-
ExprKind::Block(b, _) | ExprKind::Loop(b, _, _) => {
1863-
if addr_of {
1864-
result.push(typeck.expr_ty(expr));
1865-
}
1866-
if let Some(expr) = b.expr {
1867-
f(false, tcx, typeck, expr, result);
1868-
}
1869-
},
1870-
ExprKind::Struct(_, fields, _) => {
1871-
if addr_of {
1872-
result.push(typeck.expr_ty(expr));
1873-
}
1874-
for field in fields {
1875-
f(false, tcx, typeck, field.expr, result);
1876-
}
1877-
},
1878-
ExprKind::Repeat(expr, _) => {
1879-
if addr_of {
1880-
result.push(typeck.expr_ty(expr));
1881-
}
1882-
f(false, tcx, typeck, expr, result);
1883-
},
1794+
// Checks if there are any temporaries created in the given expression for which drop order
1795+
// matters.
1796+
fn temporaries_need_ordered_drop(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) -> bool {
1797+
struct V<'a, 'tcx> {
1798+
cx: &'a LateContext<'tcx>,
1799+
res: bool,
1800+
}
1801+
impl<'a, 'tcx> Visitor<'tcx> for V<'a, 'tcx> {
1802+
type Map = ErasedMap<'tcx>;
1803+
fn nested_visit_map(&mut self) -> NestedVisitorMap<Self::Map> {
1804+
NestedVisitorMap::None
1805+
}
1806+
1807+
fn visit_expr(&mut self, expr: &'tcx Expr<'tcx>) {
1808+
match expr.kind {
1809+
// Taking the reference of a value leaves a temporary
1810+
// e.g. In `&String::new()` the string is a temporary value.
1811+
// Remaining fields are temporary values
1812+
// e.g. In `(String::new(), 0).1` the string is a temporary value.
1813+
ExprKind::AddrOf(_, _, expr) | ExprKind::Field(expr, _) => {
1814+
if !matches!(expr.kind, ExprKind::Path(_)) {
1815+
if type_needs_ordered_drop(self.cx, self.cx.typeck_results().expr_ty(expr)) {
1816+
self.res = true;
1817+
} else {
1818+
self.visit_expr(expr);
1819+
}
1820+
}
1821+
},
1822+
// the base type is alway taken by reference.
1823+
// e.g. In `(vec![0])[0]` the vector is a temporary value.
1824+
ExprKind::Index(base, index) => {
1825+
if !matches!(base.kind, ExprKind::Path(_)) {
1826+
if type_needs_ordered_drop(self.cx, self.cx.typeck_results().expr_ty(base)) {
1827+
self.res = true;
1828+
} else {
1829+
self.visit_expr(base);
1830+
}
1831+
}
1832+
self.visit_expr(index);
1833+
},
1834+
// Method calls can take self by reference.
1835+
// e.g. In `String::new().len()` the string is a temporary value.
1836+
ExprKind::MethodCall(_, _, [self_arg, args @ ..], _) => {
1837+
if !matches!(self_arg.kind, ExprKind::Path(_)) {
1838+
let self_by_ref = self
1839+
.cx
1840+
.typeck_results()
1841+
.type_dependent_def_id(expr.hir_id)
1842+
.map_or(false, |id| self.cx.tcx.fn_sig(id).skip_binder().inputs()[0].is_ref());
1843+
if self_by_ref
1844+
&& type_needs_ordered_drop(self.cx, self.cx.typeck_results().expr_ty(self_arg))
1845+
{
1846+
self.res = true;
1847+
} else {
1848+
self.visit_expr(self_arg)
1849+
}
1850+
}
1851+
args.iter().for_each(|arg| self.visit_expr(arg));
1852+
},
1853+
// Either explicitly drops values, or changes control flow.
1854+
ExprKind::DropTemps(_)
1855+
| ExprKind::Ret(_)
1856+
| ExprKind::Break(..)
1857+
| ExprKind::Yield(..)
1858+
| ExprKind::Block(Block { expr: None, .. }, _)
1859+
| ExprKind::Loop(..) => (),
1860+
1861+
// Only consider the final expression.
1862+
ExprKind::Block(Block { expr: Some(expr), .. }, _) => self.visit_expr(expr),
1863+
1864+
_ => walk_expr(self, expr),
1865+
}
18841866
}
18851867
}
18861868

1887-
let mut res = Vec::new();
1888-
f(false, cx.tcx, cx.typeck_results(), expr, &mut res);
1889-
res
1869+
let mut v = V { cx, res: false };
1870+
v.visit_expr(expr);
1871+
v.res
18901872
}
18911873

18921874
fn find_sugg_for_if_let<'tcx>(
@@ -1904,6 +1886,8 @@ mod redundant_pattern_match {
19041886
kind = &inner.kind;
19051887
}
19061888
let op_ty = cx.typeck_results().expr_ty(op);
1889+
// Determine which function should be used, and the type contained by the corresponding
1890+
// variant.
19071891
let (good_method, inner_ty) = match kind {
19081892
PatKind::TupleStruct(ref path, ref patterns, _) if patterns.len() == 1 => {
19091893
if let PatKind::Wild = patterns[0].kind {
@@ -1934,29 +1918,28 @@ mod redundant_pattern_match {
19341918
} else {
19351919
return;
19361920
};
1937-
// drop for `None` and `Pending` do nothing
1921+
// `None` and `Pending` don't have an inner type.
19381922
(method, cx.tcx.types.unit)
19391923
},
19401924
_ => return,
19411925
};
19421926

1943-
// if this is the last expression in a block then the lifetime of the expression is extended
1944-
// past the end of the block
1927+
// If this is the last expression in a block or there is an else clause then the whole
1928+
// type needs to be considered, not just the inner type of the branch being matched on.
1929+
// Note the last expression in a block is dropped after all local bindings.
19451930
let check_ty = if has_else
1946-
|| (keyword == "if" && {
1947-
let map = cx.tcx.hir();
1948-
let id = map.get_parent_node(expr.hir_id);
1949-
id != expr.hir_id && matches!(map.find(id), Some(Node::Block(..)))
1950-
}) {
1931+
|| (keyword == "if" && matches!(cx.tcx.hir().parent_iter(expr.hir_id).next(), Some((_, Node::Block(..)))))
1932+
{
19511933
op_ty
19521934
} else {
19531935
inner_ty
19541936
};
19551937

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

19611944
// check that `while_let_on_iterator` lint does not trigger
19621945
if_chain! {

0 commit comments

Comments
 (0)