Auto merge of #8673 - Jarcho:same_functions_8139, r=Manishearth

Fix `same_functions_in_if_condition` FP

fixes #8139

changelog: Don't consider `Foo<{ SomeConstant }>` and `Foo<{ SomeOtherConstant }>` to be the same, even if the constants have the same value.

Author: bors

clippy_utils/src/hir_utils.rs

@@ -1,4 +1,4 @@
-use crate::consts::{constant_context, constant_simple};
+use crate::consts::constant_simple;
 use crate::source::snippet_opt;
 use rustc_ast::ast::InlineAsmTemplatePiece;
 use rustc_data_structures::fx::FxHasher;
@@ -16,15 +16,14 @@ use rustc_span::Symbol;
 use std::hash::{Hash, Hasher};
 
 /// Type used to check whether two ast are the same. This is different from the
-/// operator
-/// `==` on ast types as this operator would compare true equality with ID and
-/// span.
+/// operator `==` on ast types as this operator would compare true equality with
+/// ID and span.
 ///
 /// Note that some expressions kinds are not considered but could be added.
 pub struct SpanlessEq<'a, 'tcx> {
     /// Context used to evaluate constant expressions.
     cx: &'a LateContext<'tcx>,
-    maybe_typeck_results: Option<&'tcx TypeckResults<'tcx>>,
+    maybe_typeck_results: Option<(&'tcx TypeckResults<'tcx>, &'tcx TypeckResults<'tcx>)>,
     allow_side_effects: bool,
     expr_fallback: Option<Box<dyn FnMut(&Expr<'_>, &Expr<'_>) -> bool + 'a>>,
 }
@@ -33,7 +32,7 @@ impl<'a, 'tcx> SpanlessEq<'a, 'tcx> {
     pub fn new(cx: &'a LateContext<'tcx>) -> Self {
         Self {
             cx,
-            maybe_typeck_results: cx.maybe_typeck_results(),
+            maybe_typeck_results: cx.maybe_typeck_results().map(|x| (x, x)),
             allow_side_effects: true,
             expr_fallback: None,
         }
@@ -102,9 +101,9 @@ impl HirEqInterExpr<'_, '_, '_> {
             (&StmtKind::Local(l), &StmtKind::Local(r)) => {
                 // This additional check ensures that the type of the locals are equivalent even if the init
                 // expression or type have some inferred parts.
-                if let Some(typeck) = self.inner.maybe_typeck_results {
-                    let l_ty = typeck.pat_ty(l.pat);
-                    let r_ty = typeck.pat_ty(r.pat);
+                if let Some((typeck_lhs, typeck_rhs)) = self.inner.maybe_typeck_results {
+                    let l_ty = typeck_lhs.pat_ty(l.pat);
+                    let r_ty = typeck_rhs.pat_ty(r.pat);
                     if l_ty != r_ty {
                         return false;
                     }
@@ -182,9 +181,17 @@ 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)
+        // swap out TypeckResults when hashing a body
+        let old_maybe_typeck_results = self.inner.maybe_typeck_results.replace((
+            self.inner.cx.tcx.typeck_body(left),
+            self.inner.cx.tcx.typeck_body(right),
+        ));
+        let res = self.eq_expr(
+            &self.inner.cx.tcx.hir().body(left).value,
+            &self.inner.cx.tcx.hir().body(right).value,
+        );
+        self.inner.maybe_typeck_results = old_maybe_typeck_results;
+        res
     }
 
     #[allow(clippy::similar_names)]
@@ -193,10 +200,10 @@ impl HirEqInterExpr<'_, '_, '_> {
             return false;
         }
 
-        if let Some(typeck_results) = self.inner.maybe_typeck_results {
+        if let Some((typeck_lhs, typeck_rhs)) = self.inner.maybe_typeck_results {
             if let (Some(l), Some(r)) = (
-                constant_simple(self.inner.cx, typeck_results, left),
-                constant_simple(self.inner.cx, typeck_results, right),
+                constant_simple(self.inner.cx, typeck_lhs, left),
+                constant_simple(self.inner.cx, typeck_rhs, right),
             ) {
                 if l == r {
                     return true;

tests/ui/same_functions_in_if_condition.rs

@@ -1,3 +1,5 @@
+#![feature(adt_const_params)]
+#![allow(incomplete_features)]
 #![warn(clippy::same_functions_in_if_condition)]
 #![allow(clippy::ifs_same_cond)] // This warning is different from `ifs_same_cond`.
 #![allow(clippy::if_same_then_else, clippy::comparison_chain)] // all empty blocks
@@ -87,4 +89,21 @@ fn main() {
         "linux"
     };
     println!("{}", os);
+
+    #[derive(PartialEq, Eq)]
+    enum E {
+        A,
+        B,
+    }
+    fn generic<const P: E>() -> bool {
+        match P {
+            E::A => true,
+            E::B => false,
+        }
+    }
+    if generic::<{ E::A }>() {
+        println!("A");
+    } else if generic::<{ E::B }>() {
+        println!("B");
+    }
 }

tests/ui/same_functions_in_if_condition.stderr

@@ -1,72 +1,72 @@
 error: this `if` has the same function call as a previous `if`
-  --> $DIR/same_functions_in_if_condition.rs:29:15
+  --> $DIR/same_functions_in_if_condition.rs:31:15
    |
 LL |     } else if function() {
    |               ^^^^^^^^^^
    |
    = note: `-D clippy::same-functions-in-if-condition` implied by `-D warnings`
 note: same as this
-  --> $DIR/same_functions_in_if_condition.rs:28:8
+  --> $DIR/same_functions_in_if_condition.rs:30:8
    |
 LL |     if function() {
    |        ^^^^^^^^^^
 
 error: this `if` has the same function call as a previous `if`
-  --> $DIR/same_functions_in_if_condition.rs:34:15
+  --> $DIR/same_functions_in_if_condition.rs:36:15
    |
 LL |     } else if fn_arg(a) {
    |               ^^^^^^^^^
    |
 note: same as this
-  --> $DIR/same_functions_in_if_condition.rs:33:8
+  --> $DIR/same_functions_in_if_condition.rs:35:8
    |
 LL |     if fn_arg(a) {
    |        ^^^^^^^^^
 
 error: this `if` has the same function call as a previous `if`
-  --> $DIR/same_functions_in_if_condition.rs:39:15
+  --> $DIR/same_functions_in_if_condition.rs:41:15
    |
 LL |     } else if obj.method() {
    |               ^^^^^^^^^^^^
    |
 note: same as this
-  --> $DIR/same_functions_in_if_condition.rs:38:8
+  --> $DIR/same_functions_in_if_condition.rs:40:8
    |
 LL |     if obj.method() {
    |        ^^^^^^^^^^^^
 
 error: this `if` has the same function call as a previous `if`
-  --> $DIR/same_functions_in_if_condition.rs:44:15
+  --> $DIR/same_functions_in_if_condition.rs:46:15
    |
 LL |     } else if obj.method_arg(a) {
    |               ^^^^^^^^^^^^^^^^^
    |
 note: same as this
-  --> $DIR/same_functions_in_if_condition.rs:43:8
+  --> $DIR/same_functions_in_if_condition.rs:45:8
    |
 LL |     if obj.method_arg(a) {
    |        ^^^^^^^^^^^^^^^^^
 
 error: this `if` has the same function call as a previous `if`
-  --> $DIR/same_functions_in_if_condition.rs:51:15
+  --> $DIR/same_functions_in_if_condition.rs:53:15
    |
 LL |     } else if v.pop() == None {
    |               ^^^^^^^^^^^^^^^
    |
 note: same as this
-  --> $DIR/same_functions_in_if_condition.rs:49:8
+  --> $DIR/same_functions_in_if_condition.rs:51:8
    |
 LL |     if v.pop() == None {
    |        ^^^^^^^^^^^^^^^
 
 error: this `if` has the same function call as a previous `if`
-  --> $DIR/same_functions_in_if_condition.rs:56:15
+  --> $DIR/same_functions_in_if_condition.rs:58:15
    |
 LL |     } else if v.len() == 42 {
    |               ^^^^^^^^^^^^^
    |
 note: same as this
-  --> $DIR/same_functions_in_if_condition.rs:54:8
+  --> $DIR/same_functions_in_if_condition.rs:56:8
    |
 LL |     if v.len() == 42 {
    |        ^^^^^^^^^^^^^