Skip to content

Commit

Permalink
Cleanup: Use rustc's same_types for our same_tys
Browse files Browse the repository at this point in the history
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
  • Loading branch information
phansch committed Apr 25, 2020
1 parent 6ffe725 commit 56d591e
Show file tree
Hide file tree
Showing 7 changed files with 14 additions and 24 deletions.
5 changes: 2 additions & 3 deletions clippy_lints/src/copies.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<Symbol, Ty<'tcx>>,
rhs: &FxHashMap<Symbol, Ty<'tcx>>,
) -> 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 {
Expand All @@ -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();
Expand Down
6 changes: 3 additions & 3 deletions clippy_lints/src/identity_conversion.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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, "<expr>").to_string();

span_lint_and_sugg(
Expand All @@ -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, "<expr>").into_owned();
span_lint_and_sugg(
cx,
Expand All @@ -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(), "<expr>").into_owned();
let sugg_msg =
format!("consider removing `{}()`", snippet(cx, path.span, "From::from"));
Expand Down
4 changes: 2 additions & 2 deletions clippy_lints/src/loops.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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)
}
}
Expand Down
4 changes: 2 additions & 2 deletions clippy_lints/src/methods/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
})
Expand Down Expand Up @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/new_without_default.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
15 changes: 3 additions & 12 deletions clippy_lints/src/utils/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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.
Expand Down

0 comments on commit 56d591e

Please sign in to comment.