From e027b6bc49f7b0719b65aa2e366bc39c96ef698b Mon Sep 17 00:00:00 2001 From: Cameron Steffen Date: Tue, 4 May 2021 10:35:54 -0500 Subject: [PATCH 1/7] Minor SpanlessHash improvements --- clippy_utils/src/hir_utils.rs | 59 +++++++---------------------------- 1 file changed, 12 insertions(+), 47 deletions(-) diff --git a/clippy_utils/src/hir_utils.rs b/clippy_utils/src/hir_utils.rs index 2fc12702f4829..84dceb9a16581 100644 --- a/clippy_utils/src/hir_utils.rs +++ b/clippy_utils/src/hir_utils.rs @@ -6,9 +6,9 @@ use rustc_data_structures::stable_hasher::{HashStable, StableHasher}; use rustc_hir::def::Res; use rustc_hir::HirIdMap; use rustc_hir::{ - BinOpKind, Block, BlockCheckMode, BodyId, BorrowKind, CaptureBy, Expr, ExprField, ExprKind, FnRetTy, GenericArg, - GenericArgs, Guard, HirId, InlineAsmOperand, Lifetime, LifetimeName, ParamName, Pat, PatField, PatKind, Path, - PathSegment, QPath, Stmt, StmtKind, Ty, TyKind, TypeBinding, + BinOpKind, Block, BodyId, Expr, ExprField, ExprKind, FnRetTy, GenericArg, GenericArgs, Guard, HirId, + InlineAsmOperand, Lifetime, LifetimeName, ParamName, Pat, PatField, PatKind, Path, PathSegment, QPath, Stmt, + StmtKind, Ty, TyKind, TypeBinding, }; use rustc_lexer::{tokenize, TokenKind}; use rustc_lint::LateContext; @@ -537,13 +537,7 @@ impl<'a, 'tcx> SpanlessHash<'a, 'tcx> { self.hash_expr(e); } - match b.rules { - BlockCheckMode::DefaultBlock => 0, - BlockCheckMode::UnsafeBlock(_) => 1, - BlockCheckMode::PushUnsafeBlock(_) => 2, - BlockCheckMode::PopUnsafeBlock(_) => 3, - } - .hash(&mut self.s); + std::mem::discriminant(&b.rules).hash(&mut self.s); } #[allow(clippy::many_single_char_names, clippy::too_many_lines)] @@ -554,21 +548,16 @@ impl<'a, 'tcx> SpanlessHash<'a, 'tcx> { // const hashing may result in the same hash as some unrelated node, so add a sort of // discriminant depending on which path we're choosing next - simple_const.is_some().hash(&mut self.s); - - if let Some(e) = simple_const { - return e.hash(&mut self.s); + simple_const.hash(&mut self.s); + if simple_const.is_some() { + return; } std::mem::discriminant(&e.kind).hash(&mut self.s); match e.kind { ExprKind::AddrOf(kind, m, e) => { - match kind { - BorrowKind::Ref => 0, - BorrowKind::Raw => 1, - } - .hash(&mut self.s); + std::mem::discriminant(&kind).hash(&mut self.s); m.hash(&mut self.s); self.hash_expr(e); }, @@ -616,11 +605,7 @@ impl<'a, 'tcx> SpanlessHash<'a, 'tcx> { self.hash_ty(ty); }, ExprKind::Closure(cap, _, eid, _, _) => { - match cap { - CaptureBy::Value => 0, - CaptureBy::Ref => 1, - } - .hash(&mut self.s); + std::mem::discriminant(&cap).hash(&mut self.s); // closures inherit TypeckResults self.hash_expr(&self.cx.tcx.hir().body(eid).value); }, @@ -694,8 +679,6 @@ impl<'a, 'tcx> SpanlessHash<'a, 'tcx> { } }, ExprKind::If(cond, then, ref else_opt) => { - let c: fn(_, _, _) -> _ = ExprKind::If; - c.hash(&mut self.s); self.hash_expr(cond); self.hash_expr(then); if let Some(e) = *else_opt { @@ -928,10 +911,9 @@ impl<'a, 'tcx> SpanlessHash<'a, 'tcx> { for arg in bfn.decl.inputs { self.hash_ty(arg); } + std::mem::discriminant(&bfn.decl.output).hash(&mut self.s); match bfn.decl.output { - FnRetTy::DefaultReturn(_) => { - ().hash(&mut self.s); - }, + FnRetTy::DefaultReturn(_) => {}, FnRetTy::Return(ty) => { self.hash_ty(ty); }, @@ -943,24 +925,7 @@ impl<'a, 'tcx> SpanlessHash<'a, 'tcx> { self.hash_ty(ty); } }, - TyKind::Path(ref qpath) => match qpath { - QPath::Resolved(ref maybe_ty, path) => { - if let Some(ty) = maybe_ty { - self.hash_ty(ty); - } - for segment in path.segments { - segment.ident.name.hash(&mut self.s); - self.hash_generic_args(segment.args().args); - } - }, - QPath::TypeRelative(ty, segment) => { - self.hash_ty(ty); - segment.ident.name.hash(&mut self.s); - }, - QPath::LangItem(lang_item, ..) => { - lang_item.hash(&mut self.s); - }, - }, + TyKind::Path(ref qpath) => self.hash_qpath(qpath), TyKind::OpaqueDef(_, arg_list) => { self.hash_generic_args(arg_list); }, From 27ac6476490eaafdb89a9e5c6096a6e3c87a10b9 Mon Sep 17 00:00:00 2001 From: Cameron Steffen Date: Wed, 12 May 2021 10:15:28 -0500 Subject: [PATCH 2/7] Use discriminant instead of stable_hash --- clippy_utils/src/hir_utils.rs | 21 +++++++++------------ 1 file changed, 9 insertions(+), 12 deletions(-) diff --git a/clippy_utils/src/hir_utils.rs b/clippy_utils/src/hir_utils.rs index 84dceb9a16581..6c0cb22beb184 100644 --- a/clippy_utils/src/hir_utils.rs +++ b/clippy_utils/src/hir_utils.rs @@ -2,7 +2,7 @@ use crate::consts::{constant_context, constant_simple}; use crate::differing_macro_contexts; use crate::source::snippet_opt; use rustc_ast::ast::InlineAsmTemplatePiece; -use rustc_data_structures::stable_hasher::{HashStable, StableHasher}; +use rustc_data_structures::stable_hasher::StableHasher; use rustc_hir::def::Res; use rustc_hir::HirIdMap; use rustc_hir::{ @@ -12,7 +12,6 @@ use rustc_hir::{ }; use rustc_lexer::{tokenize, TokenKind}; use rustc_lint::LateContext; -use rustc_middle::ich::StableHashingContextProvider; use rustc_middle::ty::TypeckResults; use rustc_span::Symbol; use std::hash::Hash; @@ -571,8 +570,7 @@ impl<'a, 'tcx> SpanlessHash<'a, 'tcx> { self.hash_expr(r); }, ExprKind::AssignOp(ref o, l, r) => { - o.node - .hash_stable(&mut self.cx.tcx.get_stable_hashing_context(), &mut self.s); + std::mem::discriminant(&o.node).hash(&mut self.s); self.hash_expr(l); self.hash_expr(r); }, @@ -580,8 +578,7 @@ impl<'a, 'tcx> SpanlessHash<'a, 'tcx> { self.hash_block(b); }, ExprKind::Binary(op, l, r) => { - op.node - .hash_stable(&mut self.cx.tcx.get_stable_hashing_context(), &mut self.s); + std::mem::discriminant(&op.node).hash(&mut self.s); self.hash_expr(l); self.hash_expr(r); }, @@ -736,7 +733,7 @@ impl<'a, 'tcx> SpanlessHash<'a, 'tcx> { self.hash_exprs(v); }, ExprKind::Unary(lop, le) => { - lop.hash_stable(&mut self.cx.tcx.get_stable_hashing_context(), &mut self.s); + std::mem::discriminant(&lop).hash(&mut self.s); self.hash_expr(le); }, } @@ -761,7 +758,7 @@ impl<'a, 'tcx> SpanlessHash<'a, 'tcx> { self.hash_name(path.ident.name); }, QPath::LangItem(lang_item, ..) => { - lang_item.hash_stable(&mut self.cx.tcx.get_stable_hashing_context(), &mut self.s); + std::mem::discriminant(&lang_item).hash(&mut self.s); }, } // self.maybe_typeck_results.unwrap().qpath_res(p, id).hash(&mut self.s); @@ -771,7 +768,7 @@ impl<'a, 'tcx> SpanlessHash<'a, 'tcx> { std::mem::discriminant(&pat.kind).hash(&mut self.s); match pat.kind { PatKind::Binding(ann, _, _, pat) => { - ann.hash_stable(&mut self.cx.tcx.get_stable_hashing_context(), &mut self.s); + std::mem::discriminant(&ann).hash(&mut self.s); if let Some(pat) = pat { self.hash_pat(pat); } @@ -791,11 +788,11 @@ impl<'a, 'tcx> SpanlessHash<'a, 'tcx> { if let Some(e) = e { self.hash_expr(e); } - i.hash_stable(&mut self.cx.tcx.get_stable_hashing_context(), &mut self.s); + std::mem::discriminant(&i).hash(&mut self.s); }, - PatKind::Ref(pat, m) => { + PatKind::Ref(pat, mu) => { self.hash_pat(pat); - m.hash(&mut self.s); + std::mem::discriminant(&mu).hash(&mut self.s); }, PatKind::Slice(l, m, r) => { for pat in l { From f21d909e62dac842cd2415b3a4406dc3b4058ac1 Mon Sep 17 00:00:00 2001 From: Cameron Steffen Date: Wed, 12 May 2021 10:15:51 -0500 Subject: [PATCH 3/7] Use FxHasher --- clippy_utils/src/hir_utils.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/clippy_utils/src/hir_utils.rs b/clippy_utils/src/hir_utils.rs index 6c0cb22beb184..29ad24ee906d5 100644 --- a/clippy_utils/src/hir_utils.rs +++ b/clippy_utils/src/hir_utils.rs @@ -2,7 +2,7 @@ use crate::consts::{constant_context, constant_simple}; use crate::differing_macro_contexts; use crate::source::snippet_opt; use rustc_ast::ast::InlineAsmTemplatePiece; -use rustc_data_structures::stable_hasher::StableHasher; +use rustc_data_structures::fx::FxHasher; use rustc_hir::def::Res; use rustc_hir::HirIdMap; use rustc_hir::{ @@ -14,7 +14,7 @@ use rustc_lexer::{tokenize, TokenKind}; use rustc_lint::LateContext; use rustc_middle::ty::TypeckResults; use rustc_span::Symbol; -use std::hash::Hash; +use std::hash::{Hash, Hasher}; /// Type used to check whether two ast are the same. This is different from the /// operator @@ -511,7 +511,7 @@ pub struct SpanlessHash<'a, 'tcx> { /// Context used to evaluate constant expressions. cx: &'a LateContext<'tcx>, maybe_typeck_results: Option<&'tcx TypeckResults<'tcx>>, - s: StableHasher, + s: FxHasher, } impl<'a, 'tcx> SpanlessHash<'a, 'tcx> { @@ -519,7 +519,7 @@ impl<'a, 'tcx> SpanlessHash<'a, 'tcx> { Self { cx, maybe_typeck_results: cx.maybe_typeck_results(), - s: StableHasher::new(), + s: FxHasher::default(), } } From 24743b39683ef1f69e3156c312ab12d9265ca7bb Mon Sep 17 00:00:00 2001 From: Cameron Steffen Date: Wed, 12 May 2021 10:50:19 -0500 Subject: [PATCH 4/7] Use UnhashMap --- clippy_lints/src/trait_bounds.rs | 3 ++- clippy_utils/src/lib.rs | 6 +++--- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/clippy_lints/src/trait_bounds.rs b/clippy_lints/src/trait_bounds.rs index b0589b0512ef5..74a94db180006 100644 --- a/clippy_lints/src/trait_bounds.rs +++ b/clippy_lints/src/trait_bounds.rs @@ -3,6 +3,7 @@ use clippy_utils::source::{snippet, snippet_with_applicability}; use clippy_utils::{in_macro, SpanlessHash}; use if_chain::if_chain; use rustc_data_structures::fx::FxHashMap; +use rustc_data_structures::unhash::UnhashMap; use rustc_errors::Applicability; use rustc_hir::{def::Res, GenericBound, Generics, ParamName, Path, QPath, TyKind, WherePredicate}; use rustc_lint::{LateContext, LateLintPass}; @@ -100,7 +101,7 @@ impl TraitBounds { hasher.hash_ty(ty); hasher.finish() }; - let mut map = FxHashMap::default(); + let mut map: UnhashMap>> = UnhashMap::default(); let mut applicability = Applicability::MaybeIncorrect; for bound in gen.where_clause.predicates { if_chain! { diff --git a/clippy_utils/src/lib.rs b/clippy_utils/src/lib.rs index 610011af05928..18b7e6cd4bcdc 100644 --- a/clippy_utils/src/lib.rs +++ b/clippy_utils/src/lib.rs @@ -63,7 +63,7 @@ use std::hash::BuildHasherDefault; use if_chain::if_chain; use rustc_ast::ast::{self, Attribute, BorrowKind, LitKind}; -use rustc_data_structures::fx::FxHashMap; +use rustc_data_structures::unhash::UnhashMap; use rustc_hir as hir; use rustc_hir::def::{DefKind, Res}; use rustc_hir::def_id::DefId; @@ -1578,8 +1578,8 @@ where let mut match_expr_list: Vec<(&T, &T)> = Vec::new(); - let mut map: FxHashMap<_, Vec<&_>> = - FxHashMap::with_capacity_and_hasher(exprs.len(), BuildHasherDefault::default()); + let mut map: UnhashMap> = + UnhashMap::with_capacity_and_hasher(exprs.len(), BuildHasherDefault::default()); for expr in exprs { match map.entry(hash(expr)) { From 0ebd5018bf1e0de83f9c1cbe97d00e219d269e85 Mon Sep 17 00:00:00 2001 From: Cameron Steffen Date: Mon, 17 May 2021 15:40:34 -0500 Subject: [PATCH 5/7] Add a short-circuiting case --- clippy_utils/src/lib.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/clippy_utils/src/lib.rs b/clippy_utils/src/lib.rs index 18b7e6cd4bcdc..39fd324408a7f 100644 --- a/clippy_utils/src/lib.rs +++ b/clippy_utils/src/lib.rs @@ -1572,8 +1572,10 @@ where Hash: Fn(&T) -> u64, Eq: Fn(&T, &T) -> bool, { - if exprs.len() == 2 && eq(&exprs[0], &exprs[1]) { - return vec![(&exprs[0], &exprs[1])]; + match exprs { + [a, b] if eq(a, b) => return vec![(a, b)], + _ if exprs.len() <= 2 => return vec![], + _ => {}, } let mut match_expr_list: Vec<(&T, &T)> = Vec::new(); From 9d61b4e081b05228186aeb62a79ab33c28f39f6f Mon Sep 17 00:00:00 2001 From: Cameron Steffen Date: Mon, 24 May 2021 10:46:45 -0500 Subject: [PATCH 6/7] Fix SpanlessEq for GenericArg::Const --- clippy_utils/src/hir_utils.rs | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/clippy_utils/src/hir_utils.rs b/clippy_utils/src/hir_utils.rs index 29ad24ee906d5..6d092680540dc 100644 --- a/clippy_utils/src/hir_utils.rs +++ b/clippy_utils/src/hir_utils.rs @@ -168,6 +168,12 @@ impl HirEqInterExpr<'_, '_, '_> { } } + pub fn eq_body(&mut self, left: BodyId, right: BodyId) -> bool { + let cx = self.inner.cx; + let eval_const = |body| constant_context(cx, cx.tcx.typeck_body(body)).expr(&cx.tcx.hir().body(body).value); + eval_const(left) == eval_const(right) + } + #[allow(clippy::similar_names)] pub fn eq_expr(&mut self, left: &Expr<'_>, right: &Expr<'_>) -> bool { if !self.inner.allow_side_effects && differing_macro_contexts(left.span, right.span) { @@ -243,12 +249,7 @@ impl HirEqInterExpr<'_, '_, '_> { self.inner.allow_side_effects && self.eq_path_segment(l_path, r_path) && self.eq_exprs(l_args, r_args) }, (&ExprKind::Repeat(le, ref ll_id), &ExprKind::Repeat(re, ref rl_id)) => { - 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 + self.eq_expr(le, re) && self.eq_body(ll_id.body, rl_id.body) }, (&ExprKind::Ret(ref l), &ExprKind::Ret(ref r)) => both(l, r, |l, r| self.eq_expr(l, r)), (&ExprKind::Path(ref l), &ExprKind::Path(ref r)) => self.eq_qpath(l, r), @@ -284,6 +285,7 @@ impl HirEqInterExpr<'_, '_, '_> { fn eq_generic_arg(&mut self, left: &GenericArg<'_>, right: &GenericArg<'_>) -> bool { match (left, right) { + (GenericArg::Const(l), GenericArg::Const(r)) => self.eq_body(l.value.body, r.value.body), (GenericArg::Lifetime(l_lt), GenericArg::Lifetime(r_lt)) => Self::eq_lifetime(l_lt, r_lt), (GenericArg::Type(l_ty), GenericArg::Type(r_ty)) => self.eq_ty(l_ty, r_ty), _ => false, @@ -384,10 +386,7 @@ impl HirEqInterExpr<'_, '_, '_> { match (&left.kind, &right.kind) { (&TyKind::Slice(l_vec), &TyKind::Slice(r_vec)) => self.eq_ty(l_vec, r_vec), (&TyKind::Array(lt, ref ll_id), &TyKind::Array(rt, ref rl_id)) => { - 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) + self.eq_ty(lt, rt) && self.eq_body(ll_id.body, 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) @@ -837,6 +836,7 @@ impl<'a, 'tcx> SpanlessHash<'a, 'tcx> { _ => { for seg in path.segments { self.hash_name(seg.ident.name); + self.hash_generic_args(seg.args().args); } }, } From 7f340578cc33e22dfe09927b696a91d0180fe11d Mon Sep 17 00:00:00 2001 From: Cameron Steffen Date: Mon, 24 May 2021 10:51:33 -0500 Subject: [PATCH 7/7] Hash Symbol directly --- clippy_utils/src/hir_utils.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clippy_utils/src/hir_utils.rs b/clippy_utils/src/hir_utils.rs index 6d092680540dc..969193d829486 100644 --- a/clippy_utils/src/hir_utils.rs +++ b/clippy_utils/src/hir_utils.rs @@ -745,7 +745,7 @@ impl<'a, 'tcx> SpanlessHash<'a, 'tcx> { } pub fn hash_name(&mut self, n: Symbol) { - n.as_str().hash(&mut self.s); + n.hash(&mut self.s); } pub fn hash_qpath(&mut self, p: &QPath<'_>) {