From 4ac14f9e63ba71e7f15204f9cff208f022563996 Mon Sep 17 00:00:00 2001 From: Cameron Steffen Date: Wed, 17 Feb 2021 09:34:35 -0600 Subject: [PATCH 1/3] Teach SpanlessEq binding IDs --- clippy_lints/src/utils/hir_utils.rs | 134 ++++++++++++++++++---------- tests/ui/collapsible_match.rs | 8 ++ tests/ui/if_same_then_else2.rs | 6 +- tests/ui/if_same_then_else2.stderr | 4 +- 4 files changed, 102 insertions(+), 50 deletions(-) diff --git a/clippy_lints/src/utils/hir_utils.rs b/clippy_lints/src/utils/hir_utils.rs index c5870dc51248..0a9719901f06 100644 --- a/clippy_lints/src/utils/hir_utils.rs +++ b/clippy_lints/src/utils/hir_utils.rs @@ -1,10 +1,12 @@ use crate::consts::{constant_context, constant_simple}; use crate::utils::differing_macro_contexts; use rustc_ast::ast::InlineAsmTemplatePiece; +use rustc_data_structures::fx::FxHashMap; use rustc_data_structures::stable_hasher::{HashStable, StableHasher}; +use rustc_hir::def::Res; use rustc_hir::{ BinOpKind, Block, BlockCheckMode, BodyId, BorrowKind, CaptureBy, Expr, ExprKind, Field, FieldPat, FnRetTy, - GenericArg, GenericArgs, Guard, InlineAsmOperand, Lifetime, LifetimeName, ParamName, Pat, PatKind, Path, + GenericArg, GenericArgs, Guard, HirId, InlineAsmOperand, Lifetime, LifetimeName, ParamName, Pat, PatKind, Path, PathSegment, QPath, Stmt, StmtKind, Ty, TyKind, TypeBinding, }; use rustc_lint::LateContext; @@ -52,8 +54,47 @@ impl<'a, 'tcx> SpanlessEq<'a, 'tcx> { } } - /// Checks whether two statements are the same. - pub fn eq_stmt(&mut self, left: &Stmt<'_>, right: &Stmt<'_>) -> bool { + /// Use this method to wrap comparisons that may involve inter-expression context. + /// See `self.locals`. + fn inter_expr(&mut self) -> HirEqInterExpr<'_, 'a, 'tcx> { + HirEqInterExpr { + inner: self, + locals: FxHashMap::default(), + } + } + + pub fn eq_block(&mut self, left: &Block<'_>, right: &Block<'_>) -> bool { + self.inter_expr().eq_block(left, right) + } + + pub fn eq_expr(&mut self, left: &Expr<'_>, right: &Expr<'_>) -> bool { + self.inter_expr().eq_expr(left, right) + } + + pub fn eq_path_segment(&mut self, left: &PathSegment<'_>, right: &PathSegment<'_>) -> bool { + self.inter_expr().eq_path_segment(left, right) + } + + pub fn eq_path_segments(&mut self, left: &[PathSegment<'_>], right: &[PathSegment<'_>]) -> bool { + self.inter_expr().eq_path_segments(left, right) + } + + pub fn eq_ty_kind(&mut self, left: &TyKind<'_>, right: &TyKind<'_>) -> bool { + self.inter_expr().eq_ty_kind(left, right) + } +} + +struct HirEqInterExpr<'a, 'b, 'tcx> { + inner: &'a mut SpanlessEq<'b, 'tcx>, + + // When binding are declared, the binding ID in the left expression is mapped to the one on the + // right. For example, when comparing `{ let x = 1; x + 2 }` and `{ let y = 1; y + 2 }`, + // these blocks are considered equal since `x` is mapped to `y`. + locals: FxHashMap, +} + +impl HirEqInterExpr<'_, '_, '_> { + fn eq_stmt(&mut self, left: &Stmt<'_>, right: &Stmt<'_>) -> bool { match (&left.kind, &right.kind) { (&StmtKind::Local(ref l), &StmtKind::Local(ref r)) => { self.eq_pat(&l.pat, &r.pat) @@ -68,21 +109,21 @@ impl<'a, 'tcx> SpanlessEq<'a, 'tcx> { } /// Checks whether two blocks are the same. - pub fn eq_block(&mut self, left: &Block<'_>, right: &Block<'_>) -> bool { + fn eq_block(&mut self, left: &Block<'_>, right: &Block<'_>) -> bool { over(&left.stmts, &right.stmts, |l, r| self.eq_stmt(l, r)) && both(&left.expr, &right.expr, |l, r| self.eq_expr(l, r)) } #[allow(clippy::similar_names)] - pub fn eq_expr(&mut self, left: &Expr<'_>, right: &Expr<'_>) -> bool { - if !self.allow_side_effects && differing_macro_contexts(left.span, right.span) { + fn eq_expr(&mut self, left: &Expr<'_>, right: &Expr<'_>) -> bool { + if !self.inner.allow_side_effects && differing_macro_contexts(left.span, right.span) { return false; } - if let Some(typeck_results) = self.maybe_typeck_results { + if let Some(typeck_results) = self.inner.maybe_typeck_results { if let (Some(l), Some(r)) = ( - constant_simple(self.cx, typeck_results, left), - constant_simple(self.cx, typeck_results, right), + constant_simple(self.inner.cx, typeck_results, left), + constant_simple(self.inner.cx, typeck_results, right), ) { if l == r { return true; @@ -98,10 +139,10 @@ impl<'a, 'tcx> SpanlessEq<'a, 'tcx> { both(&li.label, &ri.label, |l, r| l.ident.name == r.ident.name) }, (&ExprKind::Assign(ref ll, ref lr, _), &ExprKind::Assign(ref rl, ref rr, _)) => { - self.allow_side_effects && self.eq_expr(ll, rl) && self.eq_expr(lr, rr) + self.inner.allow_side_effects && self.eq_expr(ll, rl) && self.eq_expr(lr, rr) }, (&ExprKind::AssignOp(ref lo, ref ll, ref lr), &ExprKind::AssignOp(ref ro, ref rl, ref rr)) => { - self.allow_side_effects && lo.node == ro.node && self.eq_expr(ll, rl) && self.eq_expr(lr, rr) + self.inner.allow_side_effects && lo.node == ro.node && self.eq_expr(ll, rl) && self.eq_expr(lr, rr) }, (&ExprKind::Block(ref l, _), &ExprKind::Block(ref r, _)) => self.eq_block(l, r), (&ExprKind::Binary(l_op, ref ll, ref lr), &ExprKind::Binary(r_op, ref rl, ref rr)) => { @@ -116,7 +157,7 @@ impl<'a, 'tcx> SpanlessEq<'a, 'tcx> { }, (&ExprKind::Box(ref l), &ExprKind::Box(ref r)) => self.eq_expr(l, r), (&ExprKind::Call(l_fun, l_args), &ExprKind::Call(r_fun, r_args)) => { - self.allow_side_effects && self.eq_expr(l_fun, r_fun) && self.eq_exprs(l_args, r_args) + self.inner.allow_side_effects && self.eq_expr(l_fun, r_fun) && self.eq_exprs(l_args, r_args) }, (&ExprKind::Cast(ref lx, ref lt), &ExprKind::Cast(ref rx, ref rt)) | (&ExprKind::Type(ref lx, ref lt), &ExprKind::Type(ref rx, ref rt)) => { @@ -139,19 +180,19 @@ impl<'a, 'tcx> SpanlessEq<'a, 'tcx> { ls == rs && self.eq_expr(le, re) && over(la, ra, |l, r| { - self.eq_expr(&l.body, &r.body) + self.eq_pat(&l.pat, &r.pat) && both(&l.guard, &r.guard, |l, r| self.eq_guard(l, r)) - && self.eq_pat(&l.pat, &r.pat) + && self.eq_expr(&l.body, &r.body) }) }, (&ExprKind::MethodCall(l_path, _, l_args, _), &ExprKind::MethodCall(r_path, _, r_args, _)) => { - self.allow_side_effects && self.eq_path_segment(l_path, r_path) && self.eq_exprs(l_args, r_args) + self.inner.allow_side_effects && self.eq_path_segment(l_path, r_path) && self.eq_exprs(l_args, r_args) }, (&ExprKind::Repeat(ref le, ref ll_id), &ExprKind::Repeat(ref re, ref rl_id)) => { - let mut celcx = constant_context(self.cx, self.cx.tcx.typeck_body(ll_id.body)); - let ll = celcx.expr(&self.cx.tcx.hir().body(ll_id.body).value); - let mut celcx = constant_context(self.cx, self.cx.tcx.typeck_body(rl_id.body)); - let rl = celcx.expr(&self.cx.tcx.hir().body(rl_id.body).value); + let mut celcx = constant_context(self.inner.cx, self.inner.cx.tcx.typeck_body(ll_id.body)); + let ll = celcx.expr(&self.inner.cx.tcx.hir().body(ll_id.body).value); + let mut celcx = constant_context(self.inner.cx, self.inner.cx.tcx.typeck_body(rl_id.body)); + let rl = celcx.expr(&self.inner.cx.tcx.hir().body(rl_id.body).value); self.eq_expr(le, re) && ll == rl }, @@ -168,7 +209,7 @@ impl<'a, 'tcx> SpanlessEq<'a, 'tcx> { (&ExprKind::DropTemps(ref le), &ExprKind::DropTemps(ref re)) => self.eq_expr(le, re), _ => false, }; - is_eq || self.expr_fallback.as_ref().map_or(false, |f| f(left, right)) + is_eq || self.inner.expr_fallback.as_ref().map_or(false, |f| f(left, right)) } fn eq_exprs(&mut self, left: &[Expr<'_>], right: &[Expr<'_>]) -> bool { @@ -199,13 +240,13 @@ impl<'a, 'tcx> SpanlessEq<'a, 'tcx> { left.name == right.name } - pub fn eq_fieldpat(&mut self, left: &FieldPat<'_>, right: &FieldPat<'_>) -> bool { + fn eq_fieldpat(&mut self, left: &FieldPat<'_>, right: &FieldPat<'_>) -> bool { let (FieldPat { ident: li, pat: lp, .. }, FieldPat { ident: ri, pat: rp, .. }) = (&left, &right); li.name == ri.name && self.eq_pat(lp, rp) } /// Checks whether two patterns are the same. - pub fn eq_pat(&mut self, left: &Pat<'_>, right: &Pat<'_>) -> bool { + fn eq_pat(&mut self, left: &Pat<'_>, right: &Pat<'_>) -> bool { match (&left.kind, &right.kind) { (&PatKind::Box(ref l), &PatKind::Box(ref r)) => self.eq_pat(l, r), (&PatKind::Struct(ref lp, ref la, ..), &PatKind::Struct(ref rp, ref ra, ..)) => { @@ -214,8 +255,12 @@ impl<'a, 'tcx> SpanlessEq<'a, 'tcx> { (&PatKind::TupleStruct(ref lp, ref la, ls), &PatKind::TupleStruct(ref rp, ref ra, rs)) => { self.eq_qpath(lp, rp) && over(la, ra, |l, r| self.eq_pat(l, r)) && ls == rs }, - (&PatKind::Binding(ref lb, .., ref li, ref lp), &PatKind::Binding(ref rb, .., ref ri, ref rp)) => { - lb == rb && li.name == ri.name && both(lp, rp, |l, r| self.eq_pat(l, r)) + (&PatKind::Binding(lb, li, _, ref lp), &PatKind::Binding(rb, ri, _, ref rp)) => { + let eq = lb == rb && both(lp, rp, |l, r| self.eq_pat(l, r)); + if eq { + self.locals.insert(li, ri); + } + eq }, (&PatKind::Path(ref l), &PatKind::Path(ref r)) => self.eq_qpath(l, r), (&PatKind::Lit(ref l), &PatKind::Lit(ref r)) => self.eq_expr(l, r), @@ -251,8 +296,11 @@ impl<'a, 'tcx> SpanlessEq<'a, 'tcx> { } fn eq_path(&mut self, left: &Path<'_>, right: &Path<'_>) -> bool { - left.is_global() == right.is_global() - && over(&left.segments, &right.segments, |l, r| self.eq_path_segment(l, r)) + match (left.res, right.res) { + (Res::Local(l), Res::Local(r)) => l == r || self.locals.get(&l) == Some(&r), + (Res::Local(_), _) | (_, Res::Local(_)) => false, + _ => over(&left.segments, &right.segments, |l, r| self.eq_path_segment(l, r)), + } } fn eq_path_parameters(&mut self, left: &GenericArgs<'_>, right: &GenericArgs<'_>) -> bool { @@ -279,28 +327,19 @@ impl<'a, 'tcx> SpanlessEq<'a, 'tcx> { left.ident.name == right.ident.name && both(&left.args, &right.args, |l, r| self.eq_path_parameters(l, r)) } - pub fn eq_ty(&mut self, left: &Ty<'_>, right: &Ty<'_>) -> bool { + fn eq_ty(&mut self, left: &Ty<'_>, right: &Ty<'_>) -> bool { self.eq_ty_kind(&left.kind, &right.kind) } #[allow(clippy::similar_names)] - pub fn eq_ty_kind(&mut self, left: &TyKind<'_>, right: &TyKind<'_>) -> bool { + fn eq_ty_kind(&mut self, left: &TyKind<'_>, right: &TyKind<'_>) -> bool { match (left, right) { (&TyKind::Slice(ref l_vec), &TyKind::Slice(ref r_vec)) => self.eq_ty(l_vec, r_vec), (&TyKind::Array(ref lt, ref ll_id), &TyKind::Array(ref rt, ref rl_id)) => { - let old_maybe_typeck_results = self.maybe_typeck_results; - - let mut celcx = constant_context(self.cx, self.cx.tcx.typeck_body(ll_id.body)); - self.maybe_typeck_results = Some(self.cx.tcx.typeck_body(ll_id.body)); - let ll = celcx.expr(&self.cx.tcx.hir().body(ll_id.body).value); - - let mut celcx = constant_context(self.cx, self.cx.tcx.typeck_body(rl_id.body)); - self.maybe_typeck_results = Some(self.cx.tcx.typeck_body(rl_id.body)); - let rl = celcx.expr(&self.cx.tcx.hir().body(rl_id.body).value); - - let eq_ty = self.eq_ty(lt, rt); - self.maybe_typeck_results = old_maybe_typeck_results; - eq_ty && ll == rl + let cx = self.inner.cx; + let eval_const = + |body| constant_context(cx, cx.tcx.typeck_body(body)).expr(&cx.tcx.hir().body(body).value); + self.eq_ty(lt, rt) && eval_const(ll_id.body) == eval_const(rl_id.body) }, (&TyKind::Ptr(ref l_mut), &TyKind::Ptr(ref r_mut)) => { l_mut.mutbl == r_mut.mutbl && self.eq_ty(&*l_mut.ty, &*r_mut.ty) @@ -667,10 +706,15 @@ impl<'a, 'tcx> SpanlessHash<'a, 'tcx> { // self.maybe_typeck_results.unwrap().qpath_res(p, id).hash(&mut self.s); } - pub fn hash_path(&mut self, p: &Path<'_>) { - p.is_global().hash(&mut self.s); - for p in p.segments { - self.hash_name(p.ident.name); + pub fn hash_path(&mut self, path: &Path<'_>) { + match path.res { + // constant hash since equality is dependant on inter-expression context + Res::Local(_) => 1_usize.hash(&mut self.s), + _ => { + for seg in path.segments { + self.hash_name(seg.ident.name); + } + }, } } diff --git a/tests/ui/collapsible_match.rs b/tests/ui/collapsible_match.rs index 3294da7e8146..55467cf4229d 100644 --- a/tests/ui/collapsible_match.rs +++ b/tests/ui/collapsible_match.rs @@ -232,6 +232,14 @@ fn negative_cases(res_opt: Result, String>, res_res: Result match e { + Some(e) => e, + e => e, + }, + // else branch looks the same but the binding is different + e => e, + }; } fn make() -> T { diff --git a/tests/ui/if_same_then_else2.rs b/tests/ui/if_same_then_else2.rs index e83ce47e5630..a2ff1b741ca2 100644 --- a/tests/ui/if_same_then_else2.rs +++ b/tests/ui/if_same_then_else2.rs @@ -12,7 +12,7 @@ fn if_same_then_else2() -> Result<&'static str, ()> { if true { for _ in &[42] { let foo: &Option<_> = &Some::(42); - if true { + if foo.is_some() { break; } else { continue; @@ -21,8 +21,8 @@ fn if_same_then_else2() -> Result<&'static str, ()> { } else { //~ ERROR same body as `if` block for _ in &[42] { - let foo: &Option<_> = &Some::(42); - if true { + let bar: &Option<_> = &Some::(42); + if bar.is_some() { break; } else { continue; diff --git a/tests/ui/if_same_then_else2.stderr b/tests/ui/if_same_then_else2.stderr index f98e30fa376f..454322d8aacd 100644 --- a/tests/ui/if_same_then_else2.stderr +++ b/tests/ui/if_same_then_else2.stderr @@ -5,7 +5,7 @@ LL | } else { | ____________^ LL | | //~ ERROR same body as `if` block LL | | for _ in &[42] { -LL | | let foo: &Option<_> = &Some::(42); +LL | | let bar: &Option<_> = &Some::(42); ... | LL | | } LL | | } @@ -19,7 +19,7 @@ LL | if true { | _____________^ LL | | for _ in &[42] { LL | | let foo: &Option<_> = &Some::(42); -LL | | if true { +LL | | if foo.is_some() { ... | LL | | } LL | | } else { From 742922a41a07653ec85b81a3be2c5788707e10d9 Mon Sep 17 00:00:00 2001 From: Cameron Steffen Date: Wed, 17 Feb 2021 09:35:55 -0600 Subject: [PATCH 2/3] Make expr_fallback FnMut --- clippy_lints/src/utils/hir_utils.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/clippy_lints/src/utils/hir_utils.rs b/clippy_lints/src/utils/hir_utils.rs index 0a9719901f06..c042990b759e 100644 --- a/clippy_lints/src/utils/hir_utils.rs +++ b/clippy_lints/src/utils/hir_utils.rs @@ -26,7 +26,7 @@ pub struct SpanlessEq<'a, 'tcx> { cx: &'a LateContext<'tcx>, maybe_typeck_results: Option<&'tcx TypeckResults<'tcx>>, allow_side_effects: bool, - expr_fallback: Option, &Expr<'_>) -> bool + 'a>>, + expr_fallback: Option, &Expr<'_>) -> bool + 'a>>, } impl<'a, 'tcx> SpanlessEq<'a, 'tcx> { @@ -47,7 +47,7 @@ impl<'a, 'tcx> SpanlessEq<'a, 'tcx> { } } - pub fn expr_fallback(self, expr_fallback: impl Fn(&Expr<'_>, &Expr<'_>) -> bool + 'a) -> Self { + pub fn expr_fallback(self, expr_fallback: impl FnMut(&Expr<'_>, &Expr<'_>) -> bool + 'a) -> Self { Self { expr_fallback: Some(Box::new(expr_fallback)), ..self @@ -209,7 +209,7 @@ impl HirEqInterExpr<'_, '_, '_> { (&ExprKind::DropTemps(ref le), &ExprKind::DropTemps(ref re)) => self.eq_expr(le, re), _ => false, }; - is_eq || self.inner.expr_fallback.as_ref().map_or(false, |f| f(left, right)) + is_eq || self.inner.expr_fallback.as_mut().map_or(false, |f| f(left, right)) } fn eq_exprs(&mut self, left: &[Expr<'_>], right: &[Expr<'_>]) -> bool { From 9ad6e263c9eec118a37cdbd5e182afaaad42840a Mon Sep 17 00:00:00 2001 From: Cameron Steffen Date: Wed, 17 Feb 2021 10:02:22 -0600 Subject: [PATCH 3/3] Fix match_same_arms with SpanlessEq changes --- clippy_lints/src/matches.rs | 113 ++++++++++++++++-------------------- 1 file changed, 50 insertions(+), 63 deletions(-) diff --git a/clippy_lints/src/matches.rs b/clippy_lints/src/matches.rs index e33001b16bcd..efc8b1394250 100644 --- a/clippy_lints/src/matches.rs +++ b/clippy_lints/src/matches.rs @@ -3,19 +3,19 @@ use crate::utils::sugg::Sugg; use crate::utils::visitors::LocalUsedVisitor; use crate::utils::{ expr_block, get_parent_expr, implements_trait, in_macro, indent_of, is_allowed, is_expn_of, is_refutable, - is_type_diagnostic_item, is_wild, match_qpath, match_type, meets_msrv, multispan_sugg, path_to_local_id, - peel_hir_pat_refs, peel_mid_ty_refs, peel_n_hir_expr_refs, remove_blocks, snippet, snippet_block, snippet_opt, - snippet_with_applicability, span_lint_and_help, span_lint_and_note, span_lint_and_sugg, span_lint_and_then, - strip_pat_refs, + is_type_diagnostic_item, is_wild, match_qpath, match_type, meets_msrv, multispan_sugg, path_to_local, + path_to_local_id, peel_hir_pat_refs, peel_mid_ty_refs, peel_n_hir_expr_refs, remove_blocks, snippet, snippet_block, + snippet_opt, snippet_with_applicability, span_lint_and_help, span_lint_and_note, span_lint_and_sugg, + span_lint_and_then, strip_pat_refs, }; use crate::utils::{paths, search_same, SpanlessEq, SpanlessHash}; use if_chain::if_chain; use rustc_ast::ast::LitKind; -use rustc_data_structures::fx::FxHashMap; +use rustc_data_structures::fx::{FxHashMap, FxHashSet}; use rustc_errors::Applicability; use rustc_hir::def::CtorKind; use rustc_hir::{ - Arm, BindingAnnotation, Block, BorrowKind, Expr, ExprKind, Guard, Local, MatchSource, Mutability, Node, Pat, + Arm, BindingAnnotation, Block, BorrowKind, Expr, ExprKind, Guard, HirId, Local, MatchSource, Mutability, Node, Pat, PatKind, QPath, RangeEnd, }; use rustc_lint::{LateContext, LateLintPass, LintContext}; @@ -24,7 +24,7 @@ use rustc_middle::ty::{self, Ty, TyS}; use rustc_semver::RustcVersion; use rustc_session::{declare_tool_lint, impl_lint_pass}; use rustc_span::source_map::{Span, Spanned}; -use rustc_span::{sym, Symbol}; +use rustc_span::sym; use std::cmp::Ordering; use std::collections::hash_map::Entry; use std::collections::Bound; @@ -1873,13 +1873,6 @@ fn test_overlapping() { /// Implementation of `MATCH_SAME_ARMS`. fn lint_match_arms<'tcx>(cx: &LateContext<'tcx>, expr: &Expr<'_>) { - fn same_bindings<'tcx>(lhs: &FxHashMap>, rhs: &FxHashMap>) -> bool { - lhs.len() == rhs.len() - && lhs - .iter() - .all(|(name, l_ty)| rhs.get(name).map_or(false, |r_ty| TyS::same_type(l_ty, r_ty))) - } - if let ExprKind::Match(_, ref arms, MatchSource::Normal) = expr.kind { let hash = |&(_, arm): &(usize, &Arm<'_>)| -> u64 { let mut h = SpanlessHash::new(cx); @@ -1891,12 +1884,38 @@ fn lint_match_arms<'tcx>(cx: &LateContext<'tcx>, expr: &Expr<'_>) { let min_index = usize::min(lindex, rindex); let max_index = usize::max(lindex, rindex); + let mut local_map: FxHashMap = FxHashMap::default(); + let eq_fallback = |a: &Expr<'_>, b: &Expr<'_>| { + if_chain! { + if let Some(a_id) = path_to_local(a); + if let Some(b_id) = path_to_local(b); + let entry = match local_map.entry(a_id) { + Entry::Vacant(entry) => entry, + // check if using the same bindings as before + Entry::Occupied(entry) => return *entry.get() == b_id, + }; + // the names technically don't have to match; this makes the lint more conservative + if cx.tcx.hir().name(a_id) == cx.tcx.hir().name(b_id); + if TyS::same_type(cx.typeck_results().expr_ty(a), cx.typeck_results().expr_ty(b)); + if pat_contains_local(lhs.pat, a_id); + if pat_contains_local(rhs.pat, b_id); + then { + entry.insert(b_id); + true + } else { + false + } + } + }; // Arms with a guard are ignored, those can’t always be merged together // This is also the case for arms in-between each there is an arm with a guard - (min_index..=max_index).all(|index| arms[index].guard.is_none()) && - SpanlessEq::new(cx).eq_expr(&lhs.body, &rhs.body) && - // all patterns should have the same bindings - same_bindings(&bindings(cx, &lhs.pat), &bindings(cx, &rhs.pat)) + (min_index..=max_index).all(|index| arms[index].guard.is_none()) + && SpanlessEq::new(cx) + .expr_fallback(eq_fallback) + .eq_expr(&lhs.body, &rhs.body) + // these checks could be removed to allow unused bindings + && bindings_eq(lhs.pat, local_map.keys().copied().collect()) + && bindings_eq(rhs.pat, local_map.values().copied().collect()) }; let indexed_arms: Vec<(usize, &Arm<'_>)> = arms.iter().enumerate().collect(); @@ -1939,50 +1958,18 @@ fn lint_match_arms<'tcx>(cx: &LateContext<'tcx>, expr: &Expr<'_>) { } } -/// Returns the list of bindings in a pattern. -fn bindings<'tcx>(cx: &LateContext<'tcx>, pat: &Pat<'_>) -> FxHashMap> { - fn bindings_impl<'tcx>(cx: &LateContext<'tcx>, pat: &Pat<'_>, map: &mut FxHashMap>) { - match pat.kind { - PatKind::Box(ref pat) | PatKind::Ref(ref pat, _) => bindings_impl(cx, pat, map), - PatKind::TupleStruct(_, pats, _) => { - for pat in pats { - bindings_impl(cx, pat, map); - } - }, - PatKind::Binding(.., ident, ref as_pat) => { - if let Entry::Vacant(v) = map.entry(ident.name) { - v.insert(cx.typeck_results().pat_ty(pat)); - } - if let Some(ref as_pat) = *as_pat { - bindings_impl(cx, as_pat, map); - } - }, - PatKind::Or(fields) | PatKind::Tuple(fields, _) => { - for pat in fields { - bindings_impl(cx, pat, map); - } - }, - PatKind::Struct(_, fields, _) => { - for pat in fields { - bindings_impl(cx, &pat.pat, map); - } - }, - PatKind::Slice(lhs, ref mid, rhs) => { - for pat in lhs { - bindings_impl(cx, pat, map); - } - if let Some(ref mid) = *mid { - bindings_impl(cx, mid, map); - } - for pat in rhs { - bindings_impl(cx, pat, map); - } - }, - PatKind::Lit(..) | PatKind::Range(..) | PatKind::Wild | PatKind::Path(..) => (), - } - } - - let mut result = FxHashMap::default(); - bindings_impl(cx, pat, &mut result); +fn pat_contains_local(pat: &Pat<'_>, id: HirId) -> bool { + let mut result = false; + pat.walk_short(|p| { + result |= matches!(p.kind, PatKind::Binding(_, binding_id, ..) if binding_id == id); + !result + }); result } + +/// Returns true if all the bindings in the `Pat` are in `ids` and vice versa +fn bindings_eq(pat: &Pat<'_>, mut ids: FxHashSet) -> bool { + let mut result = true; + pat.each_binding_or_first(&mut |_, id, _, _| result &= ids.remove(&id)); + result && ids.is_empty() +}