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.

Sign up for GitHub

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 spurious unreachable pattern lints #115937

Merged
merged 3 commits into from
Oct 11, 2023
Merged

Prevent spurious unreachable pattern lints #115937

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d1fd11f
Prevent spurious `unreachable pattern` lints
oli-obk Sep 18, 2023
e83467c
Avoid emitting the non_exhaustive error if other errors already occurred
oli-obk Sep 19, 2023
eca786c
Remember the `ErrorReported` used to silence follow up errors instead…
oli-obk Sep 28, 2023
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
13 changes: 8 additions & 5 deletions compiler/rustc_middle/src/thir.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -581,13 +581,13 @@ pub enum BindingMode {
ByRef(BorrowKind),
}

#[derive(Clone, Debug, HashStable)]
#[derive(Clone, Debug, HashStable, TypeVisitable)]
pub struct FieldPat<'tcx> {
pub field: FieldIdx,
pub pattern: Box<Pat<'tcx>>,
}

#[derive(Clone, Debug, HashStable)]
#[derive(Clone, Debug, HashStable, TypeVisitable)]
pub struct Pat<'tcx> {
pub ty: Ty<'tcx>,
pub span: Span,
Expand Down Expand Up @@ -664,7 +664,7 @@ impl<'tcx> IntoDiagnosticArg for Pat<'tcx> {
}
}

#[derive(Clone, Debug, HashStable)]
#[derive(Clone, Debug, HashStable, TypeVisitable)]
pub struct Ascription<'tcx> {
pub annotation: CanonicalUserTypeAnnotation<'tcx>,
/// Variance to use when relating the `user_ty` to the **type of the value being
Expand All @@ -688,7 +688,7 @@ pub struct Ascription<'tcx> {
pub variance: ty::Variance,
}

#[derive(Clone, Debug, HashStable)]
#[derive(Clone, Debug, HashStable, TypeVisitable)]
pub enum PatKind<'tcx> {
/// A wildcard pattern: `_`.
Wild,
Expand All @@ -702,7 +702,9 @@ pub enum PatKind<'tcx> {
Binding {
mutability: Mutability,
name: Symbol,
#[type_visitable(ignore)]
mode: BindingMode,
#[type_visitable(ignore)]
var: LocalVarId,
ty: Ty<'tcx>,
subpattern: Option<Box<Pat<'tcx>>>,
Expand Down Expand Up @@ -771,10 +773,11 @@ pub enum PatKind<'tcx> {
},
}

#[derive(Clone, Debug, PartialEq, HashStable)]
#[derive(Clone, Debug, PartialEq, HashStable, TypeVisitable)]
pub struct PatRange<'tcx> {
pub lo: mir::Const<'tcx>,
pub hi: mir::Const<'tcx>,
#[type_visitable(ignore)]
pub end: RangeEnd,
}

Expand Down
8 changes: 7 additions & 1 deletion compiler/rustc_mir_build/src/thir/pattern/check_match.rs
View file
Open in desktop
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};
use rustc_middle::ty::{self, AdtDef, Ty, TyCtxt, TypeVisitableExt};
use rustc_session::lint::builtin::{
BINDINGS_WITH_VARIANT_NAME, IRREFUTABLE_LET_PATTERNS, UNREACHABLE_PATTERNS,
};
Expand Down Expand Up @@ -682,6 +682,12 @@ 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() {
RalfJung marked this conversation as resolved.
Show resolved Hide resolved
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
85 changes: 48 additions & 37 deletions compiler/rustc_mir_build/src/thir/pattern/const_to_pat.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use rustc_middle::mir;
use rustc_middle::thir::{FieldPat, Pat, PatKind};
use rustc_middle::ty::{self, Ty, TyCtxt, ValTree};
use rustc_session::lint;
use rustc_span::Span;
use rustc_span::{ErrorGuaranteed, Span};
use rustc_target::abi::{FieldIdx, VariantIdx};
use rustc_trait_selection::traits::query::evaluate_obligation::InferCtxtExt;
use rustc_trait_selection::traits::{self, ObligationCause};
Expand Down Expand Up @@ -48,7 +48,7 @@ struct ConstToPat<'tcx> {
// This tracks if we emitted some hard error for a given const value, so that
// we will not subsequently issue an irrelevant lint for the same const
// value.
saw_const_match_error: Cell<bool>,
saw_const_match_error: Cell<Option<ErrorGuaranteed>>,

// This tracks if we emitted some diagnostic for a given const value, so that
// we will not subsequently issue an irrelevant lint for the same const
Expand Down Expand Up @@ -84,7 +84,7 @@ impl<'tcx> ConstToPat<'tcx> {
span,
infcx,
param_env: pat_ctxt.param_env,
saw_const_match_error: Cell::new(false),
saw_const_match_error: Cell::new(None),
saw_const_match_lint: Cell::new(false),
behind_reference: Cell::new(false),
treat_byte_string_as_slice: pat_ctxt
Expand Down Expand Up @@ -154,7 +154,7 @@ impl<'tcx> ConstToPat<'tcx> {
}),
};

