diff --git a/Cargo.lock b/Cargo.lock index 9e0624c57ae94..8221fef6bc6b8 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2729,9 +2729,9 @@ dependencies = [ [[package]] name = "psm" -version = "0.1.11" +version = "0.1.15" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "96e0536f6528466dbbbbe6b986c34175a8d0ff25b794c4bacda22e068cd2f2c5" +checksum = "0617ee61163b5d941d804065ce49040967610a4d4278fae73e096a057b01d358" dependencies = [ "cc", ] @@ -4810,12 +4810,12 @@ checksum = "a8f112729512f8e442d81f95a8a7ddf2b7c6b8a1a6f509a95864142b30cab2d3" [[package]] name = "stacker" -version = "0.1.12" +version = "0.1.14" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "21ccb4c06ec57bc82d0f610f1a2963d7648700e43a6f513e564b9c89f7991786" +checksum = "90939d5171a4420b3ff5fbc8954d641e7377335454c259dcb80786f3f21dc9b4" dependencies = [ "cc", - "cfg-if 0.1.10", + "cfg-if 1.0.0", "libc", "psm", "winapi", diff --git a/RELEASES.md b/RELEASES.md index 1d9ad3160f77e..da20d6dc9ea64 100644 --- a/RELEASES.md +++ b/RELEASES.md @@ -146,18 +146,13 @@ Version 1.54.0 (2021-07-29) Language ----------------------- -- [You can now use macros for values in built-in attribute macros.][83366] - While a seemingly minor addition on its own, this enables a lot of - powerful functionality when combined correctly. Most notably you can - now include external documentation in your crate by writing the following. +- [You can now use macros for values in some built-in attributes.][83366] + This primarily allows you to call macros within the `#[doc]` attribute. For + example, to include external documentation in your crate, you can now write + the following: ```rust #![doc = include_str!("README.md")] ``` - You can also use this to include auto-generated modules: - ```rust - #[path = concat!(env!("OUT_DIR"), "/generated.rs")] - mod generated; - ``` - [You can now cast between unsized slice types (and types which contain unsized slices) in `const fn`.][85078] diff --git a/compiler/rustc_data_structures/Cargo.toml b/compiler/rustc_data_structures/Cargo.toml index 2063cb8c52bb1..bc13ca26e2e22 100644 --- a/compiler/rustc_data_structures/Cargo.toml +++ b/compiler/rustc_data_structures/Cargo.toml @@ -25,7 +25,7 @@ rustc_index = { path = "../rustc_index", package = "rustc_index" } bitflags = "1.2.1" measureme = "9.1.0" libc = "0.2" -stacker = "0.1.12" +stacker = "0.1.14" tempfile = "3.2" [dependencies.parking_lot] diff --git a/compiler/rustc_mir_build/src/build/matches/simplify.rs b/compiler/rustc_mir_build/src/build/matches/simplify.rs index 13cfc3695cc9f..1feb8b0d7a06d 100644 --- a/compiler/rustc_mir_build/src/build/matches/simplify.rs +++ b/compiler/rustc_mir_build/src/build/matches/simplify.rs @@ -155,12 +155,16 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { ascription: thir::Ascription { variance, user_ty, user_ty_span }, } => { // Apply the type ascription to the value at `match_pair.place`, which is the - candidate.ascriptions.push(Ascription { - span: user_ty_span, - user_ty, - source: match_pair.place.clone().into_place(self.tcx, self.typeck_results), - variance, - }); + if let Ok(place_resolved) = + match_pair.place.clone().try_upvars_resolved(self.tcx, self.typeck_results) + { + candidate.ascriptions.push(Ascription { + span: user_ty_span, + user_ty, + source: place_resolved.into_place(self.tcx, self.typeck_results), + variance, + }); + } candidate.match_pairs.push(MatchPair::new(match_pair.place, subpattern)); @@ -173,15 +177,19 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { } PatKind::Binding { name, mutability, mode, var, ty, ref subpattern, is_primary: _ } => { - candidate.bindings.push(Binding { - name, - mutability, - span: match_pair.pattern.span, - source: match_pair.place.clone().into_place(self.tcx, self.typeck_results), - var_id: var, - var_ty: ty, - binding_mode: mode, - }); + if let Ok(place_resolved) = + match_pair.place.clone().try_upvars_resolved(self.tcx, self.typeck_results) + { + candidate.bindings.push(Binding { + name, + mutability, + span: match_pair.pattern.span, + source: place_resolved.into_place(self.tcx, self.typeck_results), + var_id: var, + var_ty: ty, + binding_mode: mode, + }); + } if let Some(subpattern) = subpattern.as_ref() { // this is the `x @ P` case; have to keep matching against `P` now diff --git a/compiler/rustc_mir_build/src/build/matches/util.rs b/compiler/rustc_mir_build/src/build/matches/util.rs index 3cf8ae6efd946..88dd76e37c112 100644 --- a/compiler/rustc_mir_build/src/build/matches/util.rs +++ b/compiler/rustc_mir_build/src/build/matches/util.rs @@ -31,15 +31,20 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { suffix: &'pat [Pat<'tcx>], ) { let tcx = self.tcx; - let (min_length, exact_size) = match place - .clone() - .into_place(tcx, self.typeck_results) - .ty(&self.local_decls, tcx) - .ty - .kind() + let (min_length, exact_size) = if let Ok(place_resolved) = + place.clone().try_upvars_resolved(tcx, self.typeck_results) { - ty::Array(_, length) => (length.eval_usize(tcx, self.param_env), true), - _ => ((prefix.len() + suffix.len()).try_into().unwrap(), false), + match place_resolved + .into_place(tcx, self.typeck_results) + .ty(&self.local_decls, tcx) + .ty + .kind() + { + ty::Array(_, length) => (length.eval_usize(tcx, self.param_env), true), + _ => ((prefix.len() + suffix.len()).try_into().unwrap(), false), + } + } else { + ((prefix.len() + suffix.len()).try_into().unwrap(), false) }; match_pairs.extend(prefix.iter().enumerate().map(|(idx, subpattern)| { diff --git a/compiler/rustc_typeck/src/expr_use_visitor.rs b/compiler/rustc_typeck/src/expr_use_visitor.rs index 3b241317aa3ef..b0c95939bb77d 100644 --- a/compiler/rustc_typeck/src/expr_use_visitor.rs +++ b/compiler/rustc_typeck/src/expr_use_visitor.rs @@ -269,6 +269,9 @@ impl<'a, 'tcx> ExprUseVisitor<'a, 'tcx> { if def.variants.len() > 1 { needs_to_be_read = true; } + } else { + // If it is not ty::Adt, then it should be read + needs_to_be_read = true; } } PatKind::Lit(_) | PatKind::Range(..) => { diff --git a/src/bootstrap/sanity.rs b/src/bootstrap/sanity.rs index a28762ac485ec..74e50c606107f 100644 --- a/src/bootstrap/sanity.rs +++ b/src/bootstrap/sanity.rs @@ -166,11 +166,6 @@ pub fn check(build: &mut Build) { } for target in &build.targets { - // Can't compile for iOS unless we're on macOS - if target.contains("apple-ios") && !build.build.contains("apple-darwin") { - panic!("the iOS target is only supported on macOS"); - } - build .config .target_config diff --git a/src/test/codegen/naked-noinline.rs b/src/test/codegen/naked-noinline.rs index d9e6f6c34ec5a..d576a53826c1f 100644 --- a/src/test/codegen/naked-noinline.rs +++ b/src/test/codegen/naked-noinline.rs @@ -1,5 +1,6 @@ // Checks that naked functions are never inlined. // compile-flags: -O -Zmir-opt-level=3 +// needs-asm-support // ignore-wasm32 #![crate_type = "lib"] #![feature(asm)] diff --git a/src/test/ui/closures/2229_closure_analysis/issue-87987.rs b/src/test/ui/closures/2229_closure_analysis/issue-87987.rs new file mode 100644 index 0000000000000..5dc2cb7e71099 --- /dev/null +++ b/src/test/ui/closures/2229_closure_analysis/issue-87987.rs @@ -0,0 +1,30 @@ +// run-pass +// edition:2021 + +struct Props { + field_1: u32, //~ WARNING: field is never read: `field_1` + field_2: u32, //~ WARNING: field is never read: `field_2` +} + +fn main() { + // Test 1 + let props_2 = Props { //~ WARNING: unused variable: `props_2` + field_1: 1, + field_2: 1, + }; + + let _ = || { + let _: Props = props_2; + }; + + // Test 2 + let mut arr = [1, 3, 4, 5]; + + let mref = &mut arr; + + let _c = || match arr { + [_, _, _, _] => println!("A") + }; + + println!("{:#?}", mref); +} diff --git a/src/test/ui/closures/2229_closure_analysis/issue-87987.stderr b/src/test/ui/closures/2229_closure_analysis/issue-87987.stderr new file mode 100644 index 0000000000000..aa7012c3618c2 --- /dev/null +++ b/src/test/ui/closures/2229_closure_analysis/issue-87987.stderr @@ -0,0 +1,24 @@ +warning: unused variable: `props_2` + --> $DIR/issue-87987.rs:11:9 + | +LL | let props_2 = Props { + | ^^^^^^^ help: if this is intentional, prefix it with an underscore: `_props_2` + | + = note: `#[warn(unused_variables)]` on by default + +warning: field is never read: `field_1` + --> $DIR/issue-87987.rs:5:5 + | +LL | field_1: u32, + | ^^^^^^^^^^^^ + | + = note: `#[warn(dead_code)]` on by default + +warning: field is never read: `field_2` + --> $DIR/issue-87987.rs:6:5 + | +LL | field_2: u32, + | ^^^^^^^^^^^^ + +warning: 3 warnings emitted + diff --git a/src/test/ui/closures/2229_closure_analysis/issue-87988.rs b/src/test/ui/closures/2229_closure_analysis/issue-87988.rs new file mode 100644 index 0000000000000..27e7fabf11ab6 --- /dev/null +++ b/src/test/ui/closures/2229_closure_analysis/issue-87988.rs @@ -0,0 +1,19 @@ +// run-pass +// edition:2021 + +const LIBUSB_HOTPLUG_EVENT_DEVICE_ARRIVED: i32 = 0x01; +const LIBUSB_HOTPLUG_EVENT_DEVICE_LEFT: i32 = 0x02; + +pub fn hotplug_callback(event: i32) { + let _ = || { + match event { + LIBUSB_HOTPLUG_EVENT_DEVICE_ARRIVED => (), + LIBUSB_HOTPLUG_EVENT_DEVICE_LEFT => (), + _ => (), + }; + }; +} + +fn main() { + hotplug_callback(1); +} diff --git a/src/test/ui/closures/2229_closure_analysis/match-edge-cases.rs b/src/test/ui/closures/2229_closure_analysis/match-edge-cases.rs new file mode 100644 index 0000000000000..914ebbe26a57d --- /dev/null +++ b/src/test/ui/closures/2229_closure_analysis/match-edge-cases.rs @@ -0,0 +1,44 @@ +// run-pass +// edition:2021 + +const PATTERN_REF: &str = "Hello World"; +const NUMBER: i32 = 30; +const NUMBER_POINTER: *const i32 = &NUMBER; + +pub fn edge_case_ref(event: &str) { + let _ = || { + match event { + PATTERN_REF => (), + _ => (), + }; + }; +} + +pub fn edge_case_str(event: String) { + let _ = || { + match event.as_str() { + "hello" => (), + _ => (), + }; + }; +} + +pub fn edge_case_raw_ptr(event: *const i32) { + let _ = || { + match event { + NUMBER_POINTER => (), + _ => (), + }; + }; +} + +pub fn edge_case_char(event: char) { + let _ = || { + match event { + 'a' => (), + _ => (), + }; + }; +} + +fn main() {} diff --git a/src/test/ui/feature-gates/feature-gate-global_asm.rs b/src/test/ui/feature-gates/feature-gate-global_asm.rs index 8c9f22e975257..1420eef299bbf 100644 --- a/src/test/ui/feature-gates/feature-gate-global_asm.rs +++ b/src/test/ui/feature-gates/feature-gate-global_asm.rs @@ -1,3 +1,5 @@ +// needs-asm-support + global_asm!(""); //~ ERROR `global_asm!` is not stable fn main() {} diff --git a/src/test/ui/feature-gates/feature-gate-global_asm.stderr b/src/test/ui/feature-gates/feature-gate-global_asm.stderr index e07fbf00d57c2..7c4d3e3e6e595 100644 --- a/src/test/ui/feature-gates/feature-gate-global_asm.stderr +++ b/src/test/ui/feature-gates/feature-gate-global_asm.stderr @@ -1,5 +1,5 @@ error[E0658]: use of unstable library feature 'global_asm': `global_asm!` is not stable enough for use and is subject to change - --> $DIR/feature-gate-global_asm.rs:1:1 + --> $DIR/feature-gate-global_asm.rs:3:1 | LL | global_asm!(""); | ^^^^^^^^^^ diff --git a/src/test/ui/macros/macro-expanded-include/test.rs b/src/test/ui/macros/macro-expanded-include/test.rs index f1a71059a8901..6a2b5ef72419f 100644 --- a/src/test/ui/macros/macro-expanded-include/test.rs +++ b/src/test/ui/macros/macro-expanded-include/test.rs @@ -1,4 +1,4 @@ -// ignore-emscripten no llvm_asm! support +// needs-asm-support // build-pass (FIXME(62277): could be check-pass?) #![feature(asm)] #![allow(unused)] diff --git a/src/test/ui/unsafe/inline_asm.mir.stderr b/src/test/ui/unsafe/inline_asm.mir.stderr index 7dc62a1ead138..5d9828b5594e3 100644 --- a/src/test/ui/unsafe/inline_asm.mir.stderr +++ b/src/test/ui/unsafe/inline_asm.mir.stderr @@ -1,5 +1,5 @@ error[E0133]: use of inline assembly is unsafe and requires unsafe function or block - --> $DIR/inline_asm.rs:9:5 + --> $DIR/inline_asm.rs:10:5 | LL | asm!("nop"); | ^^^^^^^^^^^^ use of inline assembly @@ -7,7 +7,7 @@ LL | asm!("nop"); = note: inline assembly is entirely unchecked and can cause undefined behavior error[E0133]: use of inline assembly is unsafe and requires unsafe function or block - --> $DIR/inline_asm.rs:10:5 + --> $DIR/inline_asm.rs:11:5 | LL | llvm_asm!("nop"); | ^^^^^^^^^^^^^^^^^ use of inline assembly diff --git a/src/test/ui/unsafe/inline_asm.rs b/src/test/ui/unsafe/inline_asm.rs index 995292a99030f..8e1325bc0a8f5 100644 --- a/src/test/ui/unsafe/inline_asm.rs +++ b/src/test/ui/unsafe/inline_asm.rs @@ -1,5 +1,6 @@ // revisions: mir thir // [thir]compile-flags: -Z thir-unsafeck +// needs-asm-support #![feature(llvm_asm)] #![feature(asm)] diff --git a/src/test/ui/unsafe/inline_asm.thir.stderr b/src/test/ui/unsafe/inline_asm.thir.stderr index 7dc62a1ead138..5d9828b5594e3 100644 --- a/src/test/ui/unsafe/inline_asm.thir.stderr +++ b/src/test/ui/unsafe/inline_asm.thir.stderr @@ -1,5 +1,5 @@ error[E0133]: use of inline assembly is unsafe and requires unsafe function or block - --> $DIR/inline_asm.rs:9:5 + --> $DIR/inline_asm.rs:10:5 | LL | asm!("nop"); | ^^^^^^^^^^^^ use of inline assembly @@ -7,7 +7,7 @@ LL | asm!("nop"); = note: inline assembly is entirely unchecked and can cause undefined behavior error[E0133]: use of inline assembly is unsafe and requires unsafe function or block - --> $DIR/inline_asm.rs:10:5 + --> $DIR/inline_asm.rs:11:5 | LL | llvm_asm!("nop"); | ^^^^^^^^^^^^^^^^^ use of inline assembly diff --git a/src/tools/clippy/clippy_lints/src/collapsible_match.rs b/src/tools/clippy/clippy_lints/src/collapsible_match.rs index 6b63c2cf157ac..a42eee53459eb 100644 --- a/src/tools/clippy/clippy_lints/src/collapsible_match.rs +++ b/src/tools/clippy/clippy_lints/src/collapsible_match.rs @@ -1,9 +1,9 @@ use clippy_utils::diagnostics::span_lint_and_then; use clippy_utils::visitors::LocalUsedVisitor; -use clippy_utils::{higher, is_lang_ctor, path_to_local, peel_ref_operators, SpanlessEq}; +use clippy_utils::{higher, is_lang_ctor, is_unit_expr, path_to_local, peel_ref_operators, SpanlessEq}; use if_chain::if_chain; use rustc_hir::LangItem::OptionNone; -use rustc_hir::{Expr, ExprKind, Guard, HirId, Pat, PatKind, StmtKind}; +use rustc_hir::{Arm, Expr, ExprKind, Guard, HirId, MatchSource, Pat, PatKind, StmtKind}; use rustc_lint::{LateContext, LateLintPass}; use rustc_session::{declare_lint_pass, declare_tool_lint}; use rustc_span::{MultiSpan, Span}; @@ -49,104 +49,87 @@ declare_lint_pass!(CollapsibleMatch => [COLLAPSIBLE_MATCH]); impl<'tcx> LateLintPass<'tcx> for CollapsibleMatch { fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &Expr<'tcx>) { - if let Some(higher::IfLet { - let_pat, - if_then, - if_else, - .. - }) = higher::IfLet::hir(expr) - { - check_arm(cx, if_then, None, let_pat, if_else); - - check_if_let(cx, if_then, let_pat); - } - - if let ExprKind::Match(_expr, arms, _source) = expr.kind { - if let Some(wild_arm) = arms.iter().rfind(|arm| is_wild_like(cx, &arm.pat.kind, &arm.guard)) { - for arm in arms { - check_arm(cx, arm.body, arm.guard.as_ref(), arm.pat, Some(wild_arm.body)); + match IfLetOrMatch::parse(cx, expr) { + Some(IfLetOrMatch::Match(_, arms, _)) => { + if let Some(els_arm) = arms.iter().rfind(|arm| arm_is_wild_like(cx, arm)) { + for arm in arms { + check_arm(cx, true, arm.pat, arm.body, arm.guard.as_ref(), Some(els_arm.body)); + } } } - - if let Some(first_arm) = arms.get(0) { - check_if_let(cx, &first_arm.body, &first_arm.pat); + Some(IfLetOrMatch::IfLet(_, pat, body, els)) => { + check_arm(cx, false, pat, body, None, els); } + None => {} } } } fn check_arm<'tcx>( cx: &LateContext<'tcx>, - outer_block: &'tcx Expr<'tcx>, - outer_guard: Option<&Guard<'tcx>>, + outer_is_match: bool, outer_pat: &'tcx Pat<'tcx>, - wild_outer_block: Option<&'tcx Expr<'tcx>>, + outer_then_body: &'tcx Expr<'tcx>, + outer_guard: Option<&'tcx Guard<'tcx>>, + outer_else_body: Option<&'tcx Expr<'tcx>> ) { - let expr = strip_singleton_blocks(outer_block); + let inner_expr = strip_singleton_blocks(outer_then_body); if_chain! { - if let ExprKind::Match(expr_in, arms_inner, _) = expr.kind; - // the outer arm pattern and the inner match - if expr_in.span.ctxt() == outer_pat.span.ctxt(); - // there must be no more than two arms in the inner match for this lint - if arms_inner.len() == 2; - // no if guards on the inner match - if arms_inner.iter().all(|arm| arm.guard.is_none()); + if let Some(inner) = IfLetOrMatch::parse(cx, inner_expr); + if let Some((inner_scrutinee, inner_then_pat, inner_else_body)) = match inner { + IfLetOrMatch::IfLet(scrutinee, pat, _, els) => Some((scrutinee, pat, els)), + IfLetOrMatch::Match(scrutinee, arms, ..) => if_chain! { + // if there are more than two arms, collapsing would be non-trivial + if arms.len() == 2 && arms.iter().all(|a| a.guard.is_none()); + // one of the arms must be "wild-like" + if let Some(wild_idx) = arms.iter().rposition(|a| arm_is_wild_like(cx, a)); + then { + let (then, els) = (&arms[1 - wild_idx], &arms[wild_idx]); + Some((scrutinee, then.pat, Some(els.body))) + } else { + None + } + }, + }; + if outer_pat.span.ctxt() == inner_scrutinee.span.ctxt(); // match expression must be a local binding // match { .. } - if let Some(binding_id) = path_to_local(peel_ref_operators(cx, expr_in)); - // one of the branches must be "wild-like" - if let Some(wild_inner_arm_idx) = arms_inner.iter().rposition(|arm_inner| is_wild_like(cx, &arm_inner.pat.kind, &arm_inner.guard)); - let (wild_inner_arm, non_wild_inner_arm) = - (&arms_inner[wild_inner_arm_idx], &arms_inner[1 - wild_inner_arm_idx]); - if !pat_contains_or(non_wild_inner_arm.pat); + if let Some(binding_id) = path_to_local(peel_ref_operators(cx, inner_scrutinee)); + if !pat_contains_or(inner_then_pat); // the binding must come from the pattern of the containing match arm // .... => match { .. } if let Some(binding_span) = find_pat_binding(outer_pat, binding_id); - // the "wild-like" branches must be equal - if wild_outer_block.map(|el| SpanlessEq::new(cx).eq_expr(wild_inner_arm.body, el)).unwrap_or(true); + // the "else" branches must be equal + if match (outer_else_body, inner_else_body) { + (None, None) => true, + (None, Some(e)) | (Some(e), None) => is_unit_expr(e), + (Some(a), Some(b)) => SpanlessEq::new(cx).eq_expr(a, b), + }; // the binding must not be used in the if guard let mut used_visitor = LocalUsedVisitor::new(cx, binding_id); - if match outer_guard { - None => true, - Some(Guard::If(expr) | Guard::IfLet(_, expr)) => !used_visitor.check_expr(expr), + if outer_guard.map_or(true, |(Guard::If(e) | Guard::IfLet(_, e))| !used_visitor.check_expr(e)); + // ...or anywhere in the inner expression + if match inner { + IfLetOrMatch::IfLet(_, _, body, els) => { + !used_visitor.check_expr(body) && els.map_or(true, |e| !used_visitor.check_expr(e)) + }, + IfLetOrMatch::Match(_, arms, ..) => !arms.iter().any(|arm| used_visitor.check_arm(arm)), }; - // ...or anywhere in the inner match - if !arms_inner.iter().any(|arm| used_visitor.check_arm(arm)); then { - span_lint_and_then( - cx, - COLLAPSIBLE_MATCH, - expr.span, - "unnecessary nested match", - |diag| { - let mut help_span = MultiSpan::from_spans(vec![binding_span, non_wild_inner_arm.pat.span]); - help_span.push_span_label(binding_span, "replace this binding".into()); - help_span.push_span_label(non_wild_inner_arm.pat.span, "with this pattern".into()); - diag.span_help(help_span, "the outer pattern can be modified to include the inner pattern"); - }, + let msg = format!( + "this `{}` can be collapsed into the outer `{}`", + if matches!(inner, IfLetOrMatch::Match(..)) { "match" } else { "if let" }, + if outer_is_match { "match" } else { "if let" }, ); - } - } -} - -fn check_if_let<'tcx>(cx: &LateContext<'tcx>, outer_expr: &'tcx Expr<'tcx>, outer_pat: &'tcx Pat<'tcx>) { - let block_inner = strip_singleton_blocks(outer_expr); - if_chain! { - if let Some(higher::IfLet { if_then: inner_if_then, let_expr: inner_let_expr, let_pat: inner_let_pat, .. }) = higher::IfLet::hir(block_inner); - if let Some(binding_id) = path_to_local(peel_ref_operators(cx, inner_let_expr)); - if let Some(binding_span) = find_pat_binding(outer_pat, binding_id); - let mut used_visitor = LocalUsedVisitor::new(cx, binding_id); - if !used_visitor.check_expr(inner_if_then); - then { span_lint_and_then( cx, COLLAPSIBLE_MATCH, - block_inner.span, - "unnecessary nested `if let` or `match`", + inner_expr.span, + &msg, |diag| { - let mut help_span = MultiSpan::from_spans(vec![binding_span, inner_let_pat.span]); + let mut help_span = MultiSpan::from_spans(vec![binding_span, inner_then_pat.span]); help_span.push_span_label(binding_span, "replace this binding".into()); - help_span.push_span_label(inner_let_pat.span, "with this pattern".into()); + help_span.push_span_label(inner_then_pat.span, "with this pattern".into()); diag.span_help(help_span, "the outer pattern can be modified to include the inner pattern"); }, ); @@ -168,14 +151,30 @@ fn strip_singleton_blocks<'hir>(mut expr: &'hir Expr<'hir>) -> &'hir Expr<'hir> expr } -/// A "wild-like" pattern is wild ("_") or `None`. -/// For this lint to apply, both the outer and inner patterns -/// must have "wild-like" branches that can be combined. -fn is_wild_like(cx: &LateContext<'_>, pat_kind: &PatKind<'_>, arm_guard: &Option>) -> bool { - if arm_guard.is_some() { +enum IfLetOrMatch<'hir> { + Match(&'hir Expr<'hir>, &'hir [Arm<'hir>], MatchSource), + /// scrutinee, pattern, then block, else block + IfLet(&'hir Expr<'hir>, &'hir Pat<'hir>, &'hir Expr<'hir>, Option<&'hir Expr<'hir>>), +} + +impl<'hir> IfLetOrMatch<'hir> { + fn parse(cx: &LateContext<'_>, expr: &Expr<'hir>) -> Option { + match expr.kind { + ExprKind::Match(expr, arms, source) => Some(Self::Match(expr, arms, source)), + _ => higher::IfLet::hir(cx, expr).map(|higher::IfLet { let_expr, let_pat, if_then, if_else }| { + Self::IfLet(let_expr, let_pat, if_then, if_else) + }) + } + } +} + +/// A "wild-like" arm has a wild (`_`) or `None` pattern and no guard. Such arms can be "collapsed" +/// into a single wild arm without any significant loss in semantics or readability. +fn arm_is_wild_like(cx: &LateContext<'_>, arm: &Arm<'_>) -> bool { + if arm.guard.is_some() { return false; } - match pat_kind { + match arm.pat.kind { PatKind::Binding(..) | PatKind::Wild => true, PatKind::Path(ref qpath) => is_lang_ctor(cx, qpath, OptionNone), _ => false, diff --git a/src/tools/clippy/clippy_lints/src/if_let_mutex.rs b/src/tools/clippy/clippy_lints/src/if_let_mutex.rs index e2d3905eacb50..7dad1c31150e2 100644 --- a/src/tools/clippy/clippy_lints/src/if_let_mutex.rs +++ b/src/tools/clippy/clippy_lints/src/if_let_mutex.rs @@ -59,7 +59,7 @@ impl<'tcx> LateLintPass<'tcx> for IfLetMutex { if_then, if_else: Some(if_else), .. - }) = higher::IfLet::hir(expr) + }) = higher::IfLet::hir(cx, expr) { op_visit.visit_expr(let_expr); if op_visit.mutex_lock_called { diff --git a/src/tools/clippy/clippy_lints/src/if_let_some_result.rs b/src/tools/clippy/clippy_lints/src/if_let_some_result.rs index cd813c639dbbf..fb5637fcec183 100644 --- a/src/tools/clippy/clippy_lints/src/if_let_some_result.rs +++ b/src/tools/clippy/clippy_lints/src/if_let_some_result.rs @@ -45,7 +45,7 @@ declare_lint_pass!(OkIfLet => [IF_LET_SOME_RESULT]); impl<'tcx> LateLintPass<'tcx> for OkIfLet { fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { if_chain! { //begin checking variables - if let Some(higher::IfLet { let_pat, let_expr, .. }) = higher::IfLet::hir(expr); + if let Some(higher::IfLet { let_pat, let_expr, .. }) = higher::IfLet::hir(cx, expr); if let ExprKind::MethodCall(_, ok_span, ref result_types, _) = let_expr.kind; //check is expr.ok() has type Result.ok(, _) if let PatKind::TupleStruct(QPath::Resolved(_, ref x), ref y, _) = let_pat.kind; //get operation if method_chain_args(let_expr, &["ok"]).is_some(); //test to see if using ok() methoduse std::marker::Sized; diff --git a/src/tools/clippy/clippy_lints/src/loops/manual_flatten.rs b/src/tools/clippy/clippy_lints/src/loops/manual_flatten.rs index 9f2bc3c7ebae7..5852674da5780 100644 --- a/src/tools/clippy/clippy_lints/src/loops/manual_flatten.rs +++ b/src/tools/clippy/clippy_lints/src/loops/manual_flatten.rs @@ -37,7 +37,7 @@ pub(super) fn check<'tcx>( if_chain! { if let Some(inner_expr) = inner_expr; - if let Some(higher::IfLet { let_pat, let_expr, if_else: None, .. }) = higher::IfLet::hir(inner_expr); + if let Some(higher::IfLet { let_pat, let_expr, if_else: None, .. }) = higher::IfLet::hir(cx, inner_expr); // Ensure match_expr in `if let` statement is the same as the pat from the for-loop if let PatKind::Binding(_, pat_hir_id, _, _) = pat.kind; if path_to_local_id(let_expr, pat_hir_id); diff --git a/src/tools/clippy/clippy_lints/src/loops/while_let_loop.rs b/src/tools/clippy/clippy_lints/src/loops/while_let_loop.rs index 6be410ca8e3cb..d6d3315e0a83e 100644 --- a/src/tools/clippy/clippy_lints/src/loops/while_let_loop.rs +++ b/src/tools/clippy/clippy_lints/src/loops/while_let_loop.rs @@ -17,7 +17,7 @@ pub(super) fn check(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, loop_block: &' let_expr, if_else: Some(if_else), .. - }) = higher::IfLet::hir(inner) + }) = higher::IfLet::hir(cx, inner) { if is_simple_break_expr(if_else) { could_be_while_let(cx, expr, let_pat, let_expr); diff --git a/src/tools/clippy/clippy_lints/src/manual_map.rs b/src/tools/clippy/clippy_lints/src/manual_map.rs index 53d97f775435a..161d884149075 100644 --- a/src/tools/clippy/clippy_lints/src/manual_map.rs +++ b/src/tools/clippy/clippy_lints/src/manual_map.rs @@ -49,7 +49,7 @@ impl LateLintPass<'_> for ManualMap { let_expr, if_then, if_else: Some(if_else), - }) = higher::IfLet::hir(expr) + }) = higher::IfLet::hir(cx, expr) { manage_lint(cx, expr, (&let_pat.kind, if_then), (&PatKind::Wild, if_else), let_expr); } diff --git a/src/tools/clippy/clippy_lints/src/matches.rs b/src/tools/clippy/clippy_lints/src/matches.rs index a183d0c66e8ce..3d0da472ddcbf 100644 --- a/src/tools/clippy/clippy_lints/src/matches.rs +++ b/src/tools/clippy/clippy_lints/src/matches.rs @@ -8,9 +8,9 @@ use clippy_utils::sugg::Sugg; use clippy_utils::ty::{implements_trait, is_type_diagnostic_item, match_type, peel_mid_ty_refs}; use clippy_utils::visitors::LocalUsedVisitor; use clippy_utils::{ - get_parent_expr, in_macro, is_expn_of, is_lang_ctor, is_lint_allowed, is_refutable, is_wild, meets_msrv, msrvs, - path_to_local, path_to_local_id, peel_hir_pat_refs, peel_n_hir_expr_refs, recurse_or_patterns, remove_blocks, - strip_pat_refs, + get_parent_expr, in_macro, is_expn_of, is_lang_ctor, is_lint_allowed, is_refutable, is_unit_expr, is_wild, + meets_msrv, msrvs, path_to_local, path_to_local_id, peel_hir_pat_refs, peel_n_hir_expr_refs, recurse_or_patterns, + remove_blocks, strip_pat_refs, }; use clippy_utils::{paths, search_same, SpanlessEq, SpanlessHash}; use core::array; @@ -634,7 +634,7 @@ impl<'tcx> LateLintPass<'tcx> for Matches { if let ExprKind::Match(ref ex, ref arms, _) = expr.kind { check_match_ref_pats(cx, ex, arms.iter().map(|el| el.pat), expr); } - if let Some(higher::IfLet { let_pat, let_expr, .. }) = higher::IfLet::hir(expr) { + if let Some(higher::IfLet { let_pat, let_expr, .. }) = higher::IfLet::hir(cx, expr) { check_match_ref_pats(cx, let_expr, once(let_pat), expr); } } @@ -1298,7 +1298,7 @@ fn check_match_like_matches<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) let_expr, if_then, if_else: Some(if_else), - }) = higher::IfLet::hir(expr) + }) = higher::IfLet::hir(cx, expr) { return find_matches_sugg( cx, @@ -1672,14 +1672,6 @@ fn type_ranges(ranges: &[SpannedRange]) -> TypedRanges { .collect() } -fn is_unit_expr(expr: &Expr<'_>) -> bool { - match expr.kind { - ExprKind::Tup(v) if v.is_empty() => true, - ExprKind::Block(b, _) if b.stmts.is_empty() && b.expr.is_none() => true, - _ => false, - } -} - // Checks if arm has the form `None => None` fn is_none_arm(cx: &LateContext<'_>, arm: &Arm<'_>) -> bool { matches!(arm.pat.kind, PatKind::Path(ref qpath) if is_lang_ctor(cx, qpath, OptionNone)) @@ -1835,7 +1827,7 @@ mod redundant_pattern_match { let_pat, let_expr, .. - }) = higher::IfLet::ast(cx, expr) + }) = higher::IfLet::hir(cx, expr) { find_sugg_for_if_let(cx, expr, let_pat, let_expr, "if", if_else.is_some()) } diff --git a/src/tools/clippy/clippy_lints/src/option_if_let_else.rs b/src/tools/clippy/clippy_lints/src/option_if_let_else.rs index d0b0bad5eb1cc..eff3d3abff80c 100644 --- a/src/tools/clippy/clippy_lints/src/option_if_let_else.rs +++ b/src/tools/clippy/clippy_lints/src/option_if_let_else.rs @@ -125,7 +125,7 @@ fn format_option_in_sugg(cx: &LateContext<'_>, cond_expr: &Expr<'_>, as_ref: boo fn detect_option_if_let_else<'tcx>(cx: &LateContext<'tcx>, expr: &Expr<'tcx>) -> Option { if_chain! { if !in_macro(expr.span); // Don't lint macros, because it behaves weirdly - if let Some(higher::IfLet { let_pat, let_expr, if_then, if_else: Some(if_else) }) = higher::IfLet::hir(expr); + if let Some(higher::IfLet { let_pat, let_expr, if_then, if_else: Some(if_else) }) = higher::IfLet::hir(cx, expr); if !is_else_clause(cx.tcx, expr); if !is_result_ok(cx, let_expr); // Don't lint on Result::ok because a different lint does it already if let PatKind::TupleStruct(struct_qpath, [inner_pat], _) = &let_pat.kind; diff --git a/src/tools/clippy/clippy_lints/src/question_mark.rs b/src/tools/clippy/clippy_lints/src/question_mark.rs index 91085c13ac4a4..7b6a0894e6d2f 100644 --- a/src/tools/clippy/clippy_lints/src/question_mark.rs +++ b/src/tools/clippy/clippy_lints/src/question_mark.rs @@ -97,7 +97,7 @@ impl QuestionMark { fn check_if_let_some_and_early_return_none(cx: &LateContext<'_>, expr: &Expr<'_>) { if_chain! { - if let Some(higher::IfLet { let_pat, let_expr, if_then, if_else: Some(if_else) }) = higher::IfLet::hir(expr); + if let Some(higher::IfLet { let_pat, let_expr, if_then, if_else: Some(if_else) }) = higher::IfLet::hir(cx, expr); if Self::is_option(cx, let_expr); if let PatKind::TupleStruct(ref path1, fields, None) = let_pat.kind; diff --git a/src/tools/clippy/clippy_utils/src/higher.rs b/src/tools/clippy/clippy_utils/src/higher.rs index 29b698e56e3c0..957ec35be6f9b 100644 --- a/src/tools/clippy/clippy_utils/src/higher.rs +++ b/src/tools/clippy/clippy_utils/src/higher.rs @@ -79,15 +79,7 @@ pub struct IfLet<'hir> { } impl<'hir> IfLet<'hir> { - #[inline] - pub fn ast(cx: &LateContext<'tcx>, expr: &Expr<'hir>) -> Option { - let rslt = Self::hir(expr)?; - Self::is_not_within_while_context(cx, expr)?; - Some(rslt) - } - - #[inline] - pub const fn hir(expr: &Expr<'hir>) -> Option { + pub fn hir(cx: &LateContext<'_>, expr: &Expr<'hir>) -> Option { if let ExprKind::If( Expr { kind: ExprKind::Let(let_pat, let_expr, _), @@ -97,6 +89,14 @@ impl<'hir> IfLet<'hir> { if_else, ) = expr.kind { + let hir = cx.tcx.hir(); + let mut iter = hir.parent_iter(expr.hir_id); + if let Some((_, Node::Block(Block { stmts: [], .. }))) = iter.next() { + if let Some((_, Node::Expr(Expr { kind: ExprKind::Loop(_, _, LoopSource::While, _), .. }))) = iter.next() { + // while loop desugar + return None; + } + } return Some(Self { let_pat, let_expr, @@ -106,22 +106,6 @@ impl<'hir> IfLet<'hir> { } None } - - #[inline] - fn is_not_within_while_context(cx: &LateContext<'tcx>, expr: &Expr<'hir>) -> Option<()> { - let hir = cx.tcx.hir(); - let parent = hir.get_parent_node(expr.hir_id); - let parent_parent = hir.get_parent_node(parent); - let parent_parent_node = hir.get(parent_parent); - if let Node::Expr(Expr { - kind: ExprKind::Loop(_, _, LoopSource::While, _), - .. - }) = parent_parent_node - { - return None; - } - Some(()) - } } pub struct IfOrIfLet<'hir> { diff --git a/src/tools/clippy/clippy_utils/src/lib.rs b/src/tools/clippy/clippy_utils/src/lib.rs index 82bfce8fe789e..7d7c3b8846c3f 100644 --- a/src/tools/clippy/clippy_utils/src/lib.rs +++ b/src/tools/clippy/clippy_utils/src/lib.rs @@ -254,6 +254,10 @@ pub fn in_macro(span: Span) -> bool { } } +pub fn is_unit_expr(expr: &Expr<'_>) -> bool { + matches!(expr.kind, ExprKind::Block(Block { stmts: [], expr: None, .. }, _) | ExprKind::Tup([])) +} + /// Checks if given pattern is a wildcard (`_`) pub fn is_wild(pat: &Pat<'_>) -> bool { matches!(pat.kind, PatKind::Wild) diff --git a/src/tools/clippy/tests/ui/collapsible_match.rs b/src/tools/clippy/tests/ui/collapsible_match.rs index 55467cf4229de..4ce365cc7649a 100644 --- a/src/tools/clippy/tests/ui/collapsible_match.rs +++ b/src/tools/clippy/tests/ui/collapsible_match.rs @@ -98,6 +98,11 @@ fn lint_cases(opt_opt: Option>, res_opt: Result, String> } fn negative_cases(res_opt: Result, String>, res_res: Result, String>) { + while let Some(x) = make() { + if let Some(1) = x { + todo!(); + } + } // no wild pattern in outer match match res_opt { Ok(val) => match val { diff --git a/src/tools/clippy/tests/ui/collapsible_match.stderr b/src/tools/clippy/tests/ui/collapsible_match.stderr index f96917f583344..c119570e8abd8 100644 --- a/src/tools/clippy/tests/ui/collapsible_match.stderr +++ b/src/tools/clippy/tests/ui/collapsible_match.stderr @@ -1,4 +1,4 @@ -error: unnecessary nested match +error: this `match` can be collapsed into the outer `match` --> $DIR/collapsible_match.rs:7:20 | LL | Ok(val) => match val { @@ -17,7 +17,7 @@ LL | Ok(val) => match val { LL | Some(n) => foo(n), | ^^^^^^^ with this pattern -error: unnecessary nested match +error: this `match` can be collapsed into the outer `match` --> $DIR/collapsible_match.rs:16:20 | LL | Ok(val) => match val { @@ -35,7 +35,7 @@ LL | Ok(val) => match val { LL | Some(n) => foo(n), | ^^^^^^^ with this pattern -error: unnecessary nested `if let` or `match` +error: this `if let` can be collapsed into the outer `if let` --> $DIR/collapsible_match.rs:25:9 | LL | / if let Some(n) = val { @@ -51,7 +51,7 @@ LL | if let Ok(val) = res_opt { LL | if let Some(n) = val { | ^^^^^^^ with this pattern -error: unnecessary nested `if let` or `match` +error: this `if let` can be collapsed into the outer `if let` --> $DIR/collapsible_match.rs:32:9 | LL | / if let Some(n) = val { @@ -69,7 +69,7 @@ LL | if let Ok(val) = res_opt { LL | if let Some(n) = val { | ^^^^^^^ with this pattern -error: unnecessary nested match +error: this `match` can be collapsed into the outer `if let` --> $DIR/collapsible_match.rs:43:9 | LL | / match val { @@ -87,7 +87,7 @@ LL | match val { LL | Some(n) => foo(n), | ^^^^^^^ with this pattern -error: unnecessary nested `if let` or `match` +error: this `if let` can be collapsed into the outer `match` --> $DIR/collapsible_match.rs:52:13 | LL | / if let Some(n) = val { @@ -103,7 +103,7 @@ LL | Ok(val) => { LL | if let Some(n) = val { | ^^^^^^^ with this pattern -error: unnecessary nested match +error: this `match` can be collapsed into the outer `if let` --> $DIR/collapsible_match.rs:61:9 | LL | / match val { @@ -121,7 +121,7 @@ LL | match val { LL | Some(n) => foo(n), | ^^^^^^^ with this pattern -error: unnecessary nested `if let` or `match` +error: this `if let` can be collapsed into the outer `match` --> $DIR/collapsible_match.rs:72:13 | LL | / if let Some(n) = val { @@ -139,7 +139,7 @@ LL | Ok(val) => { LL | if let Some(n) = val { | ^^^^^^^ with this pattern -error: unnecessary nested match +error: this `match` can be collapsed into the outer `match` --> $DIR/collapsible_match.rs:83:20 | LL | Ok(val) => match val { @@ -157,7 +157,7 @@ LL | Ok(val) => match val { LL | Some(n) => foo(n), | ^^^^^^^ with this pattern -error: unnecessary nested match +error: this `match` can be collapsed into the outer `match` --> $DIR/collapsible_match.rs:92:22 | LL | Some(val) => match val { diff --git a/src/tools/clippy/tests/ui/collapsible_match2.stderr b/src/tools/clippy/tests/ui/collapsible_match2.stderr index 8975b2efbaeeb..55e70dce208a0 100644 --- a/src/tools/clippy/tests/ui/collapsible_match2.stderr +++ b/src/tools/clippy/tests/ui/collapsible_match2.stderr @@ -1,4 +1,4 @@ -error: unnecessary nested match +error: this `match` can be collapsed into the outer `match` --> $DIR/collapsible_match2.rs:13:34 | LL | Ok(val) if make() => match val { @@ -17,7 +17,7 @@ LL | Ok(val) if make() => match val { LL | Some(n) => foo(n), | ^^^^^^^ with this pattern -error: unnecessary nested match +error: this `match` can be collapsed into the outer `match` --> $DIR/collapsible_match2.rs:20:24 | LL | Ok(val) => match val { @@ -35,7 +35,7 @@ LL | Ok(val) => match val { LL | Some(n) => foo(n), | ^^^^^^^ with this pattern -error: unnecessary nested match +error: this `match` can be collapsed into the outer `match` --> $DIR/collapsible_match2.rs:34:29 | LL | $pat => match $e { @@ -57,7 +57,7 @@ LL | mac!(res_opt => Ok(val), val => Some(n), foo(n)); | replace this binding = note: this error originates in the macro `mac` (in Nightly builds, run with -Z macro-backtrace for more info) -error: unnecessary nested match +error: this `match` can be collapsed into the outer `match` --> $DIR/collapsible_match2.rs:51:20 | LL | Some(s) => match *s { @@ -75,7 +75,7 @@ LL | Some(s) => match *s { LL | [n] => foo(n), | ^^^ with this pattern -error: unnecessary nested match +error: this `match` can be collapsed into the outer `match` --> $DIR/collapsible_match2.rs:60:24 | LL | Some(ref s) => match &*s { diff --git a/src/tools/clippy/tests/ui/if_let_some_result.fixed b/src/tools/clippy/tests/ui/if_let_some_result.fixed index 62a25ce2d128c..1bddc47721e5a 100644 --- a/src/tools/clippy/tests/ui/if_let_some_result.fixed +++ b/src/tools/clippy/tests/ui/if_let_some_result.fixed @@ -1,6 +1,7 @@ // run-rustfix #![warn(clippy::if_let_some_result)] +#![allow(dead_code)] fn str_to_int(x: &str) -> i32 { if let Ok(y) = x.parse() { y } else { 0 } @@ -20,8 +21,8 @@ fn strange_some_no_else(x: &str) -> i32 { } } -fn main() { - let _ = str_to_int("1"); - let _ = str_to_int_ok("2"); - let _ = strange_some_no_else("3"); +fn negative() { + while let Some(1) = "".parse().ok() {} } + +fn main() {} diff --git a/src/tools/clippy/tests/ui/if_let_some_result.rs b/src/tools/clippy/tests/ui/if_let_some_result.rs index 234ff5e9e80e2..d4a52ec9881d7 100644 --- a/src/tools/clippy/tests/ui/if_let_some_result.rs +++ b/src/tools/clippy/tests/ui/if_let_some_result.rs @@ -1,6 +1,7 @@ // run-rustfix #![warn(clippy::if_let_some_result)] +#![allow(dead_code)] fn str_to_int(x: &str) -> i32 { if let Some(y) = x.parse().ok() { y } else { 0 } @@ -20,8 +21,8 @@ fn strange_some_no_else(x: &str) -> i32 { } } -fn main() { - let _ = str_to_int("1"); - let _ = str_to_int_ok("2"); - let _ = strange_some_no_else("3"); +fn negative() { + while let Some(1) = "".parse().ok() {} } + +fn main() {} diff --git a/src/tools/clippy/tests/ui/if_let_some_result.stderr b/src/tools/clippy/tests/ui/if_let_some_result.stderr index 134ce9d24116b..bc3a5e7698d74 100644 --- a/src/tools/clippy/tests/ui/if_let_some_result.stderr +++ b/src/tools/clippy/tests/ui/if_let_some_result.stderr @@ -1,5 +1,5 @@ error: matching on `Some` with `ok()` is redundant - --> $DIR/if_let_some_result.rs:6:5 + --> $DIR/if_let_some_result.rs:7:5 | LL | if let Some(y) = x.parse().ok() { y } else { 0 } | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -11,7 +11,7 @@ LL | if let Ok(y) = x.parse() { y } else { 0 } | ~~~~~~~~~~~~~~~~~~~~~~~~ error: matching on `Some` with `ok()` is redundant - --> $DIR/if_let_some_result.rs:16:9 + --> $DIR/if_let_some_result.rs:17:9 | LL | if let Some(y) = x . parse() . ok () { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^