diff options
| author | bors <bors@rust-lang.org> | 2022-03-16 04:05:35 +0000 |
|---|---|---|
| committer | bors <bors@rust-lang.org> | 2022-03-16 04:05:35 +0000 |
| commit | a2af9cf1cf6ccb195eae40cdd793939bc77e7e73 (patch) | |
| tree | 85776d35bca2bd513ce46e8c0ad1c7c7d17f5588 /compiler | |
| parent | af446e1d7086d4aeed495f6b03e70009e9424ce4 (diff) | |
| parent | aaf2255379c22f93e53c5fad14453ac7a791ae9e (diff) | |
Auto merge of #94987 - Dylan-DPC:rollup-5tssuhi, r=Dylan-DPC
Rollup of 5 pull requests Successful merges: - #94868 (Format core and std macro rules, removing needless surrounding blocks) - #94951 (Extend the irrefutable_let_patterns lint to let chains) - #94955 (Refactor: Use `format_args_capture` in some parts of `rustc_parse`) - #94957 (Improve the explanation about the behaviour of read_line) - #94974 (Ensure that `let_else` does not interact with `let_chains`) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
Diffstat (limited to 'compiler')
| -rw-r--r-- | compiler/rustc_mir_build/src/thir/pattern/check_match.rs | 215 | ||||
| -rw-r--r-- | compiler/rustc_parse/src/parser/attr.rs | 4 | ||||
| -rw-r--r-- | compiler/rustc_parse/src/parser/diagnostics.rs | 28 | ||||
| -rw-r--r-- | compiler/rustc_parse/src/parser/expr.rs | 36 | ||||
| -rw-r--r-- | compiler/rustc_parse/src/parser/item.rs | 53 |
5 files changed, 240 insertions, 96 deletions
diff --git a/compiler/rustc_mir_build/src/thir/pattern/check_match.rs b/compiler/rustc_mir_build/src/thir/pattern/check_match.rs index e2c56df6014..8a3a46c1190 100644 --- a/compiler/rustc_mir_build/src/thir/pattern/check_match.rs +++ b/compiler/rustc_mir_build/src/thir/pattern/check_match.rs @@ -158,7 +158,7 @@ impl<'p, 'tcx> MatchVisitor<'_, 'p, 'tcx> { self.check_patterns(pat, Refutable); let mut cx = self.new_cx(scrutinee.hir_id); let tpat = self.lower_pattern(&mut cx, pat, &mut false); - check_let_reachability(&mut cx, pat.hir_id, tpat, span); + self.check_let_reachability(&mut cx, pat.hir_id, tpat, span); } fn check_match( @@ -176,7 +176,7 @@ impl<'p, 'tcx> MatchVisitor<'_, 'p, 'tcx> { if let Some(hir::Guard::IfLet(ref pat, _)) = arm.guard { self.check_patterns(pat, Refutable); let tpat = self.lower_pattern(&mut cx, pat, &mut false); - check_let_reachability(&mut cx, pat.hir_id, tpat, tpat.span()); + self.check_let_reachability(&mut cx, pat.hir_id, tpat, tpat.span()); } } @@ -224,6 +224,157 @@ impl<'p, 'tcx> MatchVisitor<'_, 'p, 'tcx> { } } + fn check_let_reachability( + &mut self, + cx: &mut MatchCheckCtxt<'p, 'tcx>, + pat_id: HirId, + pat: &'p DeconstructedPat<'p, 'tcx>, + span: Span, + ) { + if self.check_let_chain(cx, pat_id) { + return; + } + + if is_let_irrefutable(cx, pat_id, pat) { + irrefutable_let_pattern(cx.tcx, pat_id, span); + } + } + + fn check_let_chain(&mut self, cx: &mut MatchCheckCtxt<'p, 'tcx>, pat_id: HirId) -> bool { + let hir = self.tcx.hir(); + let parent = hir.get_parent_node(pat_id); + + // First, figure out if the given pattern is part of a let chain, + // and if so, obtain the top node of the chain. + let mut top = parent; + let mut part_of_chain = false; + loop { + let new_top = hir.get_parent_node(top); + if let hir::Node::Expr( + hir::Expr { + kind: hir::ExprKind::Binary(Spanned { node: hir::BinOpKind::And, .. }, lhs, rhs), + .. + }, + .., + ) = hir.get(new_top) + { + // If this isn't the first iteration, we need to check + // if there is a let expr before us in the chain, so + // that we avoid doubly checking the let chain. + + // The way a chain of &&s is encoded is ((let ... && let ...) && let ...) && let ... + // as && is left-to-right associative. Thus, we need to check rhs. + if part_of_chain && matches!(rhs.kind, hir::ExprKind::Let(..)) { + return true; + } + // If there is a let at the lhs, and we provide the rhs, we don't do any checking either. + if !part_of_chain && matches!(lhs.kind, hir::ExprKind::Let(..)) && rhs.hir_id == top + { + return true; + } + } else { + // We've reached the top. + break; + } + + // Since this function is called within a let context, it is reasonable to assume that any parent + // `&&` infers a let chain + part_of_chain = true; + top = new_top; + } + if !part_of_chain { + return false; + } + + // Second, obtain the refutabilities of all exprs in the chain, + // and record chain members that aren't let exprs. + let mut chain_refutabilities = Vec::new(); + let hir::Node::Expr(top_expr) = hir.get(top) else { + // We ensure right above that it's an Expr + unreachable!() + }; + let mut cur_expr = top_expr; + loop { + let mut add = |expr: &hir::Expr<'tcx>| { + let refutability = match expr.kind { + hir::ExprKind::Let(hir::Let { pat, init, span, .. }) => { + let mut ncx = self.new_cx(init.hir_id); + let tpat = self.lower_pattern(&mut ncx, pat, &mut false); + + let refutable = !is_let_irrefutable(&mut ncx, pat.hir_id, tpat); + Some((*span, refutable)) + } + _ => None, + }; + chain_refutabilities.push(refutability); + }; + if let hir::Expr { + kind: hir::ExprKind::Binary(Spanned { node: hir::BinOpKind::And, .. }, lhs, rhs), + .. + } = cur_expr + { + add(rhs); + cur_expr = lhs; + } else { + add(cur_expr); + break; + } + } + chain_refutabilities.reverse(); + + // Third, emit the actual warnings. + + if chain_refutabilities.iter().all(|r| matches!(*r, Some((_, false)))) { + // The entire chain is made up of irrefutable `let` statements + let let_source = let_source_parent(self.tcx, top, None); + irrefutable_let_patterns( + cx.tcx, + top, + let_source, + chain_refutabilities.len(), + top_expr.span, + ); + return true; + } + let lint_affix = |affix: &[Option<(Span, bool)>], kind, suggestion| { + let span_start = affix[0].unwrap().0; + let span_end = affix.last().unwrap().unwrap().0; + let span = span_start.to(span_end); + let cnt = affix.len(); + cx.tcx.struct_span_lint_hir(IRREFUTABLE_LET_PATTERNS, top, span, |lint| { + let s = pluralize!(cnt); + let mut diag = lint.build(&format!("{kind} irrefutable pattern{s} in let chain")); + diag.note(&format!( + "{these} pattern{s} will always match", + these = pluralize!("this", cnt), + )); + diag.help(&format!( + "consider moving {} {suggestion}", + if cnt > 1 { "them" } else { "it" } + )); + diag.emit() + }); + }; + if let Some(until) = chain_refutabilities.iter().position(|r| !matches!(*r, Some((_, false)))) && until > 0 { + // The chain has a non-zero prefix of irrefutable `let` statements. + + // Check if the let source is while, for there is no alternative place to put a prefix, + // and we shouldn't lint. + let let_source = let_source_parent(self.tcx, top, None); + if !matches!(let_source, LetSource::WhileLet) { + // Emit the lint + let prefix = &chain_refutabilities[..until]; + lint_affix(prefix, "leading", "outside of the construct"); + } + } + if let Some(from) = chain_refutabilities.iter().rposition(|r| !matches!(*r, Some((_, false)))) && from != (chain_refutabilities.len() - 1) { + // The chain has a non-empty suffix of irrefutable `let` statements + let suffix = &chain_refutabilities[from + 1..]; + lint_affix(suffix, "trailing", "into the body"); + } + true + } + fn check_irrefutable(&self, pat: &'tcx Pat<'tcx>, origin: &str, sp: Option<Span>) { let mut cx = self.new_cx(pat.hir_id); @@ -453,6 +604,17 @@ fn unreachable_pattern(tcx: TyCtxt<'_>, span: Span, id: HirId, catchall: Option< } fn irrefutable_let_pattern(tcx: TyCtxt<'_>, id: HirId, span: Span) { + let source = let_source(tcx, id); + irrefutable_let_patterns(tcx, id, source, 1, span); +} + +fn irrefutable_let_patterns( + tcx: TyCtxt<'_>, + id: HirId, + source: LetSource, + count: usize, + span: Span, +) { macro_rules! emit_diag { ( $lint:expr, @@ -460,14 +622,15 @@ fn irrefutable_let_pattern(tcx: TyCtxt<'_>, id: HirId, span: Span) { $note_sufix:expr, $help_sufix:expr ) => {{ - let mut diag = $lint.build(concat!("irrefutable ", $source_name, " pattern")); - diag.note(concat!("this pattern will always match, so the ", $note_sufix)); + let s = pluralize!(count); + let these = pluralize!("this", count); + let mut diag = $lint.build(&format!("irrefutable {} pattern{s}", $source_name)); + diag.note(&format!("{these} pattern{s} will always match, so the {}", $note_sufix)); diag.help(concat!("consider ", $help_sufix)); diag.emit() }}; } - let source = let_source(tcx, id); let span = match source { LetSource::LetElse(span) => span, _ => span, @@ -511,16 +674,11 @@ fn irrefutable_let_pattern(tcx: TyCtxt<'_>, id: HirId, span: Span) { }); } -fn check_let_reachability<'p, 'tcx>( +fn is_let_irrefutable<'p, 'tcx>( cx: &mut MatchCheckCtxt<'p, 'tcx>, pat_id: HirId, pat: &'p DeconstructedPat<'p, 'tcx>, - span: Span, -) { - if is_let_chain(cx.tcx, pat_id) { - return; - } - +) -> bool { let arms = [MatchArm { pat, hir_id: pat_id, has_guard: false }]; let report = compute_match_usefulness(&cx, &arms, pat_id, pat.ty()); @@ -529,10 +687,9 @@ fn check_let_reachability<'p, 'tcx>( // `is_uninhabited` check. report_arm_reachability(&cx, &report); - if report.non_exhaustiveness_witnesses.is_empty() { - // The match is exhaustive, i.e. the `if let` pattern is irrefutable. - irrefutable_let_pattern(cx.tcx, pat_id, span); - } + // If the list of witnesses is empty, the match is exhaustive, + // i.e. the `if let` pattern is irrefutable. + report.non_exhaustiveness_witnesses.is_empty() } /// Report unreachable arms, if any. @@ -941,13 +1098,19 @@ fn let_source(tcx: TyCtxt<'_>, pat_id: HirId) -> LetSource { let hir = tcx.hir(); let parent = hir.get_parent_node(pat_id); + let_source_parent(tcx, parent, Some(pat_id)) +} + +fn let_source_parent(tcx: TyCtxt<'_>, parent: HirId, pat_id: Option<HirId>) -> LetSource { + let hir = tcx.hir(); + let parent_node = hir.get(parent); match parent_node { hir::Node::Arm(hir::Arm { guard: Some(hir::Guard::IfLet(&hir::Pat { hir_id, .. }, _)), .. - }) if hir_id == pat_id => { + }) if Some(hir_id) == pat_id => { return LetSource::IfLetGuard; } hir::Node::Expr(hir::Expr { kind: hir::ExprKind::Let(..), span, .. }) => { @@ -980,21 +1143,3 @@ fn let_source(tcx: TyCtxt<'_>, pat_id: HirId) -> LetSource { LetSource::GenericLet } - -// Since this function is called within a let context, it is reasonable to assume that any parent -// `&&` infers a let chain -fn is_let_chain(tcx: TyCtxt<'_>, pat_id: HirId) -> bool { - let hir = tcx.hir(); - let parent = hir.get_parent_node(pat_id); - let parent_parent = hir.get_parent_node(parent); - matches!( - hir.get(parent_parent), - hir::Node::Expr( - hir::Expr { - kind: hir::ExprKind::Binary(Spanned { node: hir::BinOpKind::And, .. }, ..), - .. - }, - .. - ) - ) -} diff --git a/compiler/rustc_parse/src/parser/attr.rs b/compiler/rustc_parse/src/parser/attr.rs index 379e47077ea..8f759ae84fa 100644 --- a/compiler/rustc_parse/src/parser/attr.rs +++ b/compiler/rustc_parse/src/parser/attr.rs @@ -139,7 +139,7 @@ impl<'a> Parser<'a> { Ok(attr::mk_attr_from_item(item, None, style, attr_sp)) } else { let token_str = pprust::token_to_string(&this.token); - let msg = &format!("expected `#`, found `{}`", token_str); + let msg = &format!("expected `#`, found `{token_str}`"); Err(this.struct_span_err(this.token.span, msg)) } }) @@ -421,7 +421,7 @@ impl<'a> Parser<'a> { } let found = pprust::token_to_string(&self.token); - let msg = format!("expected unsuffixed literal or identifier, found `{}`", found); + let msg = format!("expected unsuffixed literal or identifier, found `{found}`"); Err(self.struct_span_err(self.token.span, &msg)) } } diff --git a/compiler/rustc_parse/src/parser/diagnostics.rs b/compiler/rustc_parse/src/parser/diagnostics.rs index 21d5bec65f0..1909e9ee749 100644 --- a/compiler/rustc_parse/src/parser/diagnostics.rs +++ b/compiler/rustc_parse/src/parser/diagnostics.rs @@ -327,8 +327,8 @@ impl<'a> Parser<'a> { expect.clone() }; ( - format!("expected one of {}, found {}", expect, actual), - (self.prev_token.span.shrink_to_hi(), format!("expected one of {}", short_expect)), + format!("expected one of {expect}, found {actual}"), + (self.prev_token.span.shrink_to_hi(), format!("expected one of {short_expect}")), ) } else if expected.is_empty() { ( @@ -337,8 +337,8 @@ impl<'a> Parser<'a> { ) } else { ( - format!("expected {}, found {}", expect, actual), - (self.prev_token.span.shrink_to_hi(), format!("expected {}", expect)), + format!("expected {expect}, found {actual}"), + (self.prev_token.span.shrink_to_hi(), format!("expected {expect}")), ) }; self.last_unexpected_token_span = Some(self.token.span); @@ -421,7 +421,7 @@ impl<'a> Parser<'a> { String::new(), Applicability::MachineApplicable, ); - err.note(&format!("the raw string started with {} `#`s", n_hashes)); + err.note(&format!("the raw string started with {n_hashes} `#`s")); true } _ => false, @@ -1191,7 +1191,7 @@ impl<'a> Parser<'a> { _ => None, }; if let Some(name) = previous_item_kind_name { - err.help(&format!("{} declarations are not followed by a semicolon", name)); + err.help(&format!("{name} declarations are not followed by a semicolon")); } } err.emit(); @@ -1226,12 +1226,12 @@ impl<'a> Parser<'a> { "expected `{}`, found {}", token_str, match (&self.token.kind, self.subparser_name) { - (token::Eof, Some(origin)) => format!("end of {}", origin), + (token::Eof, Some(origin)) => format!("end of {origin}"), _ => this_token_str, }, ); let mut err = self.struct_span_err(sp, &msg); - let label_exp = format!("expected `{}`", token_str); + let label_exp = format!("expected `{token_str}`"); match self.recover_closing_delimiter(&[t.clone()], err) { Err(e) => err = e, Ok(recovered) => { @@ -1368,7 +1368,7 @@ impl<'a> Parser<'a> { Applicability::MachineApplicable, ); } - err.span_suggestion(lo.shrink_to_lo(), &format!("{}you can still access the deprecated `try!()` macro using the \"raw identifier\" syntax", prefix), "r#".to_string(), Applicability::MachineApplicable); + err.span_suggestion(lo.shrink_to_lo(), &format!("{prefix}you can still access the deprecated `try!()` macro using the \"raw identifier\" syntax"), "r#".to_string(), Applicability::MachineApplicable); err.emit(); Ok(self.mk_expr_err(lo.to(hi))) } else { @@ -1504,7 +1504,7 @@ impl<'a> Parser<'a> { delim.retain(|c| c != '`'); err.span_suggestion_short( self.prev_token.span.shrink_to_hi(), - &format!("`{}` may belong here", delim), + &format!("`{delim}` may belong here"), delim, Applicability::MaybeIncorrect, ); @@ -1698,7 +1698,7 @@ impl<'a> Parser<'a> { ( ident, "self: ".to_string(), - format!("{}: &{}TypeName", ident, mutab), + format!("{ident}: &{mutab}TypeName"), "_: ".to_string(), pat.span.shrink_to_lo(), pat.span, @@ -1826,7 +1826,7 @@ impl<'a> Parser<'a> { let (span, msg) = match (&self.token.kind, self.subparser_name) { (&token::Eof, Some(origin)) => { let sp = self.sess.source_map().next_point(self.prev_token.span); - (sp, format!("expected expression, found end of {}", origin)) + (sp, format!("expected expression, found end of {origin}")) } _ => ( self.token.span, @@ -1975,8 +1975,8 @@ impl<'a> Parser<'a> { (ty_generics, self.sess.source_map().span_to_snippet(param.span())) { let (span, sugg) = match &generics.params[..] { - [] => (generics.span, format!("<{}>", snippet)), - [.., generic] => (generic.span().shrink_to_hi(), format!(", {}", snippet)), + [] => (generics.span, format!("<{snippet}>")), + [.., generic] => (generic.span().shrink_to_hi(), format!(", {snippet}")), }; err.multipart_suggestion( "`const` parameters must be declared for the `impl`", diff --git a/compiler/rustc_parse/src/parser/expr.rs b/compiler/rustc_parse/src/parser/expr.rs index b993d48c995..550d79a898c 100644 --- a/compiler/rustc_parse/src/parser/expr.rs +++ b/compiler/rustc_parse/src/parser/expr.rs @@ -223,7 +223,7 @@ impl<'a> Parser<'a> { AssocOp::NotEqual => "!=", _ => unreachable!(), }; - self.struct_span_err(sp, &format!("invalid comparison operator `{}=`", sugg)) + self.struct_span_err(sp, &format!("invalid comparison operator `{sugg}=`")) .span_suggestion_short( sp, &format!("`{s}=` is not a valid comparison operator, use `{s}`", s = sugg), @@ -441,10 +441,10 @@ impl<'a> Parser<'a> { /// Error on `and` and `or` suggesting `&&` and `||` respectively. fn error_bad_logical_op(&self, bad: &str, good: &str, english: &str) { - self.struct_span_err(self.token.span, &format!("`{}` is not a logical operator", bad)) + self.struct_span_err(self.token.span, &format!("`{bad}` is not a logical operator")) .span_suggestion_short( self.token.span, - &format!("use `{}` to perform logical {}", good, english), + &format!("use `{good}` to perform logical {english}"), good.to_string(), Applicability::MachineApplicable, ) @@ -766,9 +766,9 @@ impl<'a> Parser<'a> { self.look_ahead(1, |t| t.span).to(span_after_type), "interpreted as generic arguments", ) - .span_label(self.token.span, format!("not interpreted as {}", op_noun)) + .span_label(self.token.span, format!("not interpreted as {op_noun}")) .multipart_suggestion( - &format!("try {} the cast value", op_verb), + &format!("try {op_verb} the cast value"), vec![ (expr.span.shrink_to_lo(), "(".to_string()), (expr.span.shrink_to_hi(), ")".to_string()), @@ -970,7 +970,7 @@ impl<'a> Parser<'a> { fn error_unexpected_after_dot(&self) { // FIXME Could factor this out into non_fatal_unexpected or something. let actual = pprust::token_to_string(&self.token); - self.struct_span_err(self.token.span, &format!("unexpected token: `{}`", actual)).emit(); + self.struct_span_err(self.token.span, &format!("unexpected token: `{actual}`")).emit(); } // We need an identifier or integer, but the next token is a float. @@ -1151,7 +1151,7 @@ impl<'a> Parser<'a> { mem::replace(err, replacement_err).cancel(); err.multipart_suggestion( - &format!("if `{}` is a struct, use braces as delimiters", name), + &format!("if `{name}` is a struct, use braces as delimiters"), vec![ (open_paren, " { ".to_string()), (close_paren, " }".to_string()), @@ -1159,7 +1159,7 @@ impl<'a> Parser<'a> { Applicability::MaybeIncorrect, ); err.multipart_suggestion( - &format!("if `{}` is a function, use the arguments directly", name), + &format!("if `{name}` is a function, use the arguments directly"), fields .into_iter() .map(|field| (field.span.until(field.expr.span), String::new())) @@ -1776,9 +1776,9 @@ impl<'a> Parser<'a> { ) .emit(); } else { - let msg = format!("invalid suffix `{}` for number literal", suf); + let msg = format!("invalid suffix `{suf}` for number literal"); self.struct_span_err(span, &msg) - .span_label(span, format!("invalid suffix `{}`", suf)) + .span_label(span, format!("invalid suffix `{suf}`")) .help("the suffix must be one of the numeric types (`u32`, `isize`, `f32`, etc.)") .emit(); } @@ -1791,9 +1791,9 @@ impl<'a> Parser<'a> { let msg = format!("invalid width `{}` for float literal", &suf[1..]); self.struct_span_err(span, &msg).help("valid widths are 32 and 64").emit(); } else { - let msg = format!("invalid suffix `{}` for float literal", suf); + let msg = format!("invalid suffix `{suf}` for float literal"); self.struct_span_err(span, &msg) - .span_label(span, format!("invalid suffix `{}`", suf)) + .span_label(span, format!("invalid suffix `{suf}`")) .help("valid suffixes are `f32` and `f64`") .emit(); } @@ -1805,7 +1805,7 @@ impl<'a> Parser<'a> { 2 => "binary", _ => unreachable!(), }; - self.struct_span_err(span, &format!("{} float literal is not supported", descr)) + self.struct_span_err(span, &format!("{descr} float literal is not supported")) .span_label(span, "not supported") .emit(); } @@ -1825,7 +1825,7 @@ impl<'a> Parser<'a> { let mut err = self .sess .span_diagnostic - .struct_span_warn(sp, &format!("suffixes on {} are invalid", kind)); + .struct_span_warn(sp, &format!("suffixes on {kind} are invalid")); err.note(&format!( "`{}` is *temporarily* accepted on tuple index fields as it was \ incorrectly accepted on stable for a few releases", @@ -1842,10 +1842,10 @@ impl<'a> Parser<'a> { ); err } else { - self.struct_span_err(sp, &format!("suffixes on {} are invalid", kind)) + self.struct_span_err(sp, &format!("suffixes on {kind} are invalid")) .forget_guarantee() }; - err.span_label(sp, format!("invalid suffix `{}`", suf)); + err.span_label(sp, format!("invalid suffix `{suf}`")); err.emit(); } } @@ -2211,7 +2211,7 @@ impl<'a> Parser<'a> { let ctx = if is_ctx_else { "else" } else { "if" }; self.struct_span_err(last, "outer attributes are not allowed on `if` and `else` branches") .span_label(branch_span, "the attributes are attached to this branch") - .span_label(ctx_span, format!("the branch belongs to this `{}`", ctx)) + .span_label(ctx_span, format!("the branch belongs to this `{ctx}`")) .span_suggestion( span, "remove the attributes", @@ -2391,7 +2391,7 @@ impl<'a> Parser<'a> { err.span_label(arrow_span, "while parsing the `match` arm starting here"); if stmts.len() > 1 { err.multipart_suggestion( - &format!("surround the statement{} with a body", s), + &format!("surround the statement{s} with a body"), vec