if !self.saw_const_match_error.get() {
if self.saw_const_match_error.get().is_none() {
// If we were able to successfully convert the const to some pat (possibly with some
// lints, but no errors), double-check that all types in the const implement
// `Structural` and `PartialEq`.
Expand All @@ -180,23 +180,26 @@ impl<'tcx> ConstToPat<'tcx> {

if let Some(non_sm_ty) = structural {
if !self.type_has_partial_eq_impl(cv.ty()) {
if let ty::Adt(def, ..) = non_sm_ty.kind() {
let e = if let ty::Adt(def, ..) = non_sm_ty.kind() {
if def.is_union() {
let err = UnionPattern { span: self.span };
self.tcx().sess.emit_err(err);
self.tcx().sess.emit_err(err)
} else {
// fatal avoids ICE from resolution of nonexistent method (rare case).
self.tcx()
.sess
.emit_fatal(TypeNotStructural { span: self.span, non_sm_ty });
.emit_fatal(TypeNotStructural { span: self.span, non_sm_ty })
}
} else {
let err = InvalidPattern { span: self.span, non_sm_ty };
self.tcx().sess.emit_err(err);
}
self.tcx().sess.emit_err(err)
};
// All branches above emitted an error. Don't print any more lints.
// The pattern we return is irrelevant since we errored.
return Box::new(Pat { span: self.span, ty: cv.ty(), kind: PatKind::Wild });
// 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())),
};
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 {
match non_sm_ty.kind() {
Expand Down Expand Up @@ -330,7 +333,7 @@ impl<'tcx> ConstToPat<'tcx> {
// Backwards compatibility hack because we can't cause hard errors on these
// types, so we compare them via `PartialEq::eq` at runtime.
ty::Adt(..) if !self.type_marked_structural(ty) && self.behind_reference.get() => {
if !self.saw_const_match_error.get() && !self.saw_const_match_lint.get() {
if self.saw_const_match_error.get().is_none() && !self.saw_const_match_lint.get() {
self.saw_const_match_lint.set(true);
tcx.emit_spanned_lint(
lint::builtin::INDIRECT_STRUCTURAL_MATCH,
Expand All @@ -345,18 +348,18 @@ impl<'tcx> ConstToPat<'tcx> {
return Err(FallbackToOpaqueConst);
}
ty::FnDef(..) => {
self.saw_const_match_error.set(true);
tcx.sess.emit_err(InvalidPattern { span, non_sm_ty: ty });
// We errored, so the pattern we generate is irrelevant.
PatKind::Wild
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)) }
}
ty::Adt(adt_def, _) if !self.type_marked_structural(ty) => {
debug!("adt_def {:?} has !type_marked_structural for cv.ty: {:?}", adt_def, ty,);
self.saw_const_match_error.set(true);
let err = TypeNotStructural { span, non_sm_ty: ty };
tcx.sess.emit_err(err);
// We errored, so the pattern we generate is irrelevant.
PatKind::Wild
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)) }
}
ty::Adt(adt_def, args) if adt_def.is_enum() => {
let (&variant_index, fields) = cv.unwrap_branch().split_first().unwrap();
Expand Down Expand Up @@ -416,7 +419,9 @@ impl<'tcx> ConstToPat<'tcx> {
// instead of a hard error.
ty::Adt(_, _) if !self.type_marked_structural(*pointee_ty) => {
if self.behind_reference.get() {
if !self.saw_const_match_error.get() && !self.saw_const_match_lint.get() {
if self.saw_const_match_error.get().is_none()
&& !self.saw_const_match_lint.get()
{
self.saw_const_match_lint.set(true);
tcx.emit_spanned_lint(
lint::builtin::INDIRECT_STRUCTURAL_MATCH,
Expand All @@ -427,14 +432,20 @@ impl<'tcx> ConstToPat<'tcx> {
}
return Err(FallbackToOpaqueConst);
} else {
if !self.saw_const_match_error.get() {
self.saw_const_match_error.set(true);
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)),
}
} else {
let err = TypeNotStructural { span, non_sm_ty: *pointee_ty };
tcx.sess.emit_err(err);
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)),
}
}
tcx.sess.delay_span_bug(span, "`saw_const_match_error` set but no error?");
// We errored, so the pattern we generate is irrelevant.
PatKind::Wild
}
}
// All other references are converted into deref patterns and then recursively
Expand All @@ -443,11 +454,11 @@ impl<'tcx> ConstToPat<'tcx> {
_ => {
if !pointee_ty.is_sized(tcx, param_env) && !pointee_ty.is_slice() {
let err = UnsizedPattern { span, non_sm_ty: *pointee_ty };
tcx.sess.emit_err(err);

// FIXME: introduce PatKind::Error to silence follow up diagnostics due to unreachable patterns.
// We errored, so the pattern we generate is irrelevant.
PatKind::Wild
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)),
}
} 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 All @@ -474,15 +485,15 @@ impl<'tcx> ConstToPat<'tcx> {
}
ty::FnPtr(..) | ty::RawPtr(..) => unreachable!(),
_ => {
self.saw_const_match_error.set(true);
let err = InvalidPattern { span, non_sm_ty: ty };
tcx.sess.emit_err(err);
// We errored, so the pattern we generate is irrelevant.
PatKind::Wild
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)) }
}
};

