When checking if two empty hir blocks are equal also check to see if the tokens used are the same as well

This commit is contained in:
Jason Newcomb 2021-03-04 11:06:24 -05:00
parent ff51964102
commit 39c5e86337
No known key found for this signature in database
GPG key ID: DA59E8643A37ED06
4 changed files with 130 additions and 8 deletions

View file

@ -1,5 +1,5 @@
use crate::consts::{constant_context, constant_simple}; use crate::consts::{constant_context, constant_simple};
use crate::differing_macro_contexts; use crate::{differing_macro_contexts, snippet_opt};
use rustc_ast::ast::InlineAsmTemplatePiece; use rustc_ast::ast::InlineAsmTemplatePiece;
use rustc_data_structures::fx::FxHashMap; use rustc_data_structures::fx::FxHashMap;
use rustc_data_structures::stable_hasher::{HashStable, StableHasher}; use rustc_data_structures::stable_hasher::{HashStable, StableHasher};
@ -9,6 +9,7 @@ use rustc_hir::{
GenericArg, GenericArgs, Guard, HirId, InlineAsmOperand, Lifetime, LifetimeName, ParamName, Pat, PatKind, Path, GenericArg, GenericArgs, Guard, HirId, InlineAsmOperand, Lifetime, LifetimeName, ParamName, Pat, PatKind, Path,
PathSegment, QPath, Stmt, StmtKind, Ty, TyKind, TypeBinding, PathSegment, QPath, Stmt, StmtKind, Ty, TyKind, TypeBinding,
}; };
use rustc_lexer::{tokenize, TokenKind};
use rustc_lint::LateContext; use rustc_lint::LateContext;
use rustc_middle::ich::StableHashingContextProvider; use rustc_middle::ich::StableHashingContextProvider;
use rustc_middle::ty::TypeckResults; use rustc_middle::ty::TypeckResults;
@ -110,8 +111,54 @@ impl HirEqInterExpr<'_, '_, '_> {
/// Checks whether two blocks are the same. /// Checks whether two blocks are the same.
fn eq_block(&mut self, left: &Block<'_>, right: &Block<'_>) -> bool { fn eq_block(&mut self, left: &Block<'_>, right: &Block<'_>) -> bool {
match (left.stmts, left.expr, right.stmts, right.expr) {
([], None, [], None) => {
// For empty blocks, check to see if the tokens are equal. This will catch the case where a macro
// expanded to nothing, or the cfg attribute was used.
let (left, right) = match (
snippet_opt(self.inner.cx, left.span),
snippet_opt(self.inner.cx, right.span),
) {
(Some(left), Some(right)) => (left, right),
_ => return true,
};
let mut left_pos = 0;
let left = tokenize(&left)
.map(|t| {
let end = left_pos + t.len;
let s = &left[left_pos..end];
left_pos = end;
(t, s)
})
.filter(|(t, _)| {
!matches!(
t.kind,
TokenKind::LineComment { .. } | TokenKind::BlockComment { .. } | TokenKind::Whitespace
)
})
.map(|(_, s)| s);
let mut right_pos = 0;
let right = tokenize(&right)
.map(|t| {
let end = right_pos + t.len;
let s = &right[right_pos..end];
right_pos = end;
(t, s)
})
.filter(|(t, _)| {
!matches!(
t.kind,
TokenKind::LineComment { .. } | TokenKind::BlockComment { .. } | TokenKind::Whitespace
)
})
.map(|(_, s)| s);
left.eq(right)
},
_ => {
over(&left.stmts, &right.stmts, |l, r| self.eq_stmt(l, r)) over(&left.stmts, &right.stmts, |l, r| self.eq_stmt(l, r))
&& both(&left.expr, &right.expr, |l, r| self.eq_expr(l, r)) && both(&left.expr, &right.expr, |l, r| self.eq_expr(l, r))
},
}
} }
#[allow(clippy::similar_names)] #[allow(clippy::similar_names)]
@ -131,7 +178,10 @@ impl HirEqInterExpr<'_, '_, '_> {
} }
} }
let is_eq = match (&reduce_exprkind(&left.kind), &reduce_exprkind(&right.kind)) { let is_eq = match (
&reduce_exprkind(self.inner.cx, &left.kind),
&reduce_exprkind(self.inner.cx, &right.kind),
) {
(&ExprKind::AddrOf(lb, l_mut, ref le), &ExprKind::AddrOf(rb, r_mut, ref re)) => { (&ExprKind::AddrOf(lb, l_mut, ref le), &ExprKind::AddrOf(rb, r_mut, ref re)) => {
lb == rb && l_mut == r_mut && self.eq_expr(le, re) lb == rb && l_mut == r_mut && self.eq_expr(le, re)
}, },
@ -360,11 +410,30 @@ impl HirEqInterExpr<'_, '_, '_> {
} }
/// Some simple reductions like `{ return }` => `return` /// Some simple reductions like `{ return }` => `return`
fn reduce_exprkind<'hir>(kind: &'hir ExprKind<'hir>) -> &ExprKind<'hir> { fn reduce_exprkind<'hir>(cx: &LateContext<'_>, kind: &'hir ExprKind<'hir>) -> &'hir ExprKind<'hir> {
if let ExprKind::Block(block, _) = kind { if let ExprKind::Block(block, _) = kind {
match (block.stmts, block.expr) { match (block.stmts, block.expr) {
// From an `if let` expression without an `else` block. The arm for the implicit wild pattern is an empty
// block with an empty span.
([], None) if block.span.is_empty() => &ExprKind::Tup(&[]),
// `{}` => `()` // `{}` => `()`
([], None) => &ExprKind::Tup(&[]), ([], None) => match snippet_opt(cx, block.span) {
// Don't reduce if there are any tokens contained in the braces
Some(snip)
if tokenize(&snip)
.map(|t| t.kind)
.filter(|t| {
!matches!(
t,
TokenKind::LineComment { .. } | TokenKind::BlockComment { .. } | TokenKind::Whitespace
)
})
.ne([TokenKind::OpenBrace, TokenKind::CloseBrace].iter().cloned()) =>
{
kind
},
_ => &ExprKind::Tup(&[]),
},
([], Some(expr)) => match expr.kind { ([], Some(expr)) => match expr.kind {
// `{ return .. }` => `return ..` // `{ return .. }` => `return ..`
ExprKind::Ret(..) => &expr.kind, ExprKind::Ret(..) => &expr.kind,

View file

@ -14,6 +14,7 @@ extern crate rustc_errors;
extern crate rustc_hir; extern crate rustc_hir;
extern crate rustc_hir_pretty; extern crate rustc_hir_pretty;
extern crate rustc_infer; extern crate rustc_infer;
extern crate rustc_lexer;
extern crate rustc_lint; extern crate rustc_lint;
extern crate rustc_middle; extern crate rustc_middle;
extern crate rustc_mir; extern crate rustc_mir;

View file

@ -120,6 +120,35 @@ fn match_same_arms() {
}, },
} }
// False positive #1390
macro_rules! empty {
($e:expr) => {};
}
match 0 {
0 => {
empty!(0);
},
1 => {
empty!(1);
},
x => {
empty!(x);
},
};
// still lint if the tokens are the same
match 0 {
0 => {
empty!(0);
},
1 => {
empty!(0);
},
x => {
empty!(x);
},
}
match_expr_like_matches_macro_priority(); match_expr_like_matches_macro_priority();
} }

View file

@ -141,8 +141,31 @@ LL | Ok(3) => println!("ok"),
| ^^^^^ | ^^^^^
= note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info) = note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info)
error: this `match` has identical arm bodies
--> $DIR/match_same_arms2.rs:144:14
|
LL | 1 => {
| ______________^
LL | | empty!(0);
LL | | },
| |_________^
|
note: same as this
--> $DIR/match_same_arms2.rs:141:14
|
LL | 0 => {
| ______________^
LL | | empty!(0);
LL | | },
| |_________^
help: consider refactoring into `0 | 1`
--> $DIR/match_same_arms2.rs:141:9
|
LL | 0 => {
| ^
error: match expression looks like `matches!` macro error: match expression looks like `matches!` macro
--> $DIR/match_same_arms2.rs:133:16 --> $DIR/match_same_arms2.rs:162:16
| |
LL | let _ans = match x { LL | let _ans = match x {
| ________________^ | ________________^
@ -154,5 +177,5 @@ LL | | };
| |
= note: `-D clippy::match-like-matches-macro` implied by `-D warnings` = note: `-D clippy::match-like-matches-macro` implied by `-D warnings`
error: aborting due to 8 previous errors error: aborting due to 9 previous errors