Skip to content

Commit

Permalink
Make sure we don't hide errors just because a lint has been emitted
Browse files Browse the repository at this point in the history
  • Loading branch information
oli-obk committed Sep 23, 2020
1 parent 177d0ce commit 9a7e66a
Show file tree
Hide file tree
Showing 8 changed files with 34 additions and 28 deletions.
27 changes: 20 additions & 7 deletions compiler/rustc_mir_build/src/thir/pattern/const_to_pat.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,11 +43,16 @@ struct ConstToPat<'a, 'tcx> {
span: Span,
param_env: ty::ParamEnv<'tcx>,

// This tracks if we saw some error or lint for a given const value, so that
// 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>,

// 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
// value.
saw_const_match_lint: Cell<bool>,

// For backcompat we need to keep allowing non-structurally-eq types behind references.
// See also all the `cant-hide-behind` tests.
behind_reference: Cell<bool>,
Expand Down Expand Up @@ -75,6 +80,7 @@ impl<'a, 'tcx> ConstToPat<'a, 'tcx> {
param_env: pat_ctxt.param_env,
include_lint_checks: pat_ctxt.include_lint_checks,
saw_const_match_error: Cell::new(false),
saw_const_match_lint: Cell::new(false),
behind_reference: Cell::new(false),
}
}
Expand Down Expand Up @@ -165,7 +171,7 @@ impl<'a, 'tcx> ConstToPat<'a, 'tcx> {
if !self.type_has_partial_eq_impl(cv.ty) {
// span_fatal avoids ICE from resolution of non-existent method (rare case).
self.tcx().sess.span_fatal(self.span, &msg);
} else if mir_structural_match_violation {
} else if mir_structural_match_violation && !self.saw_const_match_lint.get() {
self.tcx().struct_span_lint_hir(
lint::builtin::INDIRECT_STRUCTURAL_MATCH,
self.id,
Expand Down Expand Up @@ -289,8 +295,11 @@ impl<'a, 'tcx> ConstToPat<'a, '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(cv.ty) && self.behind_reference.get() => {
if self.include_lint_checks && !self.saw_const_match_error.get() {
self.saw_const_match_error.set(true);
if self.include_lint_checks
&& !self.saw_const_match_error.get()
&& !self.saw_const_match_lint.get()
{
self.saw_const_match_lint.set(true);
let msg = format!(
"to use a constant of type `{}` in a pattern, \
`{}` must be annotated with `#[derive(PartialEq, Eq)]`",
Expand Down Expand Up @@ -429,8 +438,11 @@ impl<'a, 'tcx> ConstToPat<'a, 'tcx> {
// compilation choices change the runtime behaviour of the match.
// See https://github.com/rust-lang/rust/issues/70861 for examples.
ty::FnPtr(..) | ty::RawPtr(..) => {
if self.include_lint_checks && !self.saw_const_match_error.get() {
self.saw_const_match_error.set(true);
if self.include_lint_checks
&& !self.saw_const_match_error.get()
&& !self.saw_const_match_lint.get()
{
self.saw_const_match_lint.set(true);
let msg = "function pointers and unsized pointers in patterns behave \
unpredictably and should not be relied upon. \
See https://github.com/rust-lang/rust/issues/70861 for details.";
Expand All @@ -457,12 +469,13 @@ impl<'a, 'tcx> ConstToPat<'a, 'tcx> {

if self.include_lint_checks
&& !self.saw_const_match_error.get()
&& !self.saw_const_match_lint.get()
&& mir_structural_match_violation
// FIXME(#73448): Find a way to bring const qualification into parity with
// `search_for_structural_match_violation` and then remove this condition.
&& self.search_for_structural_match_violation(cv.ty).is_some()
{
self.saw_const_match_error.set(true);
self.saw_const_match_lint.set(true);
let msg = format!(
"to use a constant of type `{}` in a pattern, \
the constant's initializer must be trivial or all types \
Expand Down
6 changes: 3 additions & 3 deletions src/test/ui/consts/match_ice.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,10 @@ struct T;
fn main() {
const C: &S = &S;
match C {
//~^ non-exhaustive patterns: `&S` not covered
C => {}
//~^ WARN must be annotated with `#[derive(PartialEq, Eq)]`
//~| WARN was previously accepted by the compiler
//~^ ERROR must be annotated with `#[derive(PartialEq, Eq)]`
//~| WARN must be annotated
//~| WARN previously accepted
}
const K: &T = &T;
match K {
Expand Down
17 changes: 5 additions & 12 deletions src/test/ui/consts/match_ice.stderr
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
warning: to use a constant of type `&S` in a pattern, the constant's initializer must be trivial or all types in the constant must be annotated with `#[derive(PartialEq, Eq)]`
--> $DIR/match_ice.rs:12:9
--> $DIR/match_ice.rs:11:9
|
LL | C => {}
| ^
Expand All @@ -8,18 +8,11 @@ LL | C => {}
= warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
= note: for more information, see issue #73448 <https://github.com/rust-lang/rust/issues/73448>

error[E0004]: non-exhaustive patterns: `&S` not covered
--> $DIR/match_ice.rs:10:11
error: to use a constant of type `S` in a pattern, `S` must be annotated with `#[derive(PartialEq, Eq)]`
--> $DIR/match_ice.rs:11:9
|
LL | struct S;
| --------- `S` defined here
...
LL | match C {
| ^ pattern `&S` not covered
|
= help: ensure that all possible cases are being handled, possibly by adding wildcards or more match arms
= note: the matched value is of type `&S`
LL | C => {}
| ^

error: aborting due to previous error; 1 warning emitted

For more information about this error, try `rustc --explain E0004`.
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ fn main() {
match WRAP_DOUBLY_INDIRECT_INLINE {
WRAP_DOUBLY_INDIRECT_INLINE => { panic!("WRAP_DOUBLY_INDIRECT_INLINE matched itself"); }
//~^ WARN must be annotated with `#[derive(PartialEq, Eq)]`
//~| WARN will become a hard error in a future release
//~| WARN this was previously accepted
_ => { println!("WRAP_DOUBLY_INDIRECT_INLINE correctly did not match itself"); }
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ fn main() {
match WRAP_DOUBLY_INDIRECT_PARAM {
WRAP_DOUBLY_INDIRECT_PARAM => { panic!("WRAP_DOUBLY_INDIRECT_PARAM matched itself"); }
//~^ WARN must be annotated with `#[derive(PartialEq, Eq)]`
//~| WARN will become a hard error in a future release
//~| WARN this was previously accepted
_ => { println!("WRAP_DOUBLY_INDIRECT_PARAM correctly did not match itself"); }
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ fn main() {
match WRAP_INDIRECT_INLINE {
WRAP_INDIRECT_INLINE => { panic!("WRAP_INDIRECT_INLINE matched itself"); }
//~^ WARN must be annotated with `#[derive(PartialEq, Eq)]`
//~| WARN will become a hard error in a future release
//~| WARN this was previously accepted
_ => { println!("WRAP_INDIRECT_INLINE did not match itself"); }
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ fn main() {
match WRAP_INDIRECT_PARAM {
WRAP_INDIRECT_PARAM => { panic!("WRAP_INDIRECT_PARAM matched itself"); }
//~^ WARN must be annotated with `#[derive(PartialEq, Eq)]`
//~| WARN will become a hard error in a future release
//~| WARN this was previously accepted
_ => { println!("WRAP_INDIRECT_PARAM correctly did not match itself"); }
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -30,14 +30,14 @@ fn main() {
match RR_B0 {
RR_B1 => { println!("CLAIM RR0: {:?} matches {:?}", RR_B1, RR_B0); }
//~^ WARN must be annotated with `#[derive(PartialEq, Eq)]`
//~| WARN will become a hard error in a future release
//~| WARN this was previously accepted
_ => { }
}

match RR_B1 {
RR_B1 => { println!("CLAIM RR1: {:?} matches {:?}", RR_B1, RR_B1); }
//~^ WARN must be annotated with `#[derive(PartialEq, Eq)]`
//~| WARN will become a hard error in a future release
//~| WARN this was previously accepted
_ => { }
}
}

0 comments on commit 9a7e66a

Please sign in to comment.