if !self.saw_const_match_error.get()
if self.saw_const_match_error.get().is_none()
&& !self.saw_const_match_lint.get()
&& mir_structural_match_violation
// FIXME(#73448): Find a way to bring const qualification into parity with
Expand Down
6 changes: 6 additions & 0 deletions compiler/rustc_type_ir/src/structural_impls.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,12 @@ impl<I: Interner, T: TypeVisitable<I>> TypeVisitable<I> for &[T] {
}
}

impl<I: Interner, T: TypeVisitable<I>> TypeVisitable<I> for Box<[T]> {
fn visit_with<V: TypeVisitor<I>>(&self, visitor: &mut V) -> ControlFlow<V::BreakTy> {
self.iter().try_for_each(|t| t.visit_with(visitor))
}
}

impl<I: Interner, T: TypeFoldable<I>, Ix: Idx> TypeFoldable<I> for IndexVec<Ix, T> {
fn try_fold_with<F: FallibleTypeFolder<I>>(self, folder: &mut F) -> Result<Self, F::Error> {
self.try_map_id(|x| x.try_fold_with(folder))
Expand Down
14 changes: 13 additions & 1 deletion tests/ui/const-generics/generic_const_exprs/typeid-equality-by-subtyping.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,5 +7,17 @@ LL | WHAT_A_TYPE => 0,
= note: the traits must be derived, manual `impl`s are not sufficient
= note: see https://doc.rust-lang.org/stable/std/marker/trait.StructuralEq.html for details

error: aborting due to previous error
error[E0015]: cannot match on `TypeId` in constant functions
--> $DIR/typeid-equality-by-subtyping.rs:18:9
|
LL | WHAT_A_TYPE => 0,
| ^^^^^^^^^^^
|
= note: `TypeId` cannot be compared in compile-time, and therefore cannot be used in `match`es
note: impl defined here, but it is not `const`
--> $SRC_DIR/core/src/any.rs:LL:COL
= note: calls in constant functions are limited to constant functions, tuple structs and tuple variants

error: aborting due to 2 previous errors
Comment on lines +10 to +21
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where is this error coming from? I guess it was there before but got skipped because of previous errors?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh yea. We're actually generating a PartialEq call for it now, because we have an actual pattern now instead of a wildcard, which has no additional code.


For more information about this error, try `rustc --explain E0015`.
1 change: 0 additions & 1 deletion tests/ui/consts/const_in_pattern/issue-78057.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,5 @@ fn main() {
FOO => {},
//~^ ERROR must be annotated with `#[derive(PartialEq, Eq)]`
_ => {}
//~^ ERROR unreachable pattern
}
}
17 changes: 1 addition & 16 deletions tests/ui/consts/const_in_pattern/issue-78057.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,20 +7,5 @@ LL | FOO => {},
= note: the traits must be derived, manual `impl`s are not sufficient
= note: see https://doc.rust-lang.org/stable/std/marker/trait.StructuralEq.html for details

error: unreachable pattern
--> $DIR/issue-78057.rs:14:9
|
LL | FOO => {},
| --- matches any value
LL |
LL | _ => {}
| ^ unreachable pattern
|
note: the lint level is defined here
--> $DIR/issue-78057.rs:1:9
|
LL | #![deny(unreachable_patterns)]
| ^^^^^^^^^^^^^^^^^^^^

error: aborting due to 2 previous errors
error: aborting due to previous error

4 changes: 2 additions & 2 deletions tests/ui/pattern/non-structural-match-types.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,9 @@

fn main() {
match loop {} {
const { || {} } => {}, //~ ERROR cannot be used in patterns
const { || {} } => {} //~ ERROR cannot be used in patterns
}
match loop {} {
const { async {} } => {}, //~ ERROR cannot be used in patterns
const { async {} } => {} //~ ERROR cannot be used in patterns
}
}
4 changes: 2 additions & 2 deletions tests/ui/pattern/non-structural-match-types.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
error: `{closure@$DIR/non-structural-match-types.rs:9:17: 9:19}` cannot be used in patterns
--> $DIR/non-structural-match-types.rs:9:9
|
LL | const { || {} } => {},
LL | const { || {} } => {}
| ^^^^^^^^^^^^^^^

error: `{async block@$DIR/non-structural-match-types.rs:12:17: 12:25}` cannot be used in patterns
--> $DIR/non-structural-match-types.rs:12:9
|
LL | const { async {} } => {},
LL | const { async {} } => {}
| ^^^^^^^^^^^^^^^^^^

error: aborting due to 2 previous errors
Expand Down
Loading
Loading