From 56d591e12e0cca855b47fa0b046088ec5857a6ad Mon Sep 17 00:00:00 2001 From: Philipp Hansch Date: Sat, 25 Apr 2020 21:40:58 +0200 Subject: [PATCH] Cleanup: Use rustc's `same_types` for our `same_tys` This delegates our `same_tys` to [ty::TyS::same_type][same_type] to remove some duplication. Our `same_tys` was introduced 4 years ago in #730. Before, `same_tys` was building an inference context to be able to call `can_eq` to compare the types. The `rustc-dev-guide` has some more details about `can_eq` in [Type Inference -> Trying equality][try_eq]. Now, using the rustc function, we are essentially comparing the `DefId`s of the given types, which also makes more sense, IMO. I also confirmed that the FIXME is resolved via a bit of `dbg!`, however no UI tests were affected. [same_type]: https://github.com/rust-lang/rust/blob/659951c4a0d7450e43f61c61c0e87d0ceae17087/src/librustc_middle/ty/util.rs#L777 [try_eq]: https://rustc-dev-guide.rust-lang.org/type-inference.html#trying-equality --- clippy_lints/src/copies.rs | 5 ++--- clippy_lints/src/identity_conversion.rs | 6 +++--- clippy_lints/src/loops.rs | 4 ++-- clippy_lints/src/methods/mod.rs | 4 ++-- clippy_lints/src/new_without_default.rs | 2 +- clippy_lints/src/types.rs | 2 +- clippy_lints/src/utils/mod.rs | 15 +++------------ 7 files changed, 14 insertions(+), 24 deletions(-) diff --git a/clippy_lints/src/copies.rs b/clippy_lints/src/copies.rs index 66722786eab4..b8ab43522d77 100644 --- a/clippy_lints/src/copies.rs +++ b/clippy_lints/src/copies.rs @@ -243,14 +243,13 @@ fn lint_same_fns_in_if_cond(cx: &LateContext<'_, '_>, conds: &[&Expr<'_>]) { /// Implementation of `MATCH_SAME_ARMS`. fn lint_match_arms<'tcx>(cx: &LateContext<'_, 'tcx>, expr: &Expr<'_>) { fn same_bindings<'tcx>( - cx: &LateContext<'_, 'tcx>, lhs: &FxHashMap>, rhs: &FxHashMap>, ) -> bool { lhs.len() == rhs.len() && lhs .iter() - .all(|(name, l_ty)| rhs.get(name).map_or(false, |r_ty| same_tys(cx, l_ty, r_ty))) + .all(|(name, l_ty)| rhs.get(name).map_or(false, |r_ty| same_tys(l_ty, r_ty))) } if let ExprKind::Match(_, ref arms, MatchSource::Normal) = expr.kind { @@ -269,7 +268,7 @@ fn lint_match_arms<'tcx>(cx: &LateContext<'_, 'tcx>, expr: &Expr<'_>) { (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(cx, &bindings(cx, &lhs.pat), &bindings(cx, &rhs.pat)) + same_bindings(&bindings(cx, &lhs.pat), &bindings(cx, &rhs.pat)) }; let indexed_arms: Vec<(usize, &Arm<'_>)> = arms.iter().enumerate().collect(); diff --git a/clippy_lints/src/identity_conversion.rs b/clippy_lints/src/identity_conversion.rs index 33a9478f0588..f8eacfeb4817 100644 --- a/clippy_lints/src/identity_conversion.rs +++ b/clippy_lints/src/identity_conversion.rs @@ -55,7 +55,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for IdentityConversion { if match_trait_method(cx, e, &paths::INTO) && &*name.ident.as_str() == "into" { let a = cx.tables.expr_ty(e); let b = cx.tables.expr_ty(&args[0]); - if same_tys(cx, a, b) { + if same_tys(a, b) { let sugg = snippet_with_macro_callsite(cx, args[0].span, "").to_string(); span_lint_and_sugg( @@ -72,7 +72,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for IdentityConversion { if match_trait_method(cx, e, &paths::INTO_ITERATOR) && &*name.ident.as_str() == "into_iter" { let a = cx.tables.expr_ty(e); let b = cx.tables.expr_ty(&args[0]); - if same_tys(cx, a, b) { + if same_tys(a, b) { let sugg = snippet(cx, args[0].span, "").into_owned(); span_lint_and_sugg( cx, @@ -93,7 +93,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for IdentityConversion { if match_def_path(cx, def_id, &paths::FROM_FROM) { let a = cx.tables.expr_ty(e); let b = cx.tables.expr_ty(&args[0]); - if same_tys(cx, a, b) { + if same_tys(a, b) { let sugg = snippet(cx, args[0].span.source_callsite(), "").into_owned(); let sugg_msg = format!("consider removing `{}()`", snippet(cx, path.span, "From::from")); diff --git a/clippy_lints/src/loops.rs b/clippy_lints/src/loops.rs index 56dd2795c609..b12834346ffa 100644 --- a/clippy_lints/src/loops.rs +++ b/clippy_lints/src/loops.rs @@ -1348,7 +1348,7 @@ fn check_for_loop_arg(cx: &LateContext<'_, '_>, pat: &Pat<'_>, arg: &Expr<'_>, e } else if method_name == "into_iter" && match_trait_method(cx, arg, &paths::INTO_ITERATOR) { let receiver_ty = cx.tables.expr_ty(&args[0]); let receiver_ty_adjusted = cx.tables.expr_ty_adjusted(&args[0]); - if same_tys(cx, receiver_ty, receiver_ty_adjusted) { + if same_tys(receiver_ty, receiver_ty_adjusted) { let mut applicability = Applicability::MachineApplicable; let object = snippet_with_applicability(cx, args[0].span, "_", &mut applicability); span_lint_and_sugg( @@ -1369,7 +1369,7 @@ fn check_for_loop_arg(cx: &LateContext<'_, '_>, pat: &Pat<'_>, arg: &Expr<'_>, e mutbl: Mutability::Not, }, ); - if same_tys(cx, receiver_ty_adjusted, ref_receiver_ty) { + if same_tys(receiver_ty_adjusted, ref_receiver_ty) { lint_iter_method(cx, args, arg, method_name) } } diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index 7c652229d33d..6f23aef0be24 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -1524,7 +1524,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Methods { let contains_self_ty = |ty: Ty<'tcx>| { ty.walk().any(|inner| match inner.unpack() { - GenericArgKind::Type(inner_ty) => same_tys(cx, self_ty, inner_ty), + GenericArgKind::Type(inner_ty) => same_tys(self_ty, inner_ty), GenericArgKind::Lifetime(_) | GenericArgKind::Const(_) => false, }) @@ -1554,7 +1554,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Methods { } } - if name == "new" && !same_tys(cx, ret_ty, self_ty) { + if name == "new" && !same_tys(ret_ty, self_ty) { span_lint( cx, NEW_RET_NO_SELF, diff --git a/clippy_lints/src/new_without_default.rs b/clippy_lints/src/new_without_default.rs index a599667b8d8a..0b181002fcf3 100644 --- a/clippy_lints/src/new_without_default.rs +++ b/clippy_lints/src/new_without_default.rs @@ -129,7 +129,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for NewWithoutDefault { let self_did = cx.tcx.hir().local_def_id(cx.tcx.hir().get_parent_item(id)); let self_ty = cx.tcx.type_of(self_did); if_chain! { - if same_tys(cx, self_ty, return_ty(cx, id)); + if same_tys(self_ty, return_ty(cx, id)); if let Some(default_trait_id) = get_trait_def_id(cx, &paths::DEFAULT_TRAIT); then { if self.impling_types.is_none() { diff --git a/clippy_lints/src/types.rs b/clippy_lints/src/types.rs index 4d853b99bafa..e884ab3c33df 100644 --- a/clippy_lints/src/types.rs +++ b/clippy_lints/src/types.rs @@ -2462,7 +2462,7 @@ impl<'a, 'b, 'tcx> Visitor<'tcx> for ImplicitHasherConstructorVisitor<'a, 'b, 't if let ExprKind::Path(QPath::TypeRelative(ref ty, ref method)) = fun.kind; if let TyKind::Path(QPath::Resolved(None, ref ty_path)) = ty.kind; then { - if !same_tys(self.cx, self.target.ty(), self.body.expr_ty(e)) { + if !same_tys(self.target.ty(), self.body.expr_ty(e)) { return; } diff --git a/clippy_lints/src/utils/mod.rs b/clippy_lints/src/utils/mod.rs index a0543a8dcf98..f9b3587c0bef 100644 --- a/clippy_lints/src/utils/mod.rs +++ b/clippy_lints/src/utils/mod.rs @@ -41,7 +41,7 @@ use rustc_infer::infer::TyCtxtInferExt; use rustc_lint::{LateContext, Level, Lint, LintContext}; use rustc_middle::hir::map::Map; use rustc_middle::traits; -use rustc_middle::ty::{self, layout::IntegerExt, subst::GenericArg, Binder, Ty, TyCtxt, TypeFoldable}; +use rustc_middle::ty::{self, layout::IntegerExt, subst::GenericArg, Ty, TyCtxt, TypeFoldable}; use rustc_span::hygiene::{ExpnKind, MacroKind}; use rustc_span::source_map::original_sp; use rustc_span::symbol::{self, kw, Symbol}; @@ -890,17 +890,8 @@ pub fn return_ty<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, fn_item: hir::HirId) -> T } /// Checks if two types are the same. -/// -/// This discards any lifetime annotations, too. -// -// FIXME: this works correctly for lifetimes bounds (`for <'a> Foo<'a>` == -// `for <'b> Foo<'b>`, but not for type parameters). -pub fn same_tys<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, a: Ty<'tcx>, b: Ty<'tcx>) -> bool { - let a = cx.tcx.erase_late_bound_regions(&Binder::bind(a)); - let b = cx.tcx.erase_late_bound_regions(&Binder::bind(b)); - cx.tcx - .infer_ctxt() - .enter(|infcx| infcx.can_eq(cx.param_env, a, b).is_ok()) +pub fn same_tys<'tcx>(a: Ty<'tcx>, b: Ty<'tcx>) -> bool { + ty::TyS::same_type(a, b) } /// Returns `true` if the given type is an `unsafe` function.