Skip to content

Commit 131ff87

Browse files
borsbors
committed
Auto merge of rust-lang#8673 - Jarcho:same_functions_8139, r=Manishearth
Fix `same_functions_in_if_condition` FP fixes rust-lang#8139 changelog: Don't consider `Foo<{ SomeConstant }>` and `Foo<{ SomeOtherConstant }>` to be the same, even if the constants have the same value.
2 parents 18ab97d + 719a040 commit 131ff87

File tree

3 files changed

+53
-27
lines changed

3 files changed

+53
-27
lines changed

‎clippy_utils/src/hir_utils.rs‎

Lines changed: 22 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
use crate::consts::{constant_context, constant_simple};
1+
use crate::consts::constant_simple;
22
use crate::source::snippet_opt;
33
use rustc_ast::ast::InlineAsmTemplatePiece;
44
use rustc_data_structures::fx::FxHasher;
@@ -16,15 +16,14 @@ use rustc_span::Symbol;
1616
use std::hash::{Hash, Hasher};
1717

1818
/// Type used to check whether two ast are the same. This is different from the
19-
/// operator
20-
/// `==` on ast types as this operator would compare true equality with ID and
21-
/// span.
19+
/// operator `==` on ast types as this operator would compare true equality with
20+
/// ID and span.
2221
///
2322
/// Note that some expressions kinds are not considered but could be added.
2423
pub struct SpanlessEq<'a, 'tcx> {
2524
/// Context used to evaluate constant expressions.
2625
cx: &'a LateContext<'tcx>,
27-
maybe_typeck_results: Option<&'tcx TypeckResults<'tcx>>,
26+
maybe_typeck_results: Option<(&'tcx TypeckResults<'tcx>, &'tcx TypeckResults<'tcx>)>,
2827
allow_side_effects: bool,
2928
expr_fallback: Option<Box<dyn FnMut(&Expr<'_>, &Expr<'_>) -> bool + 'a>>,
3029
}
@@ -33,7 +32,7 @@ impl<'a, 'tcx> SpanlessEq<'a, 'tcx> {
3332
pub fn new(cx: &'a LateContext<'tcx>) -> Self {
3433
Self {
3534
cx,
36-
maybe_typeck_results: cx.maybe_typeck_results(),
35+
maybe_typeck_results: cx.maybe_typeck_results().map(|x| (x, x)),
3736
allow_side_effects: true,
3837
expr_fallback: None,
3938
}
@@ -102,9 +101,9 @@ impl HirEqInterExpr<'_, '_, '_> {
102101
(&StmtKind::Local(l), &StmtKind::Local(r)) => {
103102
// This additional check ensures that the type of the locals are equivalent even if the init
104103
// expression or type have some inferred parts.
105-
if let Some(typeck) = self.inner.maybe_typeck_results {
106-
let l_ty = typeck.pat_ty(l.pat);
107-
let r_ty = typeck.pat_ty(r.pat);
104+
if let Some((typeck_lhs, typeck_rhs)) = self.inner.maybe_typeck_results {
105+
let l_ty = typeck_lhs.pat_ty(l.pat);
106+
let r_ty = typeck_rhs.pat_ty(r.pat);
108107
if l_ty != r_ty {
109108
return false;
110109
}
@@ -182,9 +181,17 @@ impl HirEqInterExpr<'_, '_, '_> {
182181
}
183182

184183
pub fn eq_body(&mut self, left: BodyId, right: BodyId) -> bool {
185-
let cx = self.inner.cx;
186-
let eval_const = |body| constant_context(cx, cx.tcx.typeck_body(body)).expr(&cx.tcx.hir().body(body).value);
187-
eval_const(left) == eval_const(right)
184+
// swap out TypeckResults when hashing a body
185+
let old_maybe_typeck_results = self.inner.maybe_typeck_results.replace((
186+
self.inner.cx.tcx.typeck_body(left),
187+
self.inner.cx.tcx.typeck_body(right),
188+
));
189+
let res = self.eq_expr(
190+
&self.inner.cx.tcx.hir().body(left).value,
191+
&self.inner.cx.tcx.hir().body(right).value,
192+
);
193+
self.inner.maybe_typeck_results = old_maybe_typeck_results;
194+
res
188195
}
189196

190197
#[allow(clippy::similar_names)]
@@ -193,10 +200,10 @@ impl HirEqInterExpr<'_, '_, '_> {
193200
return false;
194201
}
195202

196-
if let Some(typeck_results) = self.inner.maybe_typeck_results {
203+
if let Some((typeck_lhs, typeck_rhs)) = self.inner.maybe_typeck_results {
197204
if let (Some(l), Some(r)) = (
198-
constant_simple(self.inner.cx, typeck_results, left),
199-
constant_simple(self.inner.cx, typeck_results, right),
205+
constant_simple(self.inner.cx, typeck_lhs, left),
206+
constant_simple(self.inner.cx, typeck_rhs, right),
200207
) {
201208
if l == r {
202209
return true;

‎tests/ui/same_functions_in_if_condition.rs‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
#![feature(adt_const_params)]
2+
#![allow(incomplete_features)]
13
#![warn(clippy::same_functions_in_if_condition)]
24
#![allow(clippy::ifs_same_cond)] // This warning is different from `ifs_same_cond`.
35
#![allow(clippy::if_same_then_else, clippy::comparison_chain)] // all empty blocks
@@ -87,4 +89,21 @@ fn main() {
8789
"linux"
8890
};
8991
println!("{}", os);
92+
93+
#[derive(PartialEq, Eq)]
94+
enum E {
95+
A,
96+
B,
97+
}
98+
fn generic<const P: E>() -> bool {
99+
match P {
100+
E::A => true,
101+
E::B => false,
102+
}
103+
}
104+
if generic::<{ E::A }>() {
105+
println!("A");
106+
} else if generic::<{ E::B }>() {
107+
println!("B");
108+
}
90109
}

‎tests/ui/same_functions_in_if_condition.stderr‎

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,72 +1,72 @@
11
error: this `if` has the same function call as a previous `if`
2-
--> $DIR/same_functions_in_if_condition.rs:29:15
2+
--> $DIR/same_functions_in_if_condition.rs:31:15
33
|
44
LL | } else if function() {
55
| ^^^^^^^^^^
66
|
77
= note: `-D clippy::same-functions-in-if-condition` implied by `-D warnings`
88
note: same as this
9-
--> $DIR/same_functions_in_if_condition.rs:28:8
9+
--> $DIR/same_functions_in_if_condition.rs:30:8
1010
|
1111
LL | if function() {
1212
| ^^^^^^^^^^
1313

1414
error: this `if` has the same function call as a previous `if`
15-
--> $DIR/same_functions_in_if_condition.rs:34:15
15+
--> $DIR/same_functions_in_if_condition.rs:36:15
1616
|
1717
LL | } else if fn_arg(a) {
1818
| ^^^^^^^^^
1919
|
2020
note: same as this
21-
--> $DIR/same_functions_in_if_condition.rs:33:8
21+
--> $DIR/same_functions_in_if_condition.rs:35:8
2222
|
2323
LL | if fn_arg(a) {
2424
| ^^^^^^^^^
2525

2626
error: this `if` has the same function call as a previous `if`
27-
--> $DIR/same_functions_in_if_condition.rs:39:15
27+
--> $DIR/same_functions_in_if_condition.rs:41:15
2828
|
2929
LL | } else if obj.method() {
3030
| ^^^^^^^^^^^^
3131
|
3232
note: same as this
33-
--> $DIR/same_functions_in_if_condition.rs:38:8
33+
--> $DIR/same_functions_in_if_condition.rs:40:8
3434
|
3535
LL | if obj.method() {
3636
| ^^^^^^^^^^^^
3737

3838
error: this `if` has the same function call as a previous `if`
39-
--> $DIR/same_functions_in_if_condition.rs:44:15
39+
--> $DIR/same_functions_in_if_condition.rs:46:15
4040
|
4141
LL | } else if obj.method_arg(a) {
4242
| ^^^^^^^^^^^^^^^^^
4343
|
4444
note: same as this
45-
--> $DIR/same_functions_in_if_condition.rs:43:8
45+
--> $DIR/same_functions_in_if_condition.rs:45:8
4646
|
4747
LL | if obj.method_arg(a) {
4848
| ^^^^^^^^^^^^^^^^^
4949

5050
error: this `if` has the same function call as a previous `if`
51-
--> $DIR/same_functions_in_if_condition.rs:51:15
51+
--> $DIR/same_functions_in_if_condition.rs:53:15
5252
|
5353
LL | } else if v.pop() == None {
5454
| ^^^^^^^^^^^^^^^
5555
|
5656
note: same as this
57-
--> $DIR/same_functions_in_if_condition.rs:49:8
57+
--> $DIR/same_functions_in_if_condition.rs:51:8
5858
|
5959
LL | if v.pop() == None {
6060
| ^^^^^^^^^^^^^^^
6161

6262
error: this `if` has the same function call as a previous `if`
63-
--> $DIR/same_functions_in_if_condition.rs:56:15
63+
--> $DIR/same_functions_in_if_condition.rs:58:15
6464
|
6565
LL | } else if v.len() == 42 {
6666
| ^^^^^^^^^^^^^
6767
|
6868
note: same as this
69-
--> $DIR/same_functions_in_if_condition.rs:54:8
69+
--> $DIR/same_functions_in_if_condition.rs:56:8
7070
|
7171
LL | if v.len() == 42 {
7272
| ^^^^^^^^^^^^^

0 commit comments

Comments
 (0)