Skip to content

Commit 3710ec5

Browse files
committed
Auto merge of #4080 - rust-lang:rustup, r=oli-obk
Rustup to rustc 1.36.0-nightly (acc7e65 2019-05-10) Fixes breakages from #59288 Not finished yet, help appreciated. Todo: - [x] Needs to build - [x] Util should handle DropTemps, rust-lang/rust-clippy#4080 (comment) - [x] Author lint should spit out higher::if_block checks - [x] Unsure what to do in consts.rs - [x] Needs to pass tests
2 parents c74aac9 + 19cfb84 commit 3710ec5

24 files changed

+136
-104
lines changed

clippy_lints/src/block_in_if_condition.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ const COMPLEX_BLOCK_MESSAGE: &str = "in an 'if' condition, avoid complex blocks
7272

7373
impl<'a, 'tcx> LateLintPass<'a, 'tcx> for BlockInIfCondition {
7474
fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr) {
75-
if let ExprKind::If(check, then, _) = &expr.node {
75+
if let Some((check, then, _)) = higher::if_block(&expr) {
7676
if let ExprKind::Block(block, _) = &check.node {
7777
if block.rules == DefaultBlock {
7878
if block.stmts.is_empty() {

clippy_lints/src/consts.rs

+6-5
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
#![allow(clippy::float_cmp)]
22

3-
use crate::utils::{clip, sext, unsext};
3+
use crate::utils::{clip, higher, sext, unsext};
44
use if_chain::if_chain;
55
use rustc::hir::def::{DefKind, Res};
66
use rustc::hir::*;
@@ -15,7 +15,6 @@ use std::convert::TryFrom;
1515
use std::convert::TryInto;
1616
use std::hash::{Hash, Hasher};
1717
use syntax::ast::{FloatTy, LitKind};
18-
use syntax::ptr::P;
1918
use syntax_pos::symbol::{LocalInternedString, Symbol};
2019

2120
/// A `LitKind`-like enum to fold constant `Expr`s into.
@@ -222,10 +221,12 @@ pub struct ConstEvalLateContext<'a, 'tcx: 'a> {
222221
impl<'c, 'cc> ConstEvalLateContext<'c, 'cc> {
223222
/// Simple constant folding: Insert an expression, get a constant or none.
224223
pub fn expr(&mut self, e: &Expr) -> Option<Constant> {
224+
if let Some((ref cond, ref then, otherwise)) = higher::if_block(&e) {
225+
return self.ifthenelse(cond, then, otherwise);
226+
}
225227
match e.node {
226228
ExprKind::Path(ref qpath) => self.fetch_path(qpath, e.hir_id),
227229
ExprKind::Block(ref block, _) => self.block(block),
228-
ExprKind::If(ref cond, ref then, ref otherwise) => self.ifthenelse(cond, then, otherwise),
229230
ExprKind::Lit(ref lit) => Some(lit_to_constant(&lit.node, self.tables.expr_ty(e))),
230231
ExprKind::Array(ref vec) => self.multi(vec).map(Constant::Vec),
231232
ExprKind::Tup(ref tup) => self.multi(tup).map(Constant::Tuple),
@@ -358,10 +359,10 @@ impl<'c, 'cc> ConstEvalLateContext<'c, 'cc> {
358359
}
359360
}
360361

361-
fn ifthenelse(&mut self, cond: &Expr, then: &P<Expr>, otherwise: &Option<P<Expr>>) -> Option<Constant> {
362+
fn ifthenelse(&mut self, cond: &Expr, then: &Expr, otherwise: Option<&Expr>) -> Option<Constant> {
362363
if let Some(Constant::Bool(b)) = self.expr(cond) {
363364
if b {
364-
self.expr(&**then)
365+
self.expr(&*then)
365366
} else {
366367
otherwise.as_ref().and_then(|expr| self.expr(expr))
367368
}

clippy_lints/src/copies.rs

+7-9
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
use crate::utils::{get_parent_expr, in_macro, snippet, span_lint_and_then, span_note_and_lint};
1+
use crate::utils::{get_parent_expr, higher, in_macro, snippet, span_lint_and_then, span_note_and_lint};
22
use crate::utils::{SpanlessEq, SpanlessHash};
33
use rustc::hir::*;
44
use rustc::lint::{LateContext, LateLintPass, LintArray, LintPass};
@@ -109,13 +109,11 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for CopyAndPaste {
109109
fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr) {
110110
if !in_macro(expr.span) {
111111
// skip ifs directly in else, it will be checked in the parent if
112-
if let Some(&Expr {
113-
node: ExprKind::If(_, _, Some(ref else_expr)),
114-
..
115-
}) = get_parent_expr(cx, expr)
116-
{
117-
if else_expr.hir_id == expr.hir_id {
118-
return;
112+
if let Some(expr) = get_parent_expr(cx, expr) {
113+
if let Some((_, _, Some(ref else_expr))) = higher::if_block(&expr) {
114+
if else_expr.hir_id == expr.hir_id {
115+
return;
116+
}
119117
}
120118
}
121119

@@ -236,7 +234,7 @@ fn if_sequence(mut expr: &Expr) -> (SmallVec<[&Expr; 1]>, SmallVec<[&Block; 1]>)
236234
let mut conds = SmallVec::new();
237235
let mut blocks: SmallVec<[&Block; 1]> = SmallVec::new();
238236

239-
while let ExprKind::If(ref cond, ref then_expr, ref else_expr) = expr.node {
237+
while let Some((ref cond, ref then_expr, ref else_expr)) = higher::if_block(&expr) {
240238
conds.push(&**cond);
241239
if let ExprKind::Block(ref block, _) = then_expr.node {
242240
blocks.push(block);

clippy_lints/src/entry.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
use crate::utils::SpanlessEq;
2-
use crate::utils::{get_item_name, match_type, paths, snippet, span_lint_and_then, walk_ptrs_ty};
2+
use crate::utils::{get_item_name, higher, match_type, paths, snippet, span_lint_and_then, walk_ptrs_ty};
33
use if_chain::if_chain;
44
use rustc::hir::intravisit::{walk_expr, NestedVisitorMap, Visitor};
55
use rustc::hir::*;
@@ -41,7 +41,7 @@ declare_lint_pass!(HashMapPass => [MAP_ENTRY]);
4141

4242
impl<'a, 'tcx> LateLintPass<'a, 'tcx> for HashMapPass {
4343
fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr) {
44-
if let ExprKind::If(ref check, ref then_block, ref else_block) = expr.node {
44+
if let Some((ref check, ref then_block, ref else_block)) = higher::if_block(&expr) {
4545
if let ExprKind::Unary(UnOp::UnNot, ref check) = check.node {
4646
if let Some((ty, map, key)) = check_cond(cx, check) {
4747
// in case of `if !m.contains_key(&k) { m.insert(k, v); }`

clippy_lints/src/implicit_return.rs

-7
Original file line numberDiff line numberDiff line change
@@ -74,13 +74,6 @@ impl ImplicitReturn {
7474
Self::lint(cx, expr.span, break_expr.span, "change `break` to `return` as shown");
7575
}
7676
},
77-
ExprKind::If(.., if_expr, else_expr) => {
78-
Self::expr_match(cx, if_expr);
79-
80-
if let Some(else_expr) = else_expr {
81-
Self::expr_match(cx, else_expr);
82-
}
83-
},
8477
ExprKind::Match(.., arms, source) => {
8578
let check_all_arms = match source {
8679
MatchSource::IfLetDesugar {

clippy_lints/src/let_if_seq.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
use crate::utils::{snippet, span_lint_and_then};
1+
use crate::utils::{higher, snippet, span_lint_and_then};
22
use if_chain::if_chain;
33
use rustc::hir;
44
use rustc::hir::def::Res;
@@ -63,7 +63,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for LetIfSeq {
6363
if let hir::StmtKind::Local(ref local) = stmt.node;
6464
if let hir::PatKind::Binding(mode, canonical_id, ident, None) = local.pat.node;
6565
if let hir::StmtKind::Expr(ref if_) = expr.node;
66-
if let hir::ExprKind::If(ref cond, ref then, ref else_) = if_.node;
66+
if let Some((ref cond, ref then, ref else_)) = higher::if_block(&if_);
6767
if !used_in_expr(cx, canonical_id, cond);
6868
if let hir::ExprKind::Block(ref then, _) = then.node;
6969
if let Some(value) = check_assign(cx, canonical_id, &*then);

clippy_lints/src/loops.rs

+1-9
Original file line numberDiff line numberDiff line change
@@ -686,14 +686,6 @@ fn never_loop_expr(expr: &Expr, main_loop_id: HirId) -> NeverLoopResult {
686686
| ExprKind::Assign(ref e1, ref e2)
687687
| ExprKind::AssignOp(_, ref e1, ref e2)
688688
| ExprKind::Index(ref e1, ref e2) => never_loop_expr_all(&mut [&**e1, &**e2].iter().cloned(), main_loop_id),
689-
ExprKind::If(ref e, ref e2, ref e3) => {
690-
let e1 = never_loop_expr(e, main_loop_id);
691-
let e2 = never_loop_expr(e2, main_loop_id);
692-
let e3 = e3
693-
.as_ref()
694-
.map_or(NeverLoopResult::Otherwise, |e| never_loop_expr(e, main_loop_id));
695-
combine_seq(e1, combine_branches(e2, e3))
696-
},
697689
ExprKind::Loop(ref b, _, _) => {
698690
// Break can come from the inner loop so remove them.
699691
absorb_break(&never_loop_block(b, main_loop_id))
@@ -2204,7 +2196,7 @@ fn is_loop(expr: &Expr) -> bool {
22042196

22052197
fn is_conditional(expr: &Expr) -> bool {
22062198
match expr.node {
2207-
ExprKind::If(..) | ExprKind::Match(..) => true,
2199+
ExprKind::Match(..) => true,
22082200
_ => false,
22092201
}
22102202
}

clippy_lints/src/methods/mod.rs

-1
Original file line numberDiff line numberDiff line change
@@ -1266,7 +1266,6 @@ fn lint_expect_fun_call(cx: &LateContext<'_, '_>, expr: &hir::Expr, method_span:
12661266
hir::ExprKind::Call(..)
12671267
| hir::ExprKind::MethodCall(..)
12681268
// These variants are debatable or require further examination
1269-
| hir::ExprKind::If(..)
12701269
| hir::ExprKind::Match(..)
12711270
| hir::ExprKind::Block{ .. } => true,
12721271
_ => false,

clippy_lints/src/methods/unnecessary_filter_map.rs

-6
Original file line numberDiff line numberDiff line change
@@ -89,12 +89,6 @@ fn check_expression<'a, 'tcx: 'a>(
8989
(false, false)
9090
}
9191
},
92-
// There must be an else_arm or there will be a type error
93-
hir::ExprKind::If(_, ref if_arm, Some(ref else_arm)) => {
94-
let if_check = check_expression(cx, arg_id, if_arm);
95-
let else_check = check_expression(cx, arg_id, else_arm);
96-
(if_check.0 | else_check.0, if_check.1 | else_check.1)
97-
},
9892
hir::ExprKind::Match(_, ref arms, _) => {
9993
let mut found_mapping = false;
10094
let mut found_filtering = false;

clippy_lints/src/needless_bool.rs

+3-3
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
//! This lint is **warn** by default
44
55
use crate::utils::sugg::Sugg;
6-
use crate::utils::{in_macro, span_lint, span_lint_and_sugg};
6+
use crate::utils::{higher, in_macro, span_lint, span_lint_and_sugg};
77
use rustc::hir::*;
88
use rustc::lint::{LateContext, LateLintPass, LintArray, LintPass};
99
use rustc::{declare_lint_pass, declare_tool_lint};
@@ -59,7 +59,7 @@ declare_lint_pass!(NeedlessBool => [NEEDLESS_BOOL]);
5959
impl<'a, 'tcx> LateLintPass<'a, 'tcx> for NeedlessBool {
6060
fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, e: &'tcx Expr) {
6161
use self::Expression::*;
62-
if let ExprKind::If(ref pred, ref then_block, Some(ref else_expr)) = e.node {
62+
if let Some((ref pred, ref then_block, Some(ref else_expr))) = higher::if_block(&e) {
6363
let reduce = |ret, not| {
6464
let mut applicability = Applicability::MachineApplicable;
6565
let snip = Sugg::hir_with_applicability(cx, pred, "<predicate>", &mut applicability);
@@ -119,7 +119,7 @@ fn parent_node_is_if_expr<'a, 'b>(expr: &Expr, cx: &LateContext<'a, 'b>) -> bool
119119
let parent_node = cx.tcx.hir().get_by_hir_id(parent_id);
120120

121121
if let rustc::hir::Node::Expr(e) = parent_node {
122-
if let ExprKind::If(_, _, _) = e.node {
122+
if higher::if_block(&e).is_some() {
123123
return true;
124124
}
125125
}

clippy_lints/src/question_mark.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ use syntax::ptr::P;
88

99
use crate::utils::paths::*;
1010
use crate::utils::sugg::Sugg;
11-
use crate::utils::{match_type, span_lint_and_then, SpanlessEq};
11+
use crate::utils::{higher, match_type, span_lint_and_then, SpanlessEq};
1212

1313
declare_clippy_lint! {
1414
/// **What it does:** Checks for expressions that could be replaced by the question mark operator.
@@ -48,7 +48,7 @@ impl QuestionMark {
4848
/// If it matches, it will suggest to use the question mark operator instead
4949
fn check_is_none_and_early_return_none(cx: &LateContext<'_, '_>, expr: &Expr) {
5050
if_chain! {
51-
if let ExprKind::If(if_expr, body, else_) = &expr.node;
51+
if let Some((if_expr, body, else_)) = higher::if_block(&expr);
5252
if let ExprKind::MethodCall(segment, _, args) = &if_expr.node;
5353
if segment.ident.name == "is_none";
5454
if Self::expression_returns_none(cx, body);

clippy_lints/src/shadow.rs

-7
Original file line numberDiff line numberDiff line change
@@ -319,13 +319,6 @@ fn check_expr<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr, bindings:
319319
check_expr(cx, e, bindings)
320320
}
321321
},
322-
ExprKind::If(ref cond, ref then, ref otherwise) => {
323-
check_expr(cx, cond, bindings);
324-
check_expr(cx, &**then, bindings);
325-
if let Some(ref o) = *otherwise {
326-
check_expr(cx, o, bindings);
327-
}
328-
},
329322
ExprKind::While(ref cond, ref block, _) => {
330323
check_expr(cx, cond, bindings);
331324
check_block(cx, block, bindings);

clippy_lints/src/unwrap.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ use if_chain::if_chain;
22
use rustc::lint::{LateContext, LateLintPass, LintArray, LintPass};
33
use rustc::{declare_lint_pass, declare_tool_lint};
44

5-
use crate::utils::{in_macro, match_type, paths, span_lint_and_then, usage::is_potentially_mutated};
5+
use crate::utils::{higher::if_block, in_macro, match_type, paths, span_lint_and_then, usage::is_potentially_mutated};
66
use rustc::hir::intravisit::*;
77
use rustc::hir::*;
88
use syntax::source_map::Span;
@@ -130,7 +130,7 @@ impl<'a, 'tcx: 'a> UnwrappableVariablesVisitor<'a, 'tcx> {
130130

131131
impl<'a, 'tcx: 'a> Visitor<'tcx> for UnwrappableVariablesVisitor<'a, 'tcx> {
132132
fn visit_expr(&mut self, expr: &'tcx Expr) {
133-
if let ExprKind::If(cond, then, els) = &expr.node {
133+
if let Some((cond, then, els)) = if_block(&expr) {
134134
walk_expr(self, cond);
135135
self.visit_branch(cond, then, false);
136136
if let Some(els) = els {

clippy_lints/src/utils/author.rs

+31-20
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
//! A group of attributes that can be attached to Rust code in order
22
//! to generate a clippy lint detecting said code automatically.
33
4-
use crate::utils::get_attr;
4+
use crate::utils::{get_attr, higher};
55
use rustc::hir;
66
use rustc::hir::intravisit::{NestedVisitorMap, Visitor};
77
use rustc::hir::{BindingAnnotation, Expr, ExprKind, Pat, PatKind, QPath, Stmt, StmtKind, TyKind};
@@ -187,6 +187,32 @@ struct PrintVisitor {
187187
impl<'tcx> Visitor<'tcx> for PrintVisitor {
188188
#[allow(clippy::too_many_lines)]
189189
fn visit_expr(&mut self, expr: &Expr) {
190+
// handle if desugarings
191+
// TODO add more desugarings here
192+
if let Some((cond, then, opt_else)) = higher::if_block(&expr) {
193+
let cond_pat = self.next("cond");
194+
let then_pat = self.next("then");
195+
if let Some(else_) = opt_else {
196+
let else_pat = self.next("else_");
197+
println!(
198+
" if let Some((ref {}, ref {}, Some({}))) = higher::if_block(&{});",
199+
cond_pat, then_pat, else_pat, self.current
200+
);
201+
self.current = else_pat;
202+
self.visit_expr(else_);
203+
} else {
204+
println!(
205+
" if let Some((ref {}, ref {}, None)) = higher::if_block(&{});",
206+
cond_pat, then_pat, self.current
207+
);
208+
}
209+
self.current = cond_pat;
210+
self.visit_expr(cond);
211+
self.current = then_pat;
212+
self.visit_expr(then);
213+
return;
214+
}
215+
190216
print!(" if let ExprKind::");
191217
let current = format!("{}.node", self.current);
192218
match expr.node {
@@ -296,25 +322,6 @@ impl<'tcx> Visitor<'tcx> for PrintVisitor {
296322
self.current = cast_pat;
297323
self.visit_expr(expr);
298324
},
299-
ExprKind::If(ref cond, ref then, ref opt_else) => {
300-
let cond_pat = self.next("cond");
301-
let then_pat = self.next("then");
302-
if let Some(ref else_) = *opt_else {
303-
let else_pat = self.next("else_");
304-
println!(
305-
"If(ref {}, ref {}, Some(ref {})) = {};",
306-
cond_pat, then_pat, else_pat, current
307-
);
308-
self.current = else_pat;
309-
self.visit_expr(else_);
310-
} else {
311-
println!("If(ref {}, ref {}, None) = {};", cond_pat, then_pat, current);
312-
}
313-
self.current = cond_pat;
314-
self.visit_expr(cond);
315-
self.current = then_pat;
316-
self.visit_expr(then);
317-
},
318325
ExprKind::While(ref cond, ref body, _) => {
319326
let cond_pat = self.next("cond");
320327
let body_pat = self.next("body");
@@ -684,6 +691,10 @@ fn desugaring_name(des: hir::MatchSource) -> String {
684691
"MatchSource::IfLetDesugar {{ contains_else_clause: {} }}",
685692
contains_else_clause
686693
),
694+
hir::MatchSource::IfDesugar { contains_else_clause } => format!(
695+
"MatchSource::IfDesugar {{ contains_else_clause: {} }}",
696+
contains_else_clause
697+
),
687698
hir::MatchSource::AwaitDesugar => "MatchSource::AwaitDesugar".to_string(),
688699
}
689700
}

clippy_lints/src/utils/higher.rs

+21
Original file line numberDiff line numberDiff line change
@@ -199,6 +199,27 @@ pub fn for_loop(expr: &hir::Expr) -> Option<(&hir::Pat, &hir::Expr, &hir::Expr)>
199199
None
200200
}
201201

202+
/// Recover the essential nodes of a desugared if block
203+
/// `if cond { then } else { els }` becomes `(cond, then, Some(els))`
204+
pub fn if_block(expr: &hir::Expr) -> Option<(&hir::Expr, &hir::Expr, Option<&hir::Expr>)> {
205+
if let hir::ExprKind::Match(ref cond, ref arms, hir::MatchSource::IfDesugar { contains_else_clause }) = expr.node {
206+
let cond = if let hir::ExprKind::DropTemps(ref cond) = cond.node {
207+
cond
208+
} else {
209+
panic!("If block desugar must contain DropTemps");
210+
};
211+
let then = &arms[0].body;
212+
let els = if contains_else_clause {
213+
Some(&*arms[1].body)
214+
} else {
215+
None
216+
};
217+
Some((cond, then, els))
218+
} else {
219+
None
220+
}
221+
}
222+
202223
/// Represent the pre-expansion arguments of a `vec!` invocation.
203224
pub enum VecArgs<'a> {
204225
/// `vec![elem; len]`

clippy_lints/src/utils/hir_utils.rs

-12
Original file line numberDiff line numberDiff line change
@@ -114,9 +114,6 @@ impl<'a, 'tcx: 'a> SpanlessEq<'a, 'tcx> {
114114
(&ExprKind::Index(ref la, ref li), &ExprKind::Index(ref ra, ref ri)) => {
115115
self.eq_expr(la, ra) && self.eq_expr(li, ri)
116116
},
117-
(&ExprKind::If(ref lc, ref lt, ref le), &ExprKind::If(ref rc, ref rt, ref re)) => {
118-
self.eq_expr(lc, rc) && self.eq_expr(&**lt, &**rt) && both(le, re, |l, r| self.eq_expr(l, r))
119-
},
120117
(&ExprKind::Lit(ref l), &ExprKind::Lit(ref r)) => l.node == r.node,
121118
(&ExprKind::Loop(ref lb, ref ll, ref lls), &ExprKind::Loop(ref rb, ref rl, ref rls)) => {
122119
lls == rls && self.eq_block(lb, rb) && both(ll, rl, |l, r| l.ident.as_str() == r.ident.as_str())
@@ -493,15 +490,6 @@ impl<'a, 'tcx: 'a> SpanlessHash<'a, 'tcx> {
493490
let c: fn(_, _, _) -> _ = ExprKind::InlineAsm;
494491
c.hash(&mut self.s);
495492
},
496-
ExprKind::If(ref cond, ref t, ref e) => {
497-
let c: fn(_, _, _) -> _ = ExprKind::If;
498-
c.hash(&mut self.s);
499-
self.hash_expr(cond);
500-
self.hash_expr(&**t);
501-
if let Some(ref e) = *e {
502-
self.hash_expr(e);
503-
}
504-
},
505493
ExprKind::Lit(ref l) => {
506494
let c: fn(_) -> _ = ExprKind::Lit;
507495
c.hash(&mut self.s);

0 commit comments

Comments
 (0)