Skip to content

Commit

Permalink
Unrolled build for rust-lang#116715
Browse files Browse the repository at this point in the history
Rollup merge of rust-lang#116715 - Nadrieril:patkind-error, r=oli-obk

Prevent more spurious unreachable pattern lints

Continues the work of rust-lang#115937 by introducing `PatKind::Error`, to be used instead of `PatKind::Wild` when an error was raised during pattern lowering. Most of match checking lints are skipped when a `PatKind::Error` is encountered. This avoids confusing extra warnings when a pattern is malformed. Now `PatKind::Wild` should indicate an actual wildcard pattern.

r? `@oli-obk`
  • Loading branch information
rust-timer authored Oct 14, 2023
2 parents 8de6f99 + 89f75ff commit 5df9381
Show file tree
Hide file tree
Showing 14 changed files with 108 additions and 103 deletions.
31 changes: 26 additions & 5 deletions compiler/rustc_middle/src/thir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,12 @@ use rustc_middle::middle::region;
use rustc_middle::mir::interpret::AllocId;
use rustc_middle::mir::{self, BinOp, BorrowKind, FakeReadCause, Mutability, UnOp};
use rustc_middle::ty::adjustment::PointerCoercion;
use rustc_middle::ty::GenericArgsRef;
use rustc_middle::ty::{self, AdtDef, FnSig, List, Ty, UpvarArgs};
use rustc_middle::ty::{CanonicalUserType, CanonicalUserTypeAnnotation};
use rustc_middle::ty::{
self, AdtDef, CanonicalUserType, CanonicalUserTypeAnnotation, FnSig, GenericArgsRef, List, Ty,
UpvarArgs,
};
use rustc_span::def_id::LocalDefId;
use rustc_span::{sym, Span, Symbol, DUMMY_SP};
use rustc_span::{sym, ErrorGuaranteed, Span, Symbol, DUMMY_SP};
use rustc_target::abi::{FieldIdx, VariantIdx};
use rustc_target::asm::InlineAsmRegOrRegClass;
use std::fmt;
Expand Down Expand Up @@ -632,7 +633,7 @@ impl<'tcx> Pat<'tcx> {

use PatKind::*;
match &self.kind {
Wild | Range(..) | Binding { subpattern: None, .. } | Constant { .. } => {}
Wild | Range(..) | Binding { subpattern: None, .. } | Constant { .. } | Error(_) => {}
AscribeUserType { subpattern, .. }
| Binding { subpattern: Some(subpattern), .. }
| Deref { subpattern } => subpattern.walk_(it),
Expand All @@ -647,6 +648,21 @@ impl<'tcx> Pat<'tcx> {
}
}

/// Whether the pattern has a `PatKind::Error` nested within.
pub fn pat_error_reported(&self) -> Result<(), ErrorGuaranteed> {
let mut error = None;
self.walk(|pat| {
if let PatKind::Error(e) = pat.kind && error.is_none() {
error = Some(e);
}
error.is_none()
});
match error {
None => Ok(()),
Some(e) => Err(e),
}
}

/// Walk the pattern in left-to-right order.
///
/// If you always want to recurse, prefer this method over `walk`.
Expand Down Expand Up @@ -771,6 +787,10 @@ pub enum PatKind<'tcx> {
Or {
pats: Box<[Box<Pat<'tcx>>]>,
},

/// An error has been encountered during lowering. We probably shouldn't report more lints
/// related to this pattern.
Error(ErrorGuaranteed),
}

#[derive(Clone, Debug, PartialEq, HashStable, TypeVisitable)]
Expand Down Expand Up @@ -934,6 +954,7 @@ impl<'tcx> fmt::Display for Pat<'tcx> {
}
Ok(())
}
PatKind::Error(_) => write!(f, "<error>"),
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_middle/src/thir/visit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,7 @@ pub fn walk_pat<'a, 'tcx: 'a, V: Visitor<'a, 'tcx>>(visitor: &mut V, pat: &Pat<'
is_primary: _,
name: _,
} => visitor.visit_pat(&subpattern),
Binding { .. } | Wild => {}
Binding { .. } | Wild | Error(_) => {}
Variant { subpatterns, adt_def: _, args: _, variant_index: _ } | Leaf { subpatterns } => {
for subpattern in subpatterns {
visitor.visit_pat(&subpattern.pattern);
Expand Down
5 changes: 4 additions & 1 deletion compiler/rustc_mir_build/src/build/matches/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -814,7 +814,10 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
}
}

PatKind::Constant { .. } | PatKind::Range { .. } | PatKind::Wild => {}
PatKind::Constant { .. }
| PatKind::Range { .. }
| PatKind::Wild
| PatKind::Error(_) => {}

PatKind::Deref { ref subpattern } => {
self.visit_primary_bindings(subpattern, pattern_user_ty.deref(), f);
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_mir_build/src/build/matches/simplify.rs
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
Ok(())
}

PatKind::Wild => {
PatKind::Wild | PatKind::Error(_) => {
// nothing left to do
Ok(())
}
Expand Down
6 changes: 4 additions & 2 deletions compiler/rustc_mir_build/src/build/matches/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,8 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
| PatKind::Wild
| PatKind::Binding { .. }
| PatKind::Leaf { .. }
| PatKind::Deref { .. } => self.error_simplifiable(match_pair),
| PatKind::Deref { .. }
| PatKind::Error(_) => self.error_simplifiable(match_pair),
}
}

Expand Down Expand Up @@ -111,7 +112,8 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
| PatKind::Binding { .. }
| PatKind::AscribeUserType { .. }
| PatKind::Leaf { .. }
| PatKind::Deref { .. } => {
| PatKind::Deref { .. }
| PatKind::Error(_) => {
// don't know how to add these patterns to a switch
false
}
Expand Down
3 changes: 2 additions & 1 deletion compiler/rustc_mir_build/src/check_unsafety.rs
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,8 @@ impl<'a, 'tcx> Visitor<'a, 'tcx> for UnsafetyVisitor<'a, 'tcx> {
PatKind::Wild |
// these just wrap other patterns
PatKind::Or { .. } |
PatKind::AscribeUserType { .. } => {}
PatKind::AscribeUserType { .. } |
PatKind::Error(_) => {}
}
};

Expand Down
34 changes: 26 additions & 8 deletions compiler/rustc_mir_build/src/thir/pattern/check_match.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ use rustc_hir::HirId;
use rustc_middle::thir::visit::{self, Visitor};
use rustc_middle::thir::*;
use rustc_middle::ty::print::with_no_trimmed_paths;
use rustc_middle::ty::{self, AdtDef, Ty, TyCtxt, TypeVisitableExt};
use rustc_middle::ty::{self, AdtDef, Ty, TyCtxt};
use rustc_session::lint::builtin::{
BINDINGS_WITH_VARIANT_NAME, IRREFUTABLE_LET_PATTERNS, UNREACHABLE_PATTERNS,
};
Expand Down Expand Up @@ -231,6 +231,10 @@ impl<'p, 'tcx> MatchVisitor<'_, 'p, 'tcx> {
if let LetSource::None = source {
return;
}
if let Err(err) = pat.pat_error_reported() {
self.error = Err(err);
return;
}
self.check_patterns(pat, Refutable);
let mut cx = self.new_cx(self.lint_level, true);
let tpat = self.lower_pattern(&mut cx, pat);
Expand All @@ -252,6 +256,10 @@ impl<'p, 'tcx> MatchVisitor<'_, 'p, 'tcx> {
self.with_lint_level(arm.lint_level, |this| {
this.check_patterns(&arm.pattern, Refutable);
});
if let Err(err) = arm.pattern.pat_error_reported() {
self.error = Err(err);
return;
}
}

let tarms: Vec<_> = arms
Expand Down Expand Up @@ -334,7 +342,8 @@ impl<'p, 'tcx> MatchVisitor<'_, 'p, 'tcx> {
// and record chain members that aren't let exprs.
let mut chain_refutabilities = Vec::new();

let add = |expr: ExprId, mut local_lint_level| {
let mut error = Ok(());
let mut add = |expr: ExprId, mut local_lint_level| {
// `local_lint_level` is the lint level enclosing the pattern inside `expr`.
let mut expr = &self.thir[expr];
debug!(?expr, ?local_lint_level, "add");
Expand All @@ -348,6 +357,10 @@ impl<'p, 'tcx> MatchVisitor<'_, 'p, 'tcx> {
debug!(?expr, ?local_lint_level, "after scopes");
match expr.kind {
ExprKind::Let { box ref pat, expr: _ } => {
if let Err(err) = pat.pat_error_reported() {
error = Err(err);
return None;
}
let mut ncx = self.new_cx(local_lint_level, true);
let tpat = self.lower_pattern(&mut ncx, pat);
let refutable = !is_let_irrefutable(&mut ncx, local_lint_level, tpat);
Expand Down Expand Up @@ -380,6 +393,11 @@ impl<'p, 'tcx> MatchVisitor<'_, 'p, 'tcx> {
debug!(?chain_refutabilities);
chain_refutabilities.reverse();

if error.is_err() {
self.error = error;
return;
}

// 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
Expand Down Expand Up @@ -426,6 +444,12 @@ impl<'p, 'tcx> MatchVisitor<'_, 'p, 'tcx> {

#[instrument(level = "trace", skip(self))]
fn check_irrefutable(&mut self, pat: &Pat<'tcx>, origin: &str, sp: Option<Span>) {
// If we got errors while lowering, don't emit anything more.
if let Err(err) = pat.pat_error_reported() {
self.error = Err(err);
return;
}

let mut cx = self.new_cx(self.lint_level, false);

let pattern = self.lower_pattern(&mut cx, pat);
Expand Down Expand Up @@ -682,12 +706,6 @@ fn non_exhaustive_match<'p, 'tcx>(
arms: &[ArmId],
expr_span: Span,
) -> ErrorGuaranteed {
for &arm in arms {
if let Err(err) = thir[arm].pattern.error_reported() {
return err;
}
}

let is_empty_match = arms.is_empty();
let non_empty_enum = match scrut_ty.kind() {
ty::Adt(def, _) => def.is_enum() && !def.variants().is_empty(),
Expand Down
22 changes: 7 additions & 15 deletions compiler/rustc_mir_build/src/thir/pattern/const_to_pat.rs
Original file line number Diff line number Diff line change
Expand Up @@ -196,9 +196,7 @@ impl<'tcx> ConstToPat<'tcx> {
};
// All branches above emitted an error. Don't print any more lints.
// We errored. Signal that in the pattern, so that follow up errors can be silenced.
let kind = PatKind::Constant {
value: mir::Const::Ty(ty::Const::new_error(self.tcx(), e, cv.ty())),
};
let kind = PatKind::Error(e);
return Box::new(Pat { span: self.span, ty: cv.ty(), kind });
} else if !self.saw_const_match_lint.get() {
if let Some(mir_structural_match_violation) = mir_structural_match_violation {
Expand Down Expand Up @@ -351,15 +349,15 @@ impl<'tcx> ConstToPat<'tcx> {
let e = tcx.sess.emit_err(InvalidPattern { span, non_sm_ty: ty });
self.saw_const_match_error.set(Some(e));
// We errored. Signal that in the pattern, so that follow up errors can be silenced.
PatKind::Constant { value: mir::Const::Ty(ty::Const::new_error(tcx, e, ty)) }
PatKind::Error(e)
}
ty::Adt(adt_def, _) if !self.type_marked_structural(ty) => {
debug!("adt_def {:?} has !type_marked_structural for cv.ty: {:?}", adt_def, ty,);
let err = TypeNotStructural { span, non_sm_ty: ty };
let e = tcx.sess.emit_err(err);
self.saw_const_match_error.set(Some(e));
// We errored. Signal that in the pattern, so that follow up errors can be silenced.
PatKind::Constant { value: mir::Const::Ty(ty::Const::new_error(tcx, e, ty)) }
PatKind::Error(e)
}
ty::Adt(adt_def, args) if adt_def.is_enum() => {
let (&variant_index, fields) = cv.unwrap_branch().split_first().unwrap();
Expand Down Expand Up @@ -434,17 +432,13 @@ impl<'tcx> ConstToPat<'tcx> {
} else {
if let Some(e) = self.saw_const_match_error.get() {
// We already errored. Signal that in the pattern, so that follow up errors can be silenced.
PatKind::Constant {
value: mir::Const::Ty(ty::Const::new_error(tcx, e, ty)),
}
PatKind::Error(e)
} else {
let err = TypeNotStructural { span, non_sm_ty: *pointee_ty };
let e = tcx.sess.emit_err(err);
self.saw_const_match_error.set(Some(e));
// We errored. Signal that in the pattern, so that follow up errors can be silenced.
PatKind::Constant {
value: mir::Const::Ty(ty::Const::new_error(tcx, e, ty)),
}
PatKind::Error(e)
}
}
}
Expand All @@ -456,9 +450,7 @@ impl<'tcx> ConstToPat<'tcx> {
let err = UnsizedPattern { span, non_sm_ty: *pointee_ty };
let e = tcx.sess.emit_err(err);
// We errored. Signal that in the pattern, so that follow up errors can be silenced.
PatKind::Constant {
value: mir::Const::Ty(ty::Const::new_error(tcx, e, ty)),
}
PatKind::Error(e)
} else {
let old = self.behind_reference.replace(true);
// `b"foo"` produces a `&[u8; 3]`, but you can't use constants of array type when
Expand Down Expand Up @@ -489,7 +481,7 @@ impl<'tcx> ConstToPat<'tcx> {
let e = tcx.sess.emit_err(err);
self.saw_const_match_error.set(Some(e));
// We errored. Signal that in the pattern, so that follow up errors can be silenced.
PatKind::Constant { value: mir::Const::Ty(ty::Const::new_error(tcx, e, ty)) }
PatKind::Error(e)
}
};

Expand Down
4 changes: 4 additions & 0 deletions compiler/rustc_mir_build/src/thir/pattern/deconstruct_pat.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1525,6 +1525,10 @@ impl<'p, 'tcx> DeconstructedPat<'p, 'tcx> {
let pats = expand_or_pat(pat);
fields = Fields::from_iter(cx, pats.into_iter().map(mkpat));
}
PatKind::Error(_) => {
ctor = Opaque;
fields = Fields::empty();
}
}
DeconstructedPat::new(ctor, fields, pat.ty, pat.span)
}
Expand Down
Loading

0 comments on commit 5df9381

Please sign in to comment.