Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prevent more spurious unreachable pattern lints #116715

Merged
merged 3 commits into from
Oct 14, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading