Skip to content

Commit

Permalink
Auto merge of rust-lang#94987 - Dylan-DPC:rollup-5tssuhi, r=Dylan-DPC
Browse files Browse the repository at this point in the history
Rollup of 5 pull requests

Successful merges:

 - rust-lang#94868 (Format core and std macro rules, removing needless surrounding blocks)
 - rust-lang#94951 (Extend the irrefutable_let_patterns lint to let chains)
 - rust-lang#94955 (Refactor: Use `format_args_capture` in some parts of `rustc_parse`)
 - rust-lang#94957 (Improve the explanation about the behaviour of read_line)
 - rust-lang#94974 (Ensure that `let_else` does not interact with `let_chains`)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
  • Loading branch information
bors committed Mar 16, 2022
2 parents af446e1 + aaf2255 commit a2af9cf
Show file tree
Hide file tree
Showing 19 changed files with 650 additions and 151 deletions.
215 changes: 180 additions & 35 deletions compiler/rustc_mir_build/src/thir/pattern/check_match.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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());
}
}

Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -453,21 +604,33 @@ 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,
$source_name:expr,
$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,
Expand Down Expand Up @@ -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());

Expand All @@ -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.
Expand Down Expand Up @@ -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, .. }) => {
Expand Down Expand Up @@ -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, .. }, ..),
..
},
..
)
)
}
4 changes: 2 additions & 2 deletions compiler/rustc_parse/src/parser/attr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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))
}
})
Expand Down Expand Up @@ -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))
}
}
Expand Down
Loading

0 comments on commit a2af9cf

Please sign in to comment.