From b54f122a1cb2593325501a2ed5b3fbfc47293615 Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Fri, 3 Apr 2020 15:22:37 +0200 Subject: [PATCH 01/21] Merge tuple and struct pattern generation. --- compiler/rustc_mir_build/src/thir/pattern/const_to_pat.rs | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/compiler/rustc_mir_build/src/thir/pattern/const_to_pat.rs b/compiler/rustc_mir_build/src/thir/pattern/const_to_pat.rs index 2816bad7eab..55b58384298 100644 --- a/compiler/rustc_mir_build/src/thir/pattern/const_to_pat.rs +++ b/compiler/rustc_mir_build/src/thir/pattern/const_to_pat.rs @@ -279,11 +279,7 @@ impl<'a, 'tcx> ConstToPat<'a, 'tcx> { subpatterns: field_pats(destructured.fields), } } - ty::Adt(_, _) => { - let destructured = tcx.destructure_const(param_env.and(cv)); - PatKind::Leaf { subpatterns: field_pats(destructured.fields) } - } - ty::Tuple(_) => { + ty::Tuple(_) | ty::Adt(_, _) => { let destructured = tcx.destructure_const(param_env.and(cv)); PatKind::Leaf { subpatterns: field_pats(destructured.fields) } } From 34c62e0abc82b7302a3b0ee16dfe445e1330ce4c Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Wed, 1 Jul 2020 11:41:38 +0200 Subject: [PATCH 02/21] Add a query for dereferencing constants of reference type --- compiler/rustc_middle/src/query/mod.rs | 8 +++++ compiler/rustc_mir/src/const_eval/mod.rs | 42 +++++++++++++++++++++++- compiler/rustc_mir/src/lib.rs | 4 +++ 3 files changed, 53 insertions(+), 1 deletion(-) diff --git a/compiler/rustc_middle/src/query/mod.rs b/compiler/rustc_middle/src/query/mod.rs index 44d906dada5..68bad428ea2 100644 --- a/compiler/rustc_middle/src/query/mod.rs +++ b/compiler/rustc_middle/src/query/mod.rs @@ -749,6 +749,14 @@ rustc_queries! { desc { "destructure constant" } } + /// Dereference a constant reference or raw pointer and turn the result into a constant + /// again. + query deref_const( + key: ty::ParamEnvAnd<'tcx, &'tcx ty::Const<'tcx>> + ) -> &'tcx ty::Const<'tcx> { + desc { "deref constant" } + } + query const_caller_location(key: (rustc_span::Symbol, u32, u32)) -> ConstValue<'tcx> { desc { "get a &core::panic::Location referring to a span" } } diff --git a/compiler/rustc_mir/src/const_eval/mod.rs b/compiler/rustc_mir/src/const_eval/mod.rs index c93feb5096b..451fa1458fd 100644 --- a/compiler/rustc_mir/src/const_eval/mod.rs +++ b/compiler/rustc_mir/src/const_eval/mod.rs @@ -2,11 +2,14 @@ use std::convert::TryFrom; +use rustc_hir::Mutability; use rustc_middle::mir; use rustc_middle::ty::{self, TyCtxt}; use rustc_span::{source_map::DUMMY_SP, symbol::Symbol}; -use crate::interpret::{intern_const_alloc_recursive, ConstValue, InternKind, InterpCx}; +use crate::interpret::{ + intern_const_alloc_recursive, ConstValue, InternKind, InterpCx, MemPlaceMeta, Scalar, +}; mod error; mod eval_queries; @@ -67,3 +70,40 @@ pub(crate) fn destructure_const<'tcx>( mir::DestructuredConst { variant, fields } } + +pub(crate) fn deref_const<'tcx>( + tcx: TyCtxt<'tcx>, + param_env: ty::ParamEnv<'tcx>, + val: &'tcx ty::Const<'tcx>, +) -> &'tcx ty::Const<'tcx> { + trace!("deref_const: {:?}", val); + let ecx = mk_eval_cx(tcx, DUMMY_SP, param_env, false); + let op = ecx.const_to_op(val, None).unwrap(); + let mplace = ecx.deref_operand(op).unwrap(); + if let Scalar::Ptr(ptr) = mplace.ptr { + assert_eq!( + ecx.memory.get_raw(ptr.alloc_id).unwrap().mutability, + Mutability::Not, + "deref_const cannot be used with mutable allocations as \ + that could allow pattern matching to observe mutable statics", + ); + } + + let ty = match mplace.meta { + MemPlaceMeta::None => mplace.layout.ty, + MemPlaceMeta::Poison => bug!("poison metadata in `deref_const`: {:#?}", mplace), + // In case of unsized types, figure out the real type behind. + MemPlaceMeta::Meta(scalar) => match mplace.layout.ty.kind { + ty::Dynamic(..) => ecx.read_drop_type_from_vtable(scalar).unwrap().1, + ty::Str => bug!("there's no sized equivalent of a `str`"), + ty::Slice(elem_ty) => tcx.mk_array(elem_ty, scalar.to_machine_usize(&tcx).unwrap()), + _ => bug!( + "type {} should not have metadata, but had {:?}", + mplace.layout.ty, + mplace.meta + ), + }, + }; + + tcx.mk_const(ty::Const { val: ty::ConstKind::Value(op_to_const(&ecx, mplace.into())), ty }) +} diff --git a/compiler/rustc_mir/src/lib.rs b/compiler/rustc_mir/src/lib.rs index 251037792c9..504c4ecd85c 100644 --- a/compiler/rustc_mir/src/lib.rs +++ b/compiler/rustc_mir/src/lib.rs @@ -60,4 +60,8 @@ pub fn provide(providers: &mut Providers) { let (param_env, value) = param_env_and_value.into_parts(); const_eval::destructure_const(tcx, param_env, value) }; + providers.deref_const = |tcx, param_env_and_value| { + let (param_env, value) = param_env_and_value.into_parts(); + const_eval::deref_const(tcx, param_env, value) + }; } From b2532a87306fafd097241a80f92f68b10df0cba4 Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Wed, 1 Jul 2020 15:10:51 +0200 Subject: [PATCH 03/21] Implement destructuring for all aggregates and for references --- compiler/rustc_mir/src/const_eval/mod.rs | 3 +- .../rustc_mir_build/src/build/matches/test.rs | 35 +++++- .../src/thir/pattern/const_to_pat.rs | 110 +++++++++++++----- library/std/src/ffi/os_str.rs | 2 +- src/test/ui/consts/consts-in-patterns.rs | 2 - src/test/ui/consts/match_ice.rs | 3 +- src/test/ui/consts/match_ice.stderr | 21 +--- src/test/ui/match/pattern-deref-miscompile.rs | 46 ++++++++ src/test/ui/pattern/const-pat-ice.rs | 8 +- src/test/ui/pattern/const-pat-ice.stderr | 13 --- .../usefulness/exhaustive_integer_patterns.rs | 2 +- .../exhaustive_integer_patterns.stderr | 8 +- .../usefulness/slice-pattern-const-2.rs | 6 +- .../usefulness/slice-pattern-const-2.stderr | 26 ++++- .../usefulness/slice-pattern-const-3.rs | 6 +- .../usefulness/slice-pattern-const-3.stderr | 26 ++++- .../slice-patterns-exhaustiveness.rs | 62 +++++++--- .../slice-patterns-exhaustiveness.stderr | 48 +++++++- 18 files changed, 312 insertions(+), 115 deletions(-) create mode 100644 src/test/ui/match/pattern-deref-miscompile.rs delete mode 100644 src/test/ui/pattern/const-pat-ice.stderr diff --git a/compiler/rustc_mir/src/const_eval/mod.rs b/compiler/rustc_mir/src/const_eval/mod.rs index 451fa1458fd..978d2fe0004 100644 --- a/compiler/rustc_mir/src/const_eval/mod.rs +++ b/compiler/rustc_mir/src/const_eval/mod.rs @@ -93,8 +93,7 @@ pub(crate) fn deref_const<'tcx>( MemPlaceMeta::None => mplace.layout.ty, MemPlaceMeta::Poison => bug!("poison metadata in `deref_const`: {:#?}", mplace), // In case of unsized types, figure out the real type behind. - MemPlaceMeta::Meta(scalar) => match mplace.layout.ty.kind { - ty::Dynamic(..) => ecx.read_drop_type_from_vtable(scalar).unwrap().1, + MemPlaceMeta::Meta(scalar) => match mplace.layout.ty.kind() { ty::Str => bug!("there's no sized equivalent of a `str`"), ty::Slice(elem_ty) => tcx.mk_array(elem_ty, scalar.to_machine_usize(&tcx).unwrap()), _ => bug!( diff --git a/compiler/rustc_mir_build/src/build/matches/test.rs b/compiler/rustc_mir_build/src/build/matches/test.rs index d81c3b68f48..796815cf216 100644 --- a/compiler/rustc_mir_build/src/build/matches/test.rs +++ b/compiler/rustc_mir_build/src/build/matches/test.rs @@ -409,7 +409,40 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { let deref_ty = match *ty.kind() { ty::Ref(_, deref_ty, _) => deref_ty, - _ => bug!("non_scalar_compare called on non-reference type: {}", ty), + _ => { + trace!("non_scalar_compare called on non-reference type: {}", ty); + // Backcompat hack: due to non-structural matches not being a hard error, we can + // reach this for types that have manual `Eq` or `PartialEq` impls. + assert!(!ty.is_structural_eq_shallow(self.hir.tcx())); + let ref_ty = self.hir.tcx().mk_imm_ref(self.hir.tcx().lifetimes.re_erased, ty); + // let y = &place; + let y = self.temp(ref_ty, source_info.span); + self.cfg.push_assign( + block, + source_info, + y, + Rvalue::Ref(self.hir.tcx().lifetimes.re_erased, BorrowKind::Shared, place), + ); + val = Operand::Move(y); + // let temp = expect; + let temp = self.temp(ty, source_info.span); + self.cfg.push_assign( + block, + source_info, + temp, + Rvalue::Use(expect), + ); + // reftemp = &temp; + let reftemp = self.temp(ref_ty, source_info.span); + self.cfg.push_assign( + block, + source_info, + reftemp, + Rvalue::Ref(self.hir.tcx().lifetimes.re_erased, BorrowKind::Shared, temp), + ); + expect = Operand::Move(reftemp); + ty + }, }; let eq_def_id = self.hir.tcx().require_lang_item(LangItem::PartialEq, None); diff --git a/compiler/rustc_mir_build/src/thir/pattern/const_to_pat.rs b/compiler/rustc_mir_build/src/thir/pattern/const_to_pat.rs index 55b58384298..d47c3e7e3ab 100644 --- a/compiler/rustc_mir_build/src/thir/pattern/const_to_pat.rs +++ b/compiler/rustc_mir_build/src/thir/pattern/const_to_pat.rs @@ -28,10 +28,13 @@ impl<'a, 'tcx> PatCtxt<'a, 'tcx> { debug!("const_to_pat: cv={:#?} id={:?}", cv, id); debug!("const_to_pat: cv.ty={:?} span={:?}", cv.ty, span); - self.tcx.infer_ctxt().enter(|infcx| { + let pat = self.tcx.infer_ctxt().enter(|infcx| { let mut convert = ConstToPat::new(self, id, span, infcx); convert.to_pat(cv, mir_structural_match_violation) - }) + }); + + debug!("const_to_pat: pat={:?}", pat); + pat } } @@ -45,6 +48,10 @@ struct ConstToPat<'a, 'tcx> { // value. saw_const_match_error: Cell, + // For backcompat we need to keep allowing non-structurally-eq types behind references. + // See also all the `cant-hide-behind` tests. + behind_reference: Cell, + // inference context used for checking `T: Structural` bounds. infcx: InferCtxt<'a, 'tcx>, @@ -65,6 +72,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), + behind_reference: Cell::new(false), } } @@ -233,7 +241,18 @@ impl<'a, 'tcx> ConstToPat<'a, 'tcx> { tcx.sess.span_err(span, "cannot use unions in constant patterns"); PatKind::Wild } - // keep old code until future-compat upgraded to errors. + // If the type is not structurally comparable, just emit the constant directly, + // causing the pattern match code to treat it opaquely. + // FIXME: This code doesn't emit errors itself, the caller emits the errors. + // So instead of specific errors, you just get blanket errors about the whole + // const type. See + // https://github.com/rust-lang/rust/pull/70743#discussion_r404701963 for + // details. + // 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() => { + PatKind::Constant { value: cv } + } ty::Adt(adt_def, _) if !self.type_marked_structural(cv.ty) => { debug!("adt_def {:?} has !type_marked_structural for cv.ty: {:?}", adt_def, cv.ty); let path = tcx.def_path_str(adt_def.did); @@ -246,28 +265,6 @@ impl<'a, 'tcx> ConstToPat<'a, 'tcx> { tcx.sess.span_err(span, &msg); PatKind::Wild } - // keep old code until future-compat upgraded to errors. - ty::Ref(_, adt_ty, _) if adt_ty.is_adt() && !self.type_marked_structural(adt_ty) => { - let adt_def = - if let ty::Adt(adt_def, _) = adt_ty.kind() { adt_def } else { unreachable!() }; - - debug!( - "adt_def {:?} has !type_marked_structural for adt_ty: {:?}", - adt_def, adt_ty - ); - - // HACK(estebank): Side-step ICE #53708, but anything other than erroring here - // would be wrong. Returnging `PatKind::Wild` is not technically correct. - let path = tcx.def_path_str(adt_def.did); - let msg = format!( - "to use a constant of type `{}` in a pattern, \ - `{}` must be annotated with `#[derive(PartialEq, Eq)]`", - path, path, - ); - self.saw_const_match_error.set(true); - tcx.sess.span_err(span, &msg); - PatKind::Wild - } ty::Adt(adt_def, substs) if adt_def.is_enum() => { let destructured = tcx.destructure_const(param_env.and(cv)); PatKind::Variant { @@ -293,7 +290,68 @@ impl<'a, 'tcx> ConstToPat<'a, 'tcx> { slice: None, suffix: Vec::new(), }, - _ => PatKind::Constant { value: cv }, + ty::Ref(_, pointee_ty, ..) => match *pointee_ty.kind() { + // These are not allowed and will error elsewhere anyway. + ty::Dynamic(..) => PatKind::Constant { value: cv }, + // `&str` and `&[u8]` are represented as `ConstValue::Slice`, let's keep using this + // optimization for now. + ty::Str => PatKind::Constant { value: cv }, + ty::Slice(elem_ty) if elem_ty == tcx.types.u8 => PatKind::Constant { value: cv }, + // `b"foo"` produces a `&[u8; 3]`, but you can't use constants of array type when + // matching against references, you can only use byte string literals. + // FIXME: clean this up, likely by permitting array patterns when matching on slices + ty::Array(elem_ty, _) if elem_ty == tcx.types.u8 => PatKind::Constant { value: cv }, + // Cannot merge this with the catch all branch below, because the `const_deref` + // changes the type from slice to array, and slice patterns behave differently from + // array patterns. + ty::Slice(..) => { + let old = self.behind_reference.replace(true); + let array = tcx.deref_const(self.param_env.and(cv)); + let val = PatKind::Deref { + subpattern: Pat { + kind: Box::new(PatKind::Slice { + prefix: tcx + .destructure_const(param_env.and(array)) + .fields + .iter() + .map(|val| self.recur(val)) + .collect(), + slice: None, + suffix: vec![], + }), + span, + ty: pointee_ty, + }, + }; + self.behind_reference.set(old); + val + } + // Backwards compatibility hack. Don't take away the reference, since + // `PartialEq::eq` takes a reference, this makes the rest of the matching logic + // simpler. + ty::Adt(..) if !self.type_marked_structural(pointee_ty) => { + PatKind::Constant { value: cv } + } + _ => { + let old = self.behind_reference.replace(true); + let val = PatKind::Deref { + subpattern: self.recur(tcx.deref_const(self.param_env.and(cv))), + }; + self.behind_reference.set(old); + val + } + }, + ty::Bool | ty::Char | ty::Int(_) | ty::Uint(_) | ty::FnDef(..) => { + PatKind::Constant { value: cv } + } + // FIXME: these can have very suprising behaviour where optimization levels or other + // compilation choices change the runtime behaviour of the match. + // See https://github.com/rust-lang/rust/issues/70861 for examples. + ty::FnPtr(..) | ty::RawPtr(..) => PatKind::Constant { value: cv }, + _ => { + tcx.sess.delay_span_bug(span, &format!("cannot make a pattern out of {}", cv.ty)); + PatKind::Wild + } }; Pat { span, ty: cv.ty, kind: Box::new(kind) } diff --git a/library/std/src/ffi/os_str.rs b/library/std/src/ffi/os_str.rs index c83e996634c..2663f682a1d 100644 --- a/library/std/src/ffi/os_str.rs +++ b/library/std/src/ffi/os_str.rs @@ -94,7 +94,7 @@ pub struct OsString { // `OsStr::from_inner` current implementation relies // on `OsStr` being layout-compatible with `Slice`. // When attribute privacy is implemented, `OsStr` should be annotated as `#[repr(transparent)]`. -// Anyway, `OsStr` representation and layout are considered implementation detail, are +// Anyway, `OsStr` representation and layout are considered implementation details, are // not documented and must not be relied upon. pub struct OsStr { inner: Slice, diff --git a/src/test/ui/consts/consts-in-patterns.rs b/src/test/ui/consts/consts-in-patterns.rs index d51215447d6..0295204c879 100644 --- a/src/test/ui/consts/consts-in-patterns.rs +++ b/src/test/ui/consts/consts-in-patterns.rs @@ -19,8 +19,6 @@ pub fn main() { assert_eq!(y, 2); let z = match &() { ZST => 9, - // FIXME: this should not be required - _ => 42, }; assert_eq!(z, 9); let z = match b"" { diff --git a/src/test/ui/consts/match_ice.rs b/src/test/ui/consts/match_ice.rs index 1e495438e83..008c03ecddc 100644 --- a/src/test/ui/consts/match_ice.rs +++ b/src/test/ui/consts/match_ice.rs @@ -10,10 +10,9 @@ fn main() { match C { C => {} //~^ ERROR to use a constant of type `S` in a pattern, `S` must be annotated with - //~| ERROR to use a constant of type `S` in a pattern, `S` must be annotated with } const K: &T = &T; - match K { //~ ERROR non-exhaustive patterns: `&T` not covered + match K { K => {} } } diff --git a/src/test/ui/consts/match_ice.stderr b/src/test/ui/consts/match_ice.stderr index 5477170fb1e..699b4a5e200 100644 --- a/src/test/ui/consts/match_ice.stderr +++ b/src/test/ui/consts/match_ice.stderr @@ -4,24 +4,5 @@ error: to use a constant of type `S` in a pattern, `S` must be annotated with `# LL | C => {} | ^ -error[E0004]: non-exhaustive patterns: `&T` not covered - --> $DIR/match_ice.rs:16:11 - | -LL | struct T; - | --------- `T` defined here -... -LL | match K { - | ^ pattern `&T` 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 `&T` +error: aborting due to previous error -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 | C => {} - | ^ - -error: aborting due to 3 previous errors - -For more information about this error, try `rustc --explain E0004`. diff --git a/src/test/ui/match/pattern-deref-miscompile.rs b/src/test/ui/match/pattern-deref-miscompile.rs new file mode 100644 index 00000000000..caa6d184a92 --- /dev/null +++ b/src/test/ui/match/pattern-deref-miscompile.rs @@ -0,0 +1,46 @@ +// run-pass + +fn main() { + match b"." as &[u8] { + b"." if true => {}, + b"." => panic!(), + b".." => panic!(), + b"" => panic!(), + _ => panic!(), + } + match b"." as &[u8] { + b"." if false => panic!(), + b"." => {}, + b".." => panic!(), + b"" => panic!(), + _ => panic!(), + } + match b".." as &[u8] { + b"." if true => panic!(), // the miscompile caused this arm to be reached + b"." => panic!(), + b".." => {}, + b"" => panic!(), + _ => panic!(), + } + match b".." as &[u8] { + b"." if false => panic!(), + b"." => panic!(), + b".." => {}, + b"" => panic!(), + _ => panic!(), + } + match b"" as &[u8] { + b"." if true => panic!(), + b"." => panic!(), + b".." => panic!(), + b"" => {}, + _ => panic!(), + } + match b"" as &[u8] { + b"." if false => panic!(), + b"." => panic!(), + b".." => panic!(), + b"" => {}, + _ => panic!(), + } +} diff --git a/src/test/ui/pattern/const-pat-ice.rs b/src/test/ui/pattern/const-pat-ice.rs index 06558767882..abfacf3936b 100644 --- a/src/test/ui/pattern/const-pat-ice.rs +++ b/src/test/ui/pattern/const-pat-ice.rs @@ -1,10 +1,4 @@ -// failure-status: 101 -// rustc-env:RUST_BACKTRACE=0 -// normalize-stderr-test "note: rustc 1.* running on .*" -> "note: rustc VERSION running on TARGET" -// normalize-stderr-test "note: compiler flags: .*" -> "note: compiler flags: FLAGS" -// normalize-stderr-test "/_match.rs:[0-9]+:[0-9]+" -> "/_match.rs:LL:CC" - -// This is a repro test for an ICE in our pattern handling of constants. +// check-pass const FOO: &&&u32 = &&&42; diff --git a/src/test/ui/pattern/const-pat-ice.stderr b/src/test/ui/pattern/const-pat-ice.stderr deleted file mode 100644 index 6b42c0e0848..00000000000 --- a/src/test/ui/pattern/const-pat-ice.stderr +++ /dev/null @@ -1,13 +0,0 @@ -thread 'rustc' panicked at 'assertion failed: rows.iter().all(|r| r.len() == v.len())', compiler/rustc_mir_build/src/thir/pattern/_match.rs:LL:CC -note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace - -error: internal compiler error: unexpected panic - -note: the compiler unexpectedly panicked. this is a bug. - -note: we would appreciate a bug report: https://github.com/rust-lang/rust/issues/new?labels=C-bug%2C+I-ICE%2C+T-compiler&template=ice.md - -note: rustc VERSION running on TARGET - -note: compiler flags: FLAGS - diff --git a/src/test/ui/pattern/usefulness/exhaustive_integer_patterns.rs b/src/test/ui/pattern/usefulness/exhaustive_integer_patterns.rs index d379dc44bf1..78cc0d28fb0 100644 --- a/src/test/ui/pattern/usefulness/exhaustive_integer_patterns.rs +++ b/src/test/ui/pattern/usefulness/exhaustive_integer_patterns.rs @@ -160,7 +160,7 @@ fn main() { match &0 { &42 => {} &FOO => {} //~ ERROR unreachable pattern - BAR => {} // Not detected as unreachable because `try_eval_bits` fails on `BAR`. + BAR => {} //~ ERROR unreachable pattern _ => {} } diff --git a/src/test/ui/pattern/usefulness/exhaustive_integer_patterns.stderr b/src/test/ui/pattern/usefulness/exhaustive_integer_patterns.stderr index de831520598..9f076c50a8f 100644 --- a/src/test/ui/pattern/usefulness/exhaustive_integer_patterns.stderr +++ b/src/test/ui/pattern/usefulness/exhaustive_integer_patterns.stderr @@ -135,6 +135,12 @@ error: unreachable pattern LL | &FOO => {} | ^^^^ -error: aborting due to 15 previous errors +error: unreachable pattern + --> $DIR/exhaustive_integer_patterns.rs:163:9 + | +LL | BAR => {} + | ^^^ + +error: aborting due to 16 previous errors For more information about this error, try `rustc --explain E0004`. diff --git a/src/test/ui/pattern/usefulness/slice-pattern-const-2.rs b/src/test/ui/pattern/usefulness/slice-pattern-const-2.rs index a36c550f530..4bf8d0fd2d3 100644 --- a/src/test/ui/pattern/usefulness/slice-pattern-const-2.rs +++ b/src/test/ui/pattern/usefulness/slice-pattern-const-2.rs @@ -6,19 +6,19 @@ fn main() { match s { MAGIC_TEST => (), [0x00, 0x00, 0x00, 0x00] => (), - [4, 5, 6, 7] => (), // FIXME(oli-obk): this should warn, but currently does not + [4, 5, 6, 7] => (), //~ ERROR unreachable pattern _ => (), } match s { [0x00, 0x00, 0x00, 0x00] => (), MAGIC_TEST => (), - [4, 5, 6, 7] => (), // FIXME(oli-obk): this should warn, but currently does not + [4, 5, 6, 7] => (), //~ ERROR unreachable pattern _ => (), } match s { [0x00, 0x00, 0x00, 0x00] => (), [4, 5, 6, 7] => (), - MAGIC_TEST => (), // FIXME(oli-obk): this should warn, but currently does not + MAGIC_TEST => (), //~ ERROR unreachable pattern _ => (), } const FOO: [u32; 1] = [4]; diff --git a/src/test/ui/pattern/usefulness/slice-pattern-const-2.stderr b/src/test/ui/pattern/usefulness/slice-pattern-const-2.stderr index cd0cb2e8876..dcad11a38a7 100644 --- a/src/test/ui/pattern/usefulness/slice-pattern-const-2.stderr +++ b/src/test/ui/pattern/usefulness/slice-pattern-const-2.stderr @@ -1,8 +1,8 @@ error: unreachable pattern - --> $DIR/slice-pattern-const-2.rs:28:9 + --> $DIR/slice-pattern-const-2.rs:9:9 | -LL | FOO => (), - | ^^^ +LL | [4, 5, 6, 7] => (), + | ^^^^^^^^^^^^ | note: the lint level is defined here --> $DIR/slice-pattern-const-2.rs:1:9 @@ -10,5 +10,23 @@ note: the lint level is defined here LL | #![deny(unreachable_patterns)] | ^^^^^^^^^^^^^^^^^^^^ -error: aborting due to previous error +error: unreachable pattern + --> $DIR/slice-pattern-const-2.rs:15:9 + | +LL | [4, 5, 6, 7] => (), + | ^^^^^^^^^^^^ + +error: unreachable pattern + --> $DIR/slice-pattern-const-2.rs:21:9 + | +LL | MAGIC_TEST => (), + | ^^^^^^^^^^ + +error: unreachable pattern + --> $DIR/slice-pattern-const-2.rs:28:9 + | +LL | FOO => (), + | ^^^ + +error: aborting due to 4 previous errors diff --git a/src/test/ui/pattern/usefulness/slice-pattern-const-3.rs b/src/test/ui/pattern/usefulness/slice-pattern-const-3.rs index 8805c43ba02..2ca8323f002 100644 --- a/src/test/ui/pattern/usefulness/slice-pattern-const-3.rs +++ b/src/test/ui/pattern/usefulness/slice-pattern-const-3.rs @@ -6,19 +6,19 @@ fn main() { match s { MAGIC_TEST => (), ["0x00", "0x00", "0x00", "0x00"] => (), - ["4", "5", "6", "7"] => (), // FIXME(oli-obk): this should warn, but currently does not + ["4", "5", "6", "7"] => (), //~ ERROR unreachable pattern _ => (), } match s { ["0x00", "0x00", "0x00", "0x00"] => (), MAGIC_TEST => (), - ["4", "5", "6", "7"] => (), // FIXME(oli-obk): this should warn, but currently does not + ["4", "5", "6", "7"] => (), //~ ERROR unreachable pattern _ => (), } match s { ["0x00", "0x00", "0x00", "0x00"] => (), ["4", "5", "6", "7"] => (), - MAGIC_TEST => (), // FIXME(oli-obk): this should warn, but currently does not + MAGIC_TEST => (), //~ ERROR unreachable pattern _ => (), } const FOO: [&str; 1] = ["boo"]; diff --git a/src/test/ui/pattern/usefulness/slice-pattern-const-3.stderr b/src/test/ui/pattern/usefulness/slice-pattern-const-3.stderr index 3ba01b9eba3..b90b3a88a18 100644 --- a/src/test/ui/pattern/usefulness/slice-pattern-const-3.stderr +++ b/src/test/ui/pattern/usefulness/slice-pattern-const-3.stderr @@ -1,8 +1,8 @@ error: unreachable pattern - --> $DIR/slice-pattern-const-3.rs:28:9 + --> $DIR/slice-pattern-const-3.rs:9:9 | -LL | FOO => (), - | ^^^ +LL | ["4", "5", "6", "7"] => (), + | ^^^^^^^^^^^^^^^^^^^^ | note: the lint level is defined here --> $DIR/slice-pattern-const-3.rs:1:9 @@ -10,5 +10,23 @@ note: the lint level is defined here LL | #![deny(unreachable_patterns)] | ^^^^^^^^^^^^^^^^^^^^ -error: aborting due to previous error +error: unreachable pattern + --> $DIR/slice-pattern-const-3.rs:15:9 + | +LL | ["4", "5", "6", "7"] => (), + | ^^^^^^^^^^^^^^^^^^^^ + +error: unreachable pattern + --> $DIR/slice-pattern-const-3.rs:21:9 + | +LL | MAGIC_TEST => (), + | ^^^^^^^^^^ + +error: unreachable pattern + --> $DIR/slice-pattern-const-3.rs:28:9 + | +LL | FOO => (), + | ^^^ + +error: aborting due to 4 previous errors diff --git a/src/test/ui/pattern/usefulness/slice-patterns-exhaustiveness.rs b/src/test/ui/pattern/usefulness/slice-patterns-exhaustiveness.rs index 52d1320dad1..46e0da5be9b 100644 --- a/src/test/ui/pattern/usefulness/slice-patterns-exhaustiveness.rs +++ b/src/test/ui/pattern/usefulness/slice-patterns-exhaustiveness.rs @@ -6,15 +6,15 @@ fn main() { let s10: &[bool; 10] = &[false; 10]; match s2 { - //~^ ERROR `&[false, _]` not covered + //~^ ERROR `&[false, _]` not covered [true, .., true] => {} } match s3 { - //~^ ERROR `&[false, ..]` not covered + //~^ ERROR `&[false, ..]` not covered [true, .., true] => {} } match s10 { - //~^ ERROR `&[false, ..]` not covered + //~^ ERROR `&[false, ..]` not covered [true, .., true] => {} } @@ -23,58 +23,58 @@ fn main() { [.., false] => {} } match s2 { - //~^ ERROR `&[false, true]` not covered + //~^ ERROR `&[false, true]` not covered [true, ..] => {} [.., false] => {} } match s3 { - //~^ ERROR `&[false, .., true]` not covered + //~^ ERROR `&[false, .., true]` not covered [true, ..] => {} [.., false] => {} } match s { - //~^ ERROR `&[false, .., true]` not covered + //~^ ERROR `&[false, .., true]` not covered [] => {} [true, ..] => {} [.., false] => {} } match s { - //~^ ERROR `&[_, ..]` not covered + //~^ ERROR `&[_, ..]` not covered [] => {} } match s { - //~^ ERROR `&[_, _, ..]` not covered + //~^ ERROR `&[_, _, ..]` not covered [] => {} [_] => {} } match s { - //~^ ERROR `&[false, ..]` not covered + //~^ ERROR `&[false, ..]` not covered [] => {} [true, ..] => {} } match s { - //~^ ERROR `&[false, _, ..]` not covered + //~^ ERROR `&[false, _, ..]` not covered [] => {} [_] => {} [true, ..] => {} } match s { - //~^ ERROR `&[_, .., false]` not covered + //~^ ERROR `&[_, .., false]` not covered [] => {} [_] => {} [.., true] => {} } match s { - //~^ ERROR `&[_, _, .., true]` not covered + //~^ ERROR `&[_, _, .., true]` not covered [] => {} [_] => {} [_, _] => {} [.., false] => {} } match s { - //~^ ERROR `&[true, _, .., _]` not covered + //~^ ERROR `&[true, _, .., _]` not covered [] => {} [_] => {} [_, _] => {} @@ -83,19 +83,43 @@ fn main() { const CONST: &[bool] = &[true]; match s { - //~^ ERROR `&[..]` not covered + //~^ ERROR `&[]` and `&[_, _, ..]` not covered + &[true] => {} + } + match s { + //~^ ERROR `&[]` and `&[_, _, ..]` not covered CONST => {} } match s { - //~^ ERROR `&[true]` not covered - [] => {}, - [false] => {}, - CONST => {}, + //~^ ERROR `&[]` and `&[_, _, ..]` not covered + CONST => {} + &[false] => {} + } + match s { + //~^ ERROR `&[]` and `&[_, _, ..]` not covered + &[false] => {} + CONST => {} + } + match s { + //~^ ERROR `&[_, _, ..]` not covered + &[] => {} + CONST => {} + } + match s { + //~^ ERROR `&[false]` not covered + &[] => {} + CONST => {} + &[_, _, ..] => {} + } + match s { + [] => {} + [false] => {} + CONST => {} [_, _, ..] => {} } const CONST1: &[bool; 1] = &[true]; match s1 { - //~^ ERROR `&[false]` not covered + //~^ ERROR `&[false]` not covered CONST1 => {} } match s1 { diff --git a/src/test/ui/pattern/usefulness/slice-patterns-exhaustiveness.stderr b/src/test/ui/pattern/usefulness/slice-patterns-exhaustiveness.stderr index 8b85eaeda0a..e34770fb912 100644 --- a/src/test/ui/pattern/usefulness/slice-patterns-exhaustiveness.stderr +++ b/src/test/ui/pattern/usefulness/slice-patterns-exhaustiveness.stderr @@ -115,26 +115,62 @@ LL | match s { = help: ensure that all possible cases are being handled, possibly by adding wildcards or more match arms = note: the matched value is of type `&[bool]` -error[E0004]: non-exhaustive patterns: `&[..]` not covered +error[E0004]: non-exhaustive patterns: `&[]` and `&[_, _, ..]` not covered --> $DIR/slice-patterns-exhaustiveness.rs:85:11 | LL | match s { - | ^ pattern `&[..]` not covered + | ^ patterns `&[]` and `&[_, _, ..]` 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 `&[bool]` -error[E0004]: non-exhaustive patterns: `&[true]` not covered +error[E0004]: non-exhaustive patterns: `&[]` and `&[_, _, ..]` not covered --> $DIR/slice-patterns-exhaustiveness.rs:89:11 | LL | match s { - | ^ pattern `&[true]` not covered + | ^ patterns `&[]` and `&[_, _, ..]` 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 `&[bool]` + +error[E0004]: non-exhaustive patterns: `&[]` and `&[_, _, ..]` not covered + --> $DIR/slice-patterns-exhaustiveness.rs:93:11 + | +LL | match s { + | ^ patterns `&[]` and `&[_, _, ..]` 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 `&[bool]` + +error[E0004]: non-exhaustive patterns: `&[]` and `&[_, _, ..]` not covered + --> $DIR/slice-patterns-exhaustiveness.rs:98:11 + | +LL | match s { + | ^ patterns `&[]` and `&[_, _, ..]` 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 `&[bool]` + +error[E0004]: non-exhaustive patterns: `&[_, _, ..]` not covered + --> $DIR/slice-patterns-exhaustiveness.rs:103:11 + | +LL | match s { + | ^ pattern `&[_, _, ..]` 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 `&[bool]` error[E0004]: non-exhaustive patterns: `&[false]` not covered - --> $DIR/slice-patterns-exhaustiveness.rs:97:11 + --> $DIR/slice-patterns-exhaustiveness.rs:108:11 + | +LL | match s { + | ^ pattern `&[false]` 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 `&[bool]` + +error[E0004]: non-exhaustive patterns: `&[false]` not covered + --> $DIR/slice-patterns-exhaustiveness.rs:121:11 | LL | match s1 { | ^^ pattern `&[false]` not covered @@ -142,6 +178,6 @@ LL | match s1 { = help: ensure that all possible cases are being handled, possibly by adding wildcards or more match arms = note: the matched value is of type `&[bool; 1]` -error: aborting due to 16 previous errors +error: aborting due to 20 previous errors For more information about this error, try `rustc --explain E0004`. From 3795886f7e1f3dc5f5dd207ba4a7c092fe929486 Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Sun, 20 Sep 2020 16:59:15 +0200 Subject: [PATCH 04/21] Split check for `PartialEq` impl into a method --- .../src/thir/pattern/const_to_pat.rs | 63 ++++++++++--------- 1 file changed, 35 insertions(+), 28 deletions(-) diff --git a/compiler/rustc_mir_build/src/thir/pattern/const_to_pat.rs b/compiler/rustc_mir_build/src/thir/pattern/const_to_pat.rs index d47c3e7e3ab..ebb71ea24ec 100644 --- a/compiler/rustc_mir_build/src/thir/pattern/const_to_pat.rs +++ b/compiler/rustc_mir_build/src/thir/pattern/const_to_pat.rs @@ -159,34 +159,7 @@ impl<'a, 'tcx> ConstToPat<'a, 'tcx> { } }); - // double-check there even *is* a semantic `PartialEq` to dispatch to. - // - // (If there isn't, then we can safely issue a hard - // error, because that's never worked, due to compiler - // using `PartialEq::eq` in this scenario in the past.) - // - // Note: To fix rust-lang/rust#65466, one could lift this check - // *before* any structural-match checking, and unconditionally error - // if `PartialEq` is not implemented. However, that breaks stable - // code at the moment, because types like `for <'a> fn(&'a ())` do - // not *yet* implement `PartialEq`. So for now we leave this here. - let ty_is_partial_eq: bool = { - let partial_eq_trait_id = - self.tcx().require_lang_item(hir::LangItem::PartialEq, Some(self.span)); - let obligation: PredicateObligation<'_> = predicate_for_trait_def( - self.tcx(), - self.param_env, - ObligationCause::misc(self.span, self.id), - partial_eq_trait_id, - 0, - cv.ty, - &[], - ); - // FIXME: should this call a `predicate_must_hold` variant instead? - self.infcx.predicate_may_hold(&obligation) - }; - - if !ty_is_partial_eq { + 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 { @@ -208,6 +181,40 @@ impl<'a, 'tcx> ConstToPat<'a, 'tcx> { inlined_const_as_pat } + fn type_has_partial_eq_impl(&self, ty: Ty<'tcx>) -> bool { + // double-check there even *is* a semantic `PartialEq` to dispatch to. + // + // (If there isn't, then we can safely issue a hard + // error, because that's never worked, due to compiler + // using `PartialEq::eq` in this scenario in the past.) + let partial_eq_trait_id = + self.tcx().require_lang_item(hir::LangItem::PartialEq, Some(self.span)); + let obligation: PredicateObligation<'_> = predicate_for_trait_def( + self.tcx(), + self.param_env, + ObligationCause::misc(self.span, self.id), + partial_eq_trait_id, + 0, + ty, + &[], + ); + // FIXME: should this call a `predicate_must_hold` variant instead? + + let has_impl = self.infcx.predicate_may_hold(&obligation); + + // Note: To fix rust-lang/rust#65466, we could just remove this type + // walk hack for function pointers, and unconditionally error + // if `PartialEq` is not implemented. However, that breaks stable + // code at the moment, because types like `for <'a> fn(&'a ())` do + // not *yet* implement `PartialEq`. So for now we leave this here. + has_impl + || ty.walk().any(|t| match t.unpack() { + ty::subst::GenericArgKind::Lifetime(_) => false, + ty::subst::GenericArgKind::Type(t) => t.is_fn_ptr(), + ty::subst::GenericArgKind::Const(_) => false, + }) + } + // Recursive helper for `to_pat`; invoke that (instead of calling this directly). fn recur(&self, cv: &'tcx ty::Const<'tcx>) -> Pat<'tcx> { let id = self.id; From aba5ea1430df393eddc90068e838de6b1707c0d8 Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Sun, 20 Sep 2020 17:11:00 +0200 Subject: [PATCH 05/21] Lint on function pointers used in patterns --- .../src/thir/pattern/const_to_pat.rs | 19 ++++++++++++- compiler/rustc_session/src/lint/builtin.rs | 27 +++++++++++++++++++ src/test/ui/issues/issue-44333.rs | 9 +++++-- src/test/ui/issues/issue-44333.stderr | 25 +++++++++++++++++ .../ui/rfc1445/issue-63479-match-fnptr.rs | 4 +++ .../ui/rfc1445/issue-63479-match-fnptr.stderr | 16 +++++++++++ 6 files changed, 97 insertions(+), 3 deletions(-) create mode 100644 src/test/ui/issues/issue-44333.stderr create mode 100644 src/test/ui/rfc1445/issue-63479-match-fnptr.stderr diff --git a/compiler/rustc_mir_build/src/thir/pattern/const_to_pat.rs b/compiler/rustc_mir_build/src/thir/pattern/const_to_pat.rs index ebb71ea24ec..6ca1ff2c5f2 100644 --- a/compiler/rustc_mir_build/src/thir/pattern/const_to_pat.rs +++ b/compiler/rustc_mir_build/src/thir/pattern/const_to_pat.rs @@ -351,10 +351,27 @@ impl<'a, 'tcx> ConstToPat<'a, 'tcx> { ty::Bool | ty::Char | ty::Int(_) | ty::Uint(_) | ty::FnDef(..) => { PatKind::Constant { value: cv } } + ty::RawPtr(pointee) if pointee.ty.is_sized(tcx.at(span), param_env) => { + PatKind::Constant { value: cv } + } // FIXME: these can have very suprising behaviour where optimization levels or other // compilation choices change the runtime behaviour of the match. // See https://github.com/rust-lang/rust/issues/70861 for examples. - ty::FnPtr(..) | ty::RawPtr(..) => PatKind::Constant { value: cv }, + ty::FnPtr(..) | ty::RawPtr(..) => { + if self.include_lint_checks && !self.saw_const_match_error.get() { + self.saw_const_match_error.set(true); + let msg = "function pointers and unsized pointers in patterns do not behave \ + deterministically. \ + See https://github.com/rust-lang/rust/issues/70861 for details."; + tcx.struct_span_lint_hir( + lint::builtin::POINTER_STRUCTURAL_MATCH, + id, + span, + |lint| lint.build(&msg).emit(), + ); + } + PatKind::Constant { value: cv } + } _ => { tcx.sess.delay_span_bug(span, &format!("cannot make a pattern out of {}", cv.ty)); PatKind::Wild diff --git a/compiler/rustc_session/src/lint/builtin.rs b/compiler/rustc_session/src/lint/builtin.rs index 562df176b14..c72b97fa1ca 100644 --- a/compiler/rustc_session/src/lint/builtin.rs +++ b/compiler/rustc_session/src/lint/builtin.rs @@ -2197,6 +2197,32 @@ declare_lint! { report_in_external_macro } +declare_lint! { + /// The `pointer_structural_match` lint detects pointers used in patterns that do not + /// behave deterministically across optimizations. + /// + /// ### Example + /// + /// ```rust,compile_fail + /// #![deny(pointer_structural_match)] + /// fn foo(a: usize, b: usize) -> usize { a + b } + /// const FOO: fn(usize, usize) -> usize = foo; + /// fn main() { + /// match FOO { + /// FOO => {}, + /// _ => {}, + /// } + /// } + /// ``` + pub POINTER_STRUCTURAL_MATCH, + Allow, + "pointers are not structural-match", + @future_incompatible = FutureIncompatibleInfo { + reference: "issue #62411 ", + edition: None, + }; +} + declare_lint! { /// The `ambiguous_associated_items` lint detects ambiguity between /// [associated items] and [enum variants]. @@ -2630,6 +2656,7 @@ declare_lint_pass! { AMBIGUOUS_ASSOCIATED_ITEMS, MUTABLE_BORROW_RESERVATION_CONFLICT, INDIRECT_STRUCTURAL_MATCH, + POINTER_STRUCTURAL_MATCH, SOFT_UNSTABLE, INLINE_NO_SANITIZE, ASM_SUB_REGISTER, diff --git a/src/test/ui/issues/issue-44333.rs b/src/test/ui/issues/issue-44333.rs index fffef975043..85f5ccbdb65 100644 --- a/src/test/ui/issues/issue-44333.rs +++ b/src/test/ui/issues/issue-44333.rs @@ -1,4 +1,7 @@ // run-pass + +#![warn(pointer_structural_match)] + type Func = fn(usize, usize) -> usize; fn foo(a: usize, b: usize) -> usize { a + b } @@ -13,8 +16,10 @@ const BAR: Func = bar; fn main() { match test(std::env::consts::ARCH.len()) { - FOO => println!("foo"), - BAR => println!("bar"), + FOO => println!("foo"), //~ WARN pointers in patterns do not behave deterministically + //~^ WARN will become a hard error + BAR => println!("bar"), //~ WARN pointers in patterns do not behave deterministically + //~^ WARN will become a hard error _ => unreachable!(), } } diff --git a/src/test/ui/issues/issue-44333.stderr b/src/test/ui/issues/issue-44333.stderr new file mode 100644 index 00000000000..a9ee5fc4dd7 --- /dev/null +++ b/src/test/ui/issues/issue-44333.stderr @@ -0,0 +1,25 @@ +warning: function pointers and unsized pointers in patterns do not behave deterministically. See https://github.com/rust-lang/rust/issues/70861 for details. + --> $DIR/issue-44333.rs:19:9 + | +LL | FOO => println!("foo"), + | ^^^ + | +note: the lint level is defined here + --> $DIR/issue-44333.rs:3:9 + | +LL | #![warn(pointer_structural_match)] + | ^^^^^^^^^^^^^^^^^^^^^^^^ + = 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 #62411 + +warning: function pointers and unsized pointers in patterns do not behave deterministically. See https://github.com/rust-lang/rust/issues/70861 for details. + --> $DIR/issue-44333.rs:21:9 + | +LL | BAR => println!("bar"), + | ^^^ + | + = 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 #62411 + +warning: 2 warnings emitted + diff --git a/src/test/ui/rfc1445/issue-63479-match-fnptr.rs b/src/test/ui/rfc1445/issue-63479-match-fnptr.rs index b3c91cec580..0984b1d480e 100644 --- a/src/test/ui/rfc1445/issue-63479-match-fnptr.rs +++ b/src/test/ui/rfc1445/issue-63479-match-fnptr.rs @@ -5,6 +5,8 @@ // cover the case this hit; I've since expanded it accordingly, but the // experience left me wary of leaving this regression test out.) +#![warn(pointer_structural_match)] + #[derive(Eq)] struct A { a: i64 @@ -31,6 +33,8 @@ fn main() { let s = B(my_fn); match s { B(TEST) => println!("matched"), + //~^ WARN pointers in patterns do not behave deterministically + //~| WARN this was previously accepted by the compiler but is being phased out _ => panic!("didn't match") }; } diff --git a/src/test/ui/rfc1445/issue-63479-match-fnptr.stderr b/src/test/ui/rfc1445/issue-63479-match-fnptr.stderr new file mode 100644 index 00000000000..34b9c359ca8 --- /dev/null +++ b/src/test/ui/rfc1445/issue-63479-match-fnptr.stderr @@ -0,0 +1,16 @@ +warning: function pointers and unsized pointers in patterns do not behave deterministically. See https://github.com/rust-lang/rust/issues/70861 for details. + --> $DIR/issue-63479-match-fnptr.rs:35:7 + | +LL | B(TEST) => println!("matched"), + | ^^^^ + | +note: the lint level is defined here + --> $DIR/issue-63479-match-fnptr.rs:8:9 + | +LL | #![warn(pointer_structural_match)] + | ^^^^^^^^^^^^^^^^^^^^^^^^ + = 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 #62411 + +warning: 1 warning emitted + From adf98ab2dc6b3d8332873d41f3371a839b4e9df1 Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Sun, 20 Sep 2020 17:22:33 +0200 Subject: [PATCH 06/21] Use precise errors during const to pat conversion instead of a catch-all on the main constant --- .../src/thir/pattern/const_to_pat.rs | 85 ++++++++++++++++--- compiler/rustc_session/src/lint/builtin.rs | 70 +++++++++++---- .../const_in_pattern/custom-eq-branch-warn.rs | 6 +- .../custom-eq-branch-warn.stderr | 12 +-- .../ui/consts/const_in_pattern/issue-65466.rs | 6 +- .../const_in_pattern/issue-65466.stderr | 15 ---- .../const_in_pattern/reject_non_partial_eq.rs | 1 + .../reject_non_partial_eq.stderr | 10 ++- .../const_in_pattern/warn_corner_cases.stderr | 18 ++-- src/test/ui/consts/match_ice.rs | 4 +- src/test/ui/consts/match_ice.stderr | 23 ++++- src/test/ui/issues/issue-34784.rs | 2 + src/test/ui/match/issue-70972-dyn-trait.rs | 3 +- .../ui/match/issue-70972-dyn-trait.stderr | 10 ++- ...opaquely-typed-constant-used-in-pattern.rs | 4 +- ...uely-typed-constant-used-in-pattern.stderr | 12 ++- ...-hide-behind-direct-unsafe-ptr-embedded.rs | 2 + ...low-hide-behind-direct-unsafe-ptr-param.rs | 2 + ...ide-behind-indirect-unsafe-ptr-embedded.rs | 2 + ...w-hide-behind-indirect-unsafe-ptr-param.rs | 2 + ...ide-behind-doubly-indirect-embedded.stderr | 10 +-- ...t-hide-behind-doubly-indirect-param.stderr | 10 +-- ...2307-match-ref-ref-forbidden-without-eq.rs | 2 +- ...-match-ref-ref-forbidden-without-eq.stderr | 14 +-- .../structural-match-no-leak.rs | 5 +- .../structural-match-no-leak.stderr | 10 ++- .../type-alias-impl-trait/structural-match.rs | 5 +- .../structural-match.stderr | 10 ++- 28 files changed, 240 insertions(+), 115 deletions(-) delete mode 100644 src/test/ui/consts/const_in_pattern/issue-65466.stderr diff --git a/compiler/rustc_mir_build/src/thir/pattern/const_to_pat.rs b/compiler/rustc_mir_build/src/thir/pattern/const_to_pat.rs index 6ca1ff2c5f2..ad0bcfeb367 100644 --- a/compiler/rustc_mir_build/src/thir/pattern/const_to_pat.rs +++ b/compiler/rustc_mir_build/src/thir/pattern/const_to_pat.rs @@ -43,7 +43,7 @@ struct ConstToPat<'a, 'tcx> { span: Span, param_env: ty::ParamEnv<'tcx>, - // This tracks if we signal some hard error for a given const value, so that + // This tracks if we saw some error or lint 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, @@ -103,7 +103,7 @@ impl<'a, 'tcx> ConstToPat<'a, 'tcx> { // once indirect_structural_match is a full fledged error, this // level of indirection can be eliminated - let inlined_const_as_pat = self.recur(cv); + let inlined_const_as_pat = self.recur(cv, mir_structural_match_violation); if self.include_lint_checks && !self.saw_const_match_error.get() { // If we were able to successfully convert the const to some pat, @@ -216,7 +216,7 @@ impl<'a, 'tcx> ConstToPat<'a, 'tcx> { } // Recursive helper for `to_pat`; invoke that (instead of calling this directly). - fn recur(&self, cv: &'tcx ty::Const<'tcx>) -> Pat<'tcx> { + fn recur(&self, cv: &'tcx ty::Const<'tcx>, mir_structural_match_violation: bool) -> Pat<'tcx> { let id = self.id; let span = self.span; let tcx = self.tcx(); @@ -227,7 +227,7 @@ impl<'a, 'tcx> ConstToPat<'a, 'tcx> { .enumerate() .map(|(idx, val)| { let field = Field::new(idx); - FieldPat { field, pattern: self.recur(val) } + FieldPat { field, pattern: self.recur(val, false) } }) .collect() }; @@ -248,6 +248,21 @@ impl<'a, 'tcx> ConstToPat<'a, 'tcx> { tcx.sess.span_err(span, "cannot use unions in constant patterns"); PatKind::Wild } + ty::Adt(..) + if !self.type_has_partial_eq_impl(cv.ty) + // 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() => + { + let msg = format!( + "to use a constant of type `{}` in a pattern, \ + `{}` must be annotated with `#[derive(PartialEq, Eq)]`", + cv.ty, cv.ty, + ); + self.saw_const_match_error.set(true); + self.tcx().sess.span_err(self.span, &msg); + PatKind::Wild + } // If the type is not structurally comparable, just emit the constant directly, // causing the pattern match code to treat it opaquely. // FIXME: This code doesn't emit errors itself, the caller emits the errors. @@ -258,6 +273,20 @@ 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); + let msg = format!( + "to use a constant of type `{}` in a pattern, \ + `{}` must be annotated with `#[derive(PartialEq, Eq)]`", + cv.ty, cv.ty, + ); + tcx.struct_span_lint_hir( + lint::builtin::INDIRECT_STRUCTURAL_MATCH, + id, + span, + |lint| lint.build(&msg).emit(), + ); + } PatKind::Constant { value: cv } } ty::Adt(adt_def, _) if !self.type_marked_structural(cv.ty) => { @@ -292,14 +321,18 @@ impl<'a, 'tcx> ConstToPat<'a, 'tcx> { .destructure_const(param_env.and(cv)) .fields .iter() - .map(|val| self.recur(val)) + .map(|val| self.recur(val, false)) .collect(), slice: None, suffix: Vec::new(), }, ty::Ref(_, pointee_ty, ..) => match *pointee_ty.kind() { // These are not allowed and will error elsewhere anyway. - ty::Dynamic(..) => PatKind::Constant { value: cv }, + ty::Dynamic(..) => { + self.saw_const_match_error.set(true); + tcx.sess.span_err(span, &format!("`{}` cannot be used in patterns", cv.ty)); + PatKind::Wild + } // `&str` and `&[u8]` are represented as `ConstValue::Slice`, let's keep using this // optimization for now. ty::Str => PatKind::Constant { value: cv }, @@ -321,7 +354,7 @@ impl<'a, 'tcx> ConstToPat<'a, 'tcx> { .destructure_const(param_env.and(array)) .fields .iter() - .map(|val| self.recur(val)) + .map(|val| self.recur(val, false)) .collect(), slice: None, suffix: vec![], @@ -333,16 +366,21 @@ impl<'a, 'tcx> ConstToPat<'a, 'tcx> { self.behind_reference.set(old); val } - // Backwards compatibility hack. Don't take away the reference, since - // `PartialEq::eq` takes a reference, this makes the rest of the matching logic - // simpler. + // Backwards compatibility hack: support references to non-structural types. + // We'll lower + // this pattern to a `PartialEq::eq` comparison and `PartialEq::eq` takes a + // reference. This makes the rest of the matching logic simpler as it doesn't have + // to figure out how to get a reference again. ty::Adt(..) if !self.type_marked_structural(pointee_ty) => { PatKind::Constant { value: cv } } + // All other references are converted into deref patterns and then recursively + // convert the dereferenced constant to a pattern that is the sub-pattern of the + // deref pattern. _ => { let old = self.behind_reference.replace(true); let val = PatKind::Deref { - subpattern: self.recur(tcx.deref_const(self.param_env.and(cv))), + subpattern: self.recur(tcx.deref_const(self.param_env.and(cv)), false), }; self.behind_reference.set(old); val @@ -373,11 +411,34 @@ impl<'a, 'tcx> ConstToPat<'a, 'tcx> { PatKind::Constant { value: cv } } _ => { - tcx.sess.delay_span_bug(span, &format!("cannot make a pattern out of {}", cv.ty)); + self.saw_const_match_error.set(true); + tcx.sess.span_err(span, &format!("`{}` cannot be used in patterns", cv.ty)); PatKind::Wild } }; + if self.include_lint_checks + && !self.saw_const_match_error.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); + let msg = format!( + "to use a constant of type `{}` in a pattern, \ + the constant's initializer must be trivial or all types \ + in the constant must be annotated with `#[derive(PartialEq, Eq)]`", + cv.ty, + ); + tcx.struct_span_lint_hir( + lint::builtin::NONTRIVIAL_STRUCTURAL_MATCH, + id, + span, + |lint| lint.build(&msg).emit(), + ); + } + Pat { span, ty: cv.ty, kind: Box::new(kind) } } } diff --git a/compiler/rustc_session/src/lint/builtin.rs b/compiler/rustc_session/src/lint/builtin.rs index c72b97fa1ca..919aaf81ed2 100644 --- a/compiler/rustc_session/src/lint/builtin.rs +++ b/compiler/rustc_session/src/lint/builtin.rs @@ -2138,22 +2138,16 @@ declare_lint! { /// ```rust,compile_fail /// #![deny(indirect_structural_match)] /// - /// struct Plus(i32, i32); - /// const ONE_PLUS_TWO: &&Plus = &&Plus(1, 2); - /// - /// impl PartialEq for Plus { - /// fn eq(&self, other: &Self) -> bool { - /// self.0 + self.1 == other.0 + other.1 - /// } - /// } - /// - /// impl Eq for Plus {} - /// + /// struct NoDerive(i32); + /// impl PartialEq for NoDerive { fn eq(&self, _: &Self) -> bool { false } } + /// impl Eq for NoDerive { } + /// #[derive(PartialEq, Eq)] + /// struct WrapParam(T); + /// const WRAP_INDIRECT_PARAM: & &WrapParam = & &WrapParam(NoDerive(0)); /// fn main() { - /// if let ONE_PLUS_TWO = &&Plus(3, 0) { - /// println!("semantic!"); - /// } else { - /// println!("structural!"); + /// match WRAP_INDIRECT_PARAM { + /// WRAP_INDIRECT_PARAM => { } + /// _ => { } /// } /// } /// ``` @@ -2170,9 +2164,8 @@ declare_lint! { /// [issue #62411]: https://github.com/rust-lang/rust/issues/62411 /// [future-incompatible]: ../index.md#future-incompatible-lints pub INDIRECT_STRUCTURAL_MATCH, - // defaulting to allow until rust-lang/rust#62614 is fixed. - Allow, - "pattern with const indirectly referencing non-structural-match type", + Warn, + "constant used in pattern contains value of non-structural-match type in a field or a variant", @future_incompatible = FutureIncompatibleInfo { reference: "issue #62411 ", edition: None, @@ -2223,6 +2216,46 @@ declare_lint! { }; } +declare_lint! { + /// The `nontrivial_structural_match` lint detects constants that are used in patterns, + /// whose type is not structural-match and whose initializer body actually uses values + /// that are not structural-match. So `Option` is ok if the constant + /// is just `None`. + /// + /// ### Example + /// + /// ```rust,compile_fail + /// #![deny(nontrivial_structural_match)] + /// + /// struct Plus(i32, i32); + /// const ONE_PLUS_TWO: &&Plus = &&Plus(1, 2); + /// + /// impl PartialEq for Plus { + /// fn eq(&self, other: &Self) -> bool { + /// self.0 + self.1 == other.0 + other.1 + /// } + /// } + /// + /// impl Eq for Plus {} + /// + /// fn main() { + /// if let ONE_PLUS_TWO = &&Plus(3, 0) { + /// println!("semantic!"); + /// } else { + /// println!("structural!"); + /// } + /// } + /// ``` + pub NONTRIVIAL_STRUCTURAL_MATCH, + Warn, + "constant used in pattern of non-structural-match type and the constant's initializer \ + expression contains values of non-structural-match types", + @future_incompatible = FutureIncompatibleInfo { + reference: "issue #73448 ", + edition: None, + }; +} + declare_lint! { /// The `ambiguous_associated_items` lint detects ambiguity between /// [associated items] and [enum variants]. @@ -2657,6 +2690,7 @@ declare_lint_pass! { MUTABLE_BORROW_RESERVATION_CONFLICT, INDIRECT_STRUCTURAL_MATCH, POINTER_STRUCTURAL_MATCH, + NONTRIVIAL_STRUCTURAL_MATCH, SOFT_UNSTABLE, INLINE_NO_SANITIZE, ASM_SUB_REGISTER, diff --git a/src/test/ui/consts/const_in_pattern/custom-eq-branch-warn.rs b/src/test/ui/consts/const_in_pattern/custom-eq-branch-warn.rs index a1f9838ca08..856d204178d 100644 --- a/src/test/ui/consts/const_in_pattern/custom-eq-branch-warn.rs +++ b/src/test/ui/consts/const_in_pattern/custom-eq-branch-warn.rs @@ -1,8 +1,5 @@ // check-pass -#![warn(indirect_structural_match)] -//~^ NOTE lint level is defined here - struct CustomEq; impl Eq for CustomEq {} @@ -32,7 +29,8 @@ fn main() { BAR_BAZ => panic!(), //~^ WARN must be annotated with `#[derive(PartialEq, Eq)]` //~| WARN this was previously accepted - //~| NOTE see issue #62411 + //~| NOTE see issue #73448 + //~| NOTE `#[warn(nontrivial_structural_match)]` on by default _ => {} } } diff --git a/src/test/ui/consts/const_in_pattern/custom-eq-branch-warn.stderr b/src/test/ui/consts/const_in_pattern/custom-eq-branch-warn.stderr index 0be1cca806e..fd6732195e4 100644 --- a/src/test/ui/consts/const_in_pattern/custom-eq-branch-warn.stderr +++ b/src/test/ui/consts/const_in_pattern/custom-eq-branch-warn.stderr @@ -1,16 +1,12 @@ -warning: to use a constant of type `CustomEq` in a pattern, `CustomEq` must be annotated with `#[derive(PartialEq, Eq)]` - --> $DIR/custom-eq-branch-warn.rs:32:9 +warning: to use a constant of type `Foo` in a pattern, the constant's initializer must be trivial or all types in the constant must be annotated with `#[derive(PartialEq, Eq)]` + --> $DIR/custom-eq-branch-warn.rs:29:9 | LL | BAR_BAZ => panic!(), | ^^^^^^^ | -note: the lint level is defined here - --> $DIR/custom-eq-branch-warn.rs:3:9 - | -LL | #![warn(indirect_structural_match)] - | ^^^^^^^^^^^^^^^^^^^^^^^^^ + = note: `#[warn(nontrivial_structural_match)]` on by default = 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 #62411 + = note: for more information, see issue #73448 warning: 1 warning emitted diff --git a/src/test/ui/consts/const_in_pattern/issue-65466.rs b/src/test/ui/consts/const_in_pattern/issue-65466.rs index 0e3e0f6dd88..2b421f4c705 100644 --- a/src/test/ui/consts/const_in_pattern/issue-65466.rs +++ b/src/test/ui/consts/const_in_pattern/issue-65466.rs @@ -1,9 +1,7 @@ -// FIXME: This still ICEs. -// -// ignore-test - #![deny(indirect_structural_match)] +// check-pass + #[derive(PartialEq, Eq)] enum O { Some(*const T), // Can also use PhantomData diff --git a/src/test/ui/consts/const_in_pattern/issue-65466.stderr b/src/test/ui/consts/const_in_pattern/issue-65466.stderr deleted file mode 100644 index 9fe3049d1d8..00000000000 --- a/src/test/ui/consts/const_in_pattern/issue-65466.stderr +++ /dev/null @@ -1,15 +0,0 @@ -error[E0601]: `main` function not found in crate `issue_65466` - --> $DIR/issue-65466.rs:1:1 - | -LL | / #![deny(indirect_structural_match)] -LL | | -LL | | #[derive(PartialEq, Eq)] -LL | | enum O { -... | -LL | | } -LL | | } - | |_^ consider adding a `main` function to `$DIR/issue-65466.rs` - -error: aborting due to previous error - -For more information about this error, try `rustc --explain E0601`. diff --git a/src/test/ui/consts/const_in_pattern/reject_non_partial_eq.rs b/src/test/ui/consts/const_in_pattern/reject_non_partial_eq.rs index a8216901c02..b2b2daa830f 100644 --- a/src/test/ui/consts/const_in_pattern/reject_non_partial_eq.rs +++ b/src/test/ui/consts/const_in_pattern/reject_non_partial_eq.rs @@ -27,6 +27,7 @@ fn main() { match None { NO_PARTIAL_EQ_NONE => println!("NO_PARTIAL_EQ_NONE"), //~^ ERROR must be annotated with `#[derive(PartialEq, Eq)]` + //~| ERROR must be annotated with `#[derive(PartialEq, Eq)]` _ => panic!("whoops"), } } diff --git a/src/test/ui/consts/const_in_pattern/reject_non_partial_eq.stderr b/src/test/ui/consts/const_in_pattern/reject_non_partial_eq.stderr index 95cfa4a9ebe..dc830d36003 100644 --- a/src/test/ui/consts/const_in_pattern/reject_non_partial_eq.stderr +++ b/src/test/ui/consts/const_in_pattern/reject_non_partial_eq.stderr @@ -1,8 +1,14 @@ -error: to use a constant of type `NoPartialEq` in a pattern, `NoPartialEq` must be annotated with `#[derive(PartialEq, Eq)]` +error: to use a constant of type `Option` in a pattern, `Option` must be annotated with `#[derive(PartialEq, Eq)]` --> $DIR/reject_non_partial_eq.rs:28:9 | LL | NO_PARTIAL_EQ_NONE => println!("NO_PARTIAL_EQ_NONE"), | ^^^^^^^^^^^^^^^^^^ -error: aborting due to previous error +error: to use a constant of type `Option` in a pattern, `Option` must be annotated with `#[derive(PartialEq, Eq)]` + --> $DIR/reject_non_partial_eq.rs:28:9 + | +LL | NO_PARTIAL_EQ_NONE => println!("NO_PARTIAL_EQ_NONE"), + | ^^^^^^^^^^^^^^^^^^ + +error: aborting due to 2 previous errors diff --git a/src/test/ui/consts/const_in_pattern/warn_corner_cases.stderr b/src/test/ui/consts/const_in_pattern/warn_corner_cases.stderr index 3e7ed573c74..a4feaff55b2 100644 --- a/src/test/ui/consts/const_in_pattern/warn_corner_cases.stderr +++ b/src/test/ui/consts/const_in_pattern/warn_corner_cases.stderr @@ -1,34 +1,30 @@ -warning: to use a constant of type `NoDerive` in a pattern, `NoDerive` must be annotated with `#[derive(PartialEq, Eq)]` +warning: to use a constant of type `Option` in a pattern, the constant's initializer must be trivial or all types in the constant must be annotated with `#[derive(PartialEq, Eq)]` --> $DIR/warn_corner_cases.rs:26:47 | LL | match None { Some(_) => panic!("whoops"), INDEX => dbg!(INDEX), }; | ^^^^^ | -note: the lint level is defined here - --> $DIR/warn_corner_cases.rs:15:9 - | -LL | #![warn(indirect_structural_match)] - | ^^^^^^^^^^^^^^^^^^^^^^^^^ + = note: `#[warn(nontrivial_structural_match)]` on by default = 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 #62411 + = note: for more information, see issue #73448 -warning: to use a constant of type `NoDerive` in a pattern, `NoDerive` must be annotated with `#[derive(PartialEq, Eq)]` +warning: to use a constant of type `Option` in a pattern, the constant's initializer must be trivial or all types in the constant must be annotated with `#[derive(PartialEq, Eq)]` --> $DIR/warn_corner_cases.rs:32:47 | LL | match None { Some(_) => panic!("whoops"), CALL => dbg!(CALL), }; | ^^^^ | = 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 #62411 + = note: for more information, see issue #73448 -warning: to use a constant of type `NoDerive` in a pattern, `NoDerive` must be annotated with `#[derive(PartialEq, Eq)]` +warning: to use a constant of type `Option` in a pattern, the constant's initializer must be trivial or all types in the constant must be annotated with `#[derive(PartialEq, Eq)]` --> $DIR/warn_corner_cases.rs:38:47 | LL | match None { Some(_) => panic!("whoops"), METHOD_CALL => dbg!(METHOD_CALL), }; | ^^^^^^^^^^^ | = 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 #62411 + = note: for more information, see issue #73448 warning: 3 warnings emitted diff --git a/src/test/ui/consts/match_ice.rs b/src/test/ui/consts/match_ice.rs index 008c03ecddc..db76e230070 100644 --- a/src/test/ui/consts/match_ice.rs +++ b/src/test/ui/consts/match_ice.rs @@ -8,8 +8,10 @@ struct T; fn main() { const C: &S = &S; match C { + //~^ non-exhaustive patterns: `&S` not covered C => {} - //~^ ERROR to use a constant of type `S` in a pattern, `S` must be annotated with + //~^ WARN must be annotated with `#[derive(PartialEq, Eq)]` + //~| WARN was previously accepted by the compiler } const K: &T = &T; match K { diff --git a/src/test/ui/consts/match_ice.stderr b/src/test/ui/consts/match_ice.stderr index 699b4a5e200..6cc79dbca7c 100644 --- a/src/test/ui/consts/match_ice.stderr +++ b/src/test/ui/consts/match_ice.stderr @@ -1,8 +1,25 @@ -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 +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 | LL | C => {} | ^ + | + = note: `#[warn(nontrivial_structural_match)]` on by default + = 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 -error: aborting due to previous error +error[E0004]: non-exhaustive patterns: `&S` not covered + --> $DIR/match_ice.rs:10:11 + | +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` +error: aborting due to previous error; 1 warning emitted + +For more information about this error, try `rustc --explain E0004`. diff --git a/src/test/ui/issues/issue-34784.rs b/src/test/ui/issues/issue-34784.rs index d3206e99430..98d943470a7 100644 --- a/src/test/ui/issues/issue-34784.rs +++ b/src/test/ui/issues/issue-34784.rs @@ -1,4 +1,6 @@ // run-pass + +#![warn(pointer_structural_match)] #![allow(dead_code)] const C: *const u8 = &0; diff --git a/src/test/ui/match/issue-70972-dyn-trait.rs b/src/test/ui/match/issue-70972-dyn-trait.rs index a9b2699cafd..b69e2dab265 100644 --- a/src/test/ui/match/issue-70972-dyn-trait.rs +++ b/src/test/ui/match/issue-70972-dyn-trait.rs @@ -4,7 +4,8 @@ fn main() { let a: &dyn Send = &7u32; match a { F => panic!(), - //~^ ERROR trait objects cannot be used in patterns + //~^ ERROR `&dyn Send` cannot be used in patterns + //~| ERROR `&dyn Send` cannot be used in patterns _ => {} } } diff --git a/src/test/ui/match/issue-70972-dyn-trait.stderr b/src/test/ui/match/issue-70972-dyn-trait.stderr index a4e827357de..985799b3c83 100644 --- a/src/test/ui/match/issue-70972-dyn-trait.stderr +++ b/src/test/ui/match/issue-70972-dyn-trait.stderr @@ -1,8 +1,14 @@ -error: trait objects cannot be used in patterns +error: `&dyn Send` cannot be used in patterns --> $DIR/issue-70972-dyn-trait.rs:6:9 | LL | F => panic!(), | ^ -error: aborting due to previous error +error: `&dyn Send` cannot be used in patterns + --> $DIR/issue-70972-dyn-trait.rs:6:9 + | +LL | F => panic!(), + | ^ + +error: aborting due to 2 previous errors diff --git a/src/test/ui/pattern/issue-71042-opaquely-typed-constant-used-in-pattern.rs b/src/test/ui/pattern/issue-71042-opaquely-typed-constant-used-in-pattern.rs index c5e4a72fb9f..0c38b533a16 100644 --- a/src/test/ui/pattern/issue-71042-opaquely-typed-constant-used-in-pattern.rs +++ b/src/test/ui/pattern/issue-71042-opaquely-typed-constant-used-in-pattern.rs @@ -4,6 +4,8 @@ fn main() { const C: impl Copy = 0; match C { - C | _ => {} //~ ERROR: opaque types cannot be used in patterns + C => {} //~ ERROR: `impl Copy` cannot be used in patterns + //~^ ERROR: `impl Copy` cannot be used in patterns + _ => {} } } diff --git a/src/test/ui/pattern/issue-71042-opaquely-typed-constant-used-in-pattern.stderr b/src/test/ui/pattern/issue-71042-opaquely-typed-constant-used-in-pattern.stderr index 7695223f2cf..ad6cc0aa3e3 100644 --- a/src/test/ui/pattern/issue-71042-opaquely-typed-constant-used-in-pattern.stderr +++ b/src/test/ui/pattern/issue-71042-opaquely-typed-constant-used-in-pattern.stderr @@ -1,8 +1,14 @@ -error: opaque types cannot be used in patterns +error: `impl Copy` cannot be used in patterns --> $DIR/issue-71042-opaquely-typed-constant-used-in-pattern.rs:7:9 | -LL | C | _ => {} +LL | C => {} | ^ -error: aborting due to previous error +error: `impl Copy` cannot be used in patterns + --> $DIR/issue-71042-opaquely-typed-constant-used-in-pattern.rs:7:9 + | +LL | C => {} + | ^ + +error: aborting due to 2 previous errors diff --git a/src/test/ui/rfc1445/allow-hide-behind-direct-unsafe-ptr-embedded.rs b/src/test/ui/rfc1445/allow-hide-behind-direct-unsafe-ptr-embedded.rs index b90a750cc16..c6d7166e740 100644 --- a/src/test/ui/rfc1445/allow-hide-behind-direct-unsafe-ptr-embedded.rs +++ b/src/test/ui/rfc1445/allow-hide-behind-direct-unsafe-ptr-embedded.rs @@ -3,6 +3,8 @@ // run-pass +#![warn(pointer_structural_match)] + struct NoDerive(i32); // This impl makes NoDerive irreflexive diff --git a/src/test/ui/rfc1445/allow-hide-behind-direct-unsafe-ptr-param.rs b/src/test/ui/rfc1445/allow-hide-behind-direct-unsafe-ptr-param.rs index 1076b9f25d8..cc7ea6cde8d 100644 --- a/src/test/ui/rfc1445/allow-hide-behind-direct-unsafe-ptr-param.rs +++ b/src/test/ui/rfc1445/allow-hide-behind-direct-unsafe-ptr-param.rs @@ -3,6 +3,8 @@ // run-pass +#![warn(pointer_structural_match)] + struct NoDerive(i32); // This impl makes NoDerive irreflexive diff --git a/src/test/ui/rfc1445/allow-hide-behind-indirect-unsafe-ptr-embedded.rs b/src/test/ui/rfc1445/allow-hide-behind-indirect-unsafe-ptr-embedded.rs index a4b832d377d..86db09cc08f 100644 --- a/src/test/ui/rfc1445/allow-hide-behind-indirect-unsafe-ptr-embedded.rs +++ b/src/test/ui/rfc1445/allow-hide-behind-indirect-unsafe-ptr-embedded.rs @@ -3,6 +3,8 @@ // run-pass +#![warn(pointer_structural_match)] + struct NoDerive(i32); // This impl makes NoDerive irreflexive diff --git a/src/test/ui/rfc1445/allow-hide-behind-indirect-unsafe-ptr-param.rs b/src/test/ui/rfc1445/allow-hide-behind-indirect-unsafe-ptr-param.rs index 47b70e2e9cc..99c574d0780 100644 --- a/src/test/ui/rfc1445/allow-hide-behind-indirect-unsafe-ptr-param.rs +++ b/src/test/ui/rfc1445/allow-hide-behind-indirect-unsafe-ptr-param.rs @@ -3,6 +3,8 @@ // run-pass +#![warn(pointer_structural_match)] + struct NoDerive(i32); // This impl makes NoDerive irreflexive diff --git a/src/test/ui/rfc1445/cant-hide-behind-doubly-indirect-embedded.stderr b/src/test/ui/rfc1445/cant-hide-behind-doubly-indirect-embedded.stderr index 659a9812672..eb13eed6ec1 100644 --- a/src/test/ui/rfc1445/cant-hide-behind-doubly-indirect-embedded.stderr +++ b/src/test/ui/rfc1445/cant-hide-behind-doubly-indirect-embedded.stderr @@ -1,16 +1,12 @@ -warning: to use a constant of type `NoDerive` in a pattern, `NoDerive` must be annotated with `#[derive(PartialEq, Eq)]` +warning: to use a constant of type `&&WrapInline` in a pattern, the constant's initializer must be trivial or all types in the constant must be annotated with `#[derive(PartialEq, Eq)]` --> $DIR/cant-hide-behind-doubly-indirect-embedded.rs:24:9 | LL | WRAP_DOUBLY_INDIRECT_INLINE => { panic!("WRAP_DOUBLY_INDIRECT_INLINE matched itself"); } | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ | -note: the lint level is defined here - --> $DIR/cant-hide-behind-doubly-indirect-embedded.rs:7:9 - | -LL | #![warn(indirect_structural_match)] - | ^^^^^^^^^^^^^^^^^^^^^^^^^ + = note: `#[warn(nontrivial_structural_match)]` on by default = 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 #62411 + = note: for more information, see issue #73448 warning: 1 warning emitted diff --git a/src/test/ui/rfc1445/cant-hide-behind-doubly-indirect-param.stderr b/src/test/ui/rfc1445/cant-hide-behind-doubly-indirect-param.stderr index c8c36510542..ddee99e76bb 100644 --- a/src/test/ui/rfc1445/cant-hide-behind-doubly-indirect-param.stderr +++ b/src/test/ui/rfc1445/cant-hide-behind-doubly-indirect-param.stderr @@ -1,16 +1,12 @@ -warning: to use a constant of type `NoDerive` in a pattern, `NoDerive` must be annotated with `#[derive(PartialEq, Eq)]` +warning: to use a constant of type `&&WrapParam` in a pattern, the constant's initializer must be trivial or all types in the constant must be annotated with `#[derive(PartialEq, Eq)]` --> $DIR/cant-hide-behind-doubly-indirect-param.rs:24:9 | LL | WRAP_DOUBLY_INDIRECT_PARAM => { panic!("WRAP_DOUBLY_INDIRECT_PARAM matched itself"); } | ^^^^^^^^^^^^^^^^^^^^^^^^^^ | -note: the lint level is defined here - --> $DIR/cant-hide-behind-doubly-indirect-param.rs:7:9 - | -LL | #![warn(indirect_structural_match)] - | ^^^^^^^^^^^^^^^^^^^^^^^^^ + = note: `#[warn(nontrivial_structural_match)]` on by default = 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 #62411 + = note: for more information, see issue #73448 warning: 1 warning emitted diff --git a/src/test/ui/rfc1445/issue-62307-match-ref-ref-forbidden-without-eq.rs b/src/test/ui/rfc1445/issue-62307-match-ref-ref-forbidden-without-eq.rs index 6ebb948d736..b5e19611da8 100644 --- a/src/test/ui/rfc1445/issue-62307-match-ref-ref-forbidden-without-eq.rs +++ b/src/test/ui/rfc1445/issue-62307-match-ref-ref-forbidden-without-eq.rs @@ -10,7 +10,7 @@ // Issue 62307 pointed out a case where the structural-match checking // was too shallow. -#![warn(indirect_structural_match)] +#![warn(indirect_structural_match, nontrivial_structural_match)] // run-pass #[derive(Debug)] diff --git a/src/test/ui/rfc1445/issue-62307-match-ref-ref-forbidden-without-eq.stderr b/src/test/ui/rfc1445/issue-62307-match-ref-ref-forbidden-without-eq.stderr index ae011dfcdba..7f4b4923332 100644 --- a/src/test/ui/rfc1445/issue-62307-match-ref-ref-forbidden-without-eq.stderr +++ b/src/test/ui/rfc1445/issue-62307-match-ref-ref-forbidden-without-eq.stderr @@ -1,25 +1,25 @@ -warning: to use a constant of type `B` in a pattern, `B` must be annotated with `#[derive(PartialEq, Eq)]` +warning: to use a constant of type `&&B` in a pattern, the constant's initializer must be trivial or all types in the constant must be annotated with `#[derive(PartialEq, Eq)]` --> $DIR/issue-62307-match-ref-ref-forbidden-without-eq.rs:31:9 | LL | RR_B1 => { println!("CLAIM RR0: {:?} matches {:?}", RR_B1, RR_B0); } | ^^^^^ | note: the lint level is defined here - --> $DIR/issue-62307-match-ref-ref-forbidden-without-eq.rs:13:9 + --> $DIR/issue-62307-match-ref-ref-forbidden-without-eq.rs:13:36 | -LL | #![warn(indirect_structural_match)] - | ^^^^^^^^^^^^^^^^^^^^^^^^^ +LL | #![warn(indirect_structural_match, nontrivial_structural_match)] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ = 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 #62411 + = note: for more information, see issue #73448 -warning: to use a constant of type `B` in a pattern, `B` must be annotated with `#[derive(PartialEq, Eq)]` +warning: to use a constant of type `&&B` in a pattern, the constant's initializer must be trivial or all types in the constant must be annotated with `#[derive(PartialEq, Eq)]` --> $DIR/issue-62307-match-ref-ref-forbidden-without-eq.rs:38:9 | LL | RR_B1 => { println!("CLAIM RR1: {:?} matches {:?}", RR_B1, RR_B1); } | ^^^^^ | = 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 #62411 + = note: for more information, see issue #73448 warning: 2 warnings emitted diff --git a/src/test/ui/type-alias-impl-trait/structural-match-no-leak.rs b/src/test/ui/type-alias-impl-trait/structural-match-no-leak.rs index 479d6cd9af7..54fc4956b9b 100644 --- a/src/test/ui/type-alias-impl-trait/structural-match-no-leak.rs +++ b/src/test/ui/type-alias-impl-trait/structural-match-no-leak.rs @@ -12,9 +12,10 @@ const LEAK_FREE: Bar = leak_free(); fn leak_free_test() { match todo!() { LEAK_FREE => (), - //~^ opaque types cannot be used in patterns + //~^ `impl Send` cannot be used in patterns + //~| `impl Send` cannot be used in patterns _ => (), } } -fn main() { } +fn main() {} diff --git a/src/test/ui/type-alias-impl-trait/structural-match-no-leak.stderr b/src/test/ui/type-alias-impl-trait/structural-match-no-leak.stderr index ae0d8e8d423..90b44f6598d 100644 --- a/src/test/ui/type-alias-impl-trait/structural-match-no-leak.stderr +++ b/src/test/ui/type-alias-impl-trait/structural-match-no-leak.stderr @@ -1,8 +1,14 @@ -error: opaque types cannot be used in patterns +error: `impl Send` cannot be used in patterns --> $DIR/structural-match-no-leak.rs:14:9 | LL | LEAK_FREE => (), | ^^^^^^^^^ -error: aborting due to previous error +error: `impl Send` cannot be used in patterns + --> $DIR/structural-match-no-leak.rs:14:9 + | +LL | LEAK_FREE => (), + | ^^^^^^^^^ + +error: aborting due to 2 previous errors diff --git a/src/test/ui/type-alias-impl-trait/structural-match.rs b/src/test/ui/type-alias-impl-trait/structural-match.rs index 481448d64b1..5fe5bb4bdea 100644 --- a/src/test/ui/type-alias-impl-trait/structural-match.rs +++ b/src/test/ui/type-alias-impl-trait/structural-match.rs @@ -13,9 +13,10 @@ const VALUE: Foo = value(); fn test() { match todo!() { VALUE => (), - //~^ opaque types cannot be used in patterns + //~^ `impl Send` cannot be used in patterns + //~| `impl Send` cannot be used in patterns _ => (), } } -fn main() { } +fn main() {} diff --git a/src/test/ui/type-alias-impl-trait/structural-match.stderr b/src/test/ui/type-alias-impl-trait/structural-match.stderr index ad9036a87d1..7aca3ba8640 100644 --- a/src/test/ui/type-alias-impl-trait/structural-match.stderr +++ b/src/test/ui/type-alias-impl-trait/structural-match.stderr @@ -1,8 +1,14 @@ -error: opaque types cannot be used in patterns +error: `impl Send` cannot be used in patterns --> $DIR/structural-match.rs:15:9 | LL | VALUE => (), | ^^^^^ -error: aborting due to previous error +error: `impl Send` cannot be used in patterns + --> $DIR/structural-match.rs:15:9 + | +LL | VALUE => (), + | ^^^^^ + +error: aborting due to 2 previous errors From f7eceef653c791ee9f5eef7728c50295d6808e14 Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Mon, 21 Sep 2020 14:27:14 +0200 Subject: [PATCH 07/21] Document future incompat lints --- compiler/rustc_session/src/lint/builtin.rs | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/compiler/rustc_session/src/lint/builtin.rs b/compiler/rustc_session/src/lint/builtin.rs index 919aaf81ed2..966b8f7e5f3 100644 --- a/compiler/rustc_session/src/lint/builtin.rs +++ b/compiler/rustc_session/src/lint/builtin.rs @@ -2207,6 +2207,16 @@ declare_lint! { /// } /// } /// ``` + /// + /// ### Explanation + /// + /// Previous versions of Rust allowed function pointers and wide raw pointers in patterns. + /// While these work in many cases as expected by users, it is possible that due to + /// optimizations pointers are "not equal to themselves" or pointers to different functions + /// compare as equal during runtime. This is because LLVM optimizations can deduplicate + /// functions if their bodies are the same, thus also making pointers to these functions point + /// to the same location. Additionally functions may get duplicated if they are instantiated + /// in different crates and not deduplicated again via LTO. pub POINTER_STRUCTURAL_MATCH, Allow, "pointers are not structural-match", @@ -2246,6 +2256,13 @@ declare_lint! { /// } /// } /// ``` + /// + /// ### Explanation + /// + /// Previous versions of Rust accepted constants in patterns, even if those constants's types + /// did not have `PartialEq` derived. Thus the compiler falls back to runtime execution of + /// `PartialEq`, which can report that two constants are not equal even if they are + /// bit-equivalent. pub NONTRIVIAL_STRUCTURAL_MATCH, Warn, "constant used in pattern of non-structural-match type and the constant's initializer \ From f12583a3302df829d2cf55f25303a74531067a97 Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Mon, 21 Sep 2020 16:38:29 +0200 Subject: [PATCH 08/21] Pacify tidy --- compiler/rustc_session/src/lint/builtin.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/compiler/rustc_session/src/lint/builtin.rs b/compiler/rustc_session/src/lint/builtin.rs index 966b8f7e5f3..b39e01a009e 100644 --- a/compiler/rustc_session/src/lint/builtin.rs +++ b/compiler/rustc_session/src/lint/builtin.rs @@ -2208,6 +2208,8 @@ declare_lint! { /// } /// ``` /// + /// {{produces}} + /// /// ### Explanation /// /// Previous versions of Rust allowed function pointers and wide raw pointers in patterns. @@ -2257,6 +2259,8 @@ declare_lint! { /// } /// ``` /// + /// {{produces}} + /// /// ### Explanation /// /// Previous versions of Rust accepted constants in patterns, even if those constants's types From 1b1b6eabaa9ea0090dd47e642b6bb6606cc52e8c Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Wed, 23 Sep 2020 16:28:45 +0200 Subject: [PATCH 09/21] Remove the "lift constant to reference" logic --- .../rustc_mir_build/src/build/matches/test.rs | 35 +---------------- .../src/thir/pattern/const_to_pat.rs | 39 +++++++++++++------ 2 files changed, 28 insertions(+), 46 deletions(-) diff --git a/compiler/rustc_mir_build/src/build/matches/test.rs b/compiler/rustc_mir_build/src/build/matches/test.rs index 796815cf216..d81c3b68f48 100644 --- a/compiler/rustc_mir_build/src/build/matches/test.rs +++ b/compiler/rustc_mir_build/src/build/matches/test.rs @@ -409,40 +409,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { let deref_ty = match *ty.kind() { ty::Ref(_, deref_ty, _) => deref_ty, - _ => { - trace!("non_scalar_compare called on non-reference type: {}", ty); - // Backcompat hack: due to non-structural matches not being a hard error, we can - // reach this for types that have manual `Eq` or `PartialEq` impls. - assert!(!ty.is_structural_eq_shallow(self.hir.tcx())); - let ref_ty = self.hir.tcx().mk_imm_ref(self.hir.tcx().lifetimes.re_erased, ty); - // let y = &place; - let y = self.temp(ref_ty, source_info.span); - self.cfg.push_assign( - block, - source_info, - y, - Rvalue::Ref(self.hir.tcx().lifetimes.re_erased, BorrowKind::Shared, place), - ); - val = Operand::Move(y); - // let temp = expect; - let temp = self.temp(ty, source_info.span); - self.cfg.push_assign( - block, - source_info, - temp, - Rvalue::Use(expect), - ); - // reftemp = &temp; - let reftemp = self.temp(ref_ty, source_info.span); - self.cfg.push_assign( - block, - source_info, - reftemp, - Rvalue::Ref(self.hir.tcx().lifetimes.re_erased, BorrowKind::Shared, temp), - ); - expect = Operand::Move(reftemp); - ty - }, + _ => bug!("non_scalar_compare called on non-reference type: {}", ty), }; let eq_def_id = self.hir.tcx().require_lang_item(LangItem::PartialEq, None); diff --git a/compiler/rustc_mir_build/src/thir/pattern/const_to_pat.rs b/compiler/rustc_mir_build/src/thir/pattern/const_to_pat.rs index ad0bcfeb367..4a50bbca066 100644 --- a/compiler/rustc_mir_build/src/thir/pattern/const_to_pat.rs +++ b/compiler/rustc_mir_build/src/thir/pattern/const_to_pat.rs @@ -58,6 +58,9 @@ struct ConstToPat<'a, 'tcx> { include_lint_checks: bool, } +#[derive(Debug)] +struct FallbackToConstRef; + impl<'a, 'tcx> ConstToPat<'a, 'tcx> { fn new( pat_ctxt: &PatCtxt<'_, 'tcx>, @@ -103,7 +106,7 @@ impl<'a, 'tcx> ConstToPat<'a, 'tcx> { // once indirect_structural_match is a full fledged error, this // level of indirection can be eliminated - let inlined_const_as_pat = self.recur(cv, mir_structural_match_violation); + let inlined_const_as_pat = self.recur(cv, mir_structural_match_violation).unwrap(); if self.include_lint_checks && !self.saw_const_match_error.get() { // If we were able to successfully convert the const to some pat, @@ -216,18 +219,22 @@ impl<'a, 'tcx> ConstToPat<'a, 'tcx> { } // Recursive helper for `to_pat`; invoke that (instead of calling this directly). - fn recur(&self, cv: &'tcx ty::Const<'tcx>, mir_structural_match_violation: bool) -> Pat<'tcx> { + fn recur( + &self, + cv: &'tcx ty::Const<'tcx>, + mir_structural_match_violation: bool, + ) -> Result, FallbackToConstRef> { let id = self.id; let span = self.span; let tcx = self.tcx(); let param_env = self.param_env; - let field_pats = |vals: &[&'tcx ty::Const<'tcx>]| { + let field_pats = |vals: &[&'tcx ty::Const<'tcx>]| -> Result<_, _> { vals.iter() .enumerate() .map(|(idx, val)| { let field = Field::new(idx); - FieldPat { field, pattern: self.recur(val, false) } + Ok(FieldPat { field, pattern: self.recur(val, false)? }) }) .collect() }; @@ -287,7 +294,10 @@ impl<'a, 'tcx> ConstToPat<'a, 'tcx> { |lint| lint.build(&msg).emit(), ); } - PatKind::Constant { value: cv } + // Since we are behind a reference, we can just bubble the error up so we get a + // constant at reference type, making it easy to let the fallback call + // `PartialEq::eq` on it. + return Err(FallbackToConstRef); } ty::Adt(adt_def, _) if !self.type_marked_structural(cv.ty) => { debug!("adt_def {:?} has !type_marked_structural for cv.ty: {:?}", adt_def, cv.ty); @@ -309,12 +319,12 @@ impl<'a, 'tcx> ConstToPat<'a, 'tcx> { variant_index: destructured .variant .expect("destructed const of adt without variant id"), - subpatterns: field_pats(destructured.fields), + subpatterns: field_pats(destructured.fields)?, } } ty::Tuple(_) | ty::Adt(_, _) => { let destructured = tcx.destructure_const(param_env.and(cv)); - PatKind::Leaf { subpatterns: field_pats(destructured.fields) } + PatKind::Leaf { subpatterns: field_pats(destructured.fields)? } } ty::Array(..) => PatKind::Array { prefix: tcx @@ -322,7 +332,7 @@ impl<'a, 'tcx> ConstToPat<'a, 'tcx> { .fields .iter() .map(|val| self.recur(val, false)) - .collect(), + .collect::>()?, slice: None, suffix: Vec::new(), }, @@ -355,7 +365,7 @@ impl<'a, 'tcx> ConstToPat<'a, 'tcx> { .fields .iter() .map(|val| self.recur(val, false)) - .collect(), + .collect::>()?, slice: None, suffix: vec![], }), @@ -379,8 +389,13 @@ impl<'a, 'tcx> ConstToPat<'a, 'tcx> { // deref pattern. _ => { let old = self.behind_reference.replace(true); - let val = PatKind::Deref { - subpattern: self.recur(tcx.deref_const(self.param_env.and(cv)), false), + // In case there are structural-match violations somewhere in this subpattern, + // we fall back to a const pattern. If we do not do this, we may end up with + // a !structural-match constant that is not of reference type, which makes it + // very hard to invoke `PartialEq::eq` on it as a fallback. + let val = match self.recur(tcx.deref_const(self.param_env.and(cv)), false) { + Ok(subpattern) => PatKind::Deref { subpattern }, + Err(FallbackToConstRef) => PatKind::Constant { value: cv }, }; self.behind_reference.set(old); val @@ -439,6 +454,6 @@ impl<'a, 'tcx> ConstToPat<'a, 'tcx> { ); } - Pat { span, ty: cv.ty, kind: Box::new(kind) } + Ok(Pat { span, ty: cv.ty, kind: Box::new(kind) }) } } From d486486afdb545e43a27326be07a92d5fc9ad387 Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Wed, 23 Sep 2020 16:38:30 +0200 Subject: [PATCH 10/21] Talk about unpredictable instead of "not deterministic" --- compiler/rustc_mir_build/src/thir/pattern/const_to_pat.rs | 4 ++-- src/test/ui/issues/issue-44333.rs | 4 ++-- src/test/ui/issues/issue-44333.stderr | 4 ++-- src/test/ui/rfc1445/issue-63479-match-fnptr.rs | 2 +- src/test/ui/rfc1445/issue-63479-match-fnptr.stderr | 2 +- 5 files changed, 8 insertions(+), 8 deletions(-) diff --git a/compiler/rustc_mir_build/src/thir/pattern/const_to_pat.rs b/compiler/rustc_mir_build/src/thir/pattern/const_to_pat.rs index 4a50bbca066..3c0dec64693 100644 --- a/compiler/rustc_mir_build/src/thir/pattern/const_to_pat.rs +++ b/compiler/rustc_mir_build/src/thir/pattern/const_to_pat.rs @@ -413,8 +413,8 @@ impl<'a, 'tcx> ConstToPat<'a, 'tcx> { ty::FnPtr(..) | ty::RawPtr(..) => { if self.include_lint_checks && !self.saw_const_match_error.get() { self.saw_const_match_error.set(true); - let msg = "function pointers and unsized pointers in patterns do not behave \ - deterministically. \ + 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."; tcx.struct_span_lint_hir( lint::builtin::POINTER_STRUCTURAL_MATCH, diff --git a/src/test/ui/issues/issue-44333.rs b/src/test/ui/issues/issue-44333.rs index 85f5ccbdb65..96e8795e52d 100644 --- a/src/test/ui/issues/issue-44333.rs +++ b/src/test/ui/issues/issue-44333.rs @@ -16,9 +16,9 @@ const BAR: Func = bar; fn main() { match test(std::env::consts::ARCH.len()) { - FOO => println!("foo"), //~ WARN pointers in patterns do not behave deterministically + FOO => println!("foo"), //~ WARN pointers in patterns behave unpredictably //~^ WARN will become a hard error - BAR => println!("bar"), //~ WARN pointers in patterns do not behave deterministically + BAR => println!("bar"), //~ WARN pointers in patterns behave unpredictably //~^ WARN will become a hard error _ => unreachable!(), } diff --git a/src/test/ui/issues/issue-44333.stderr b/src/test/ui/issues/issue-44333.stderr index a9ee5fc4dd7..8302b09e533 100644 --- a/src/test/ui/issues/issue-44333.stderr +++ b/src/test/ui/issues/issue-44333.stderr @@ -1,4 +1,4 @@ -warning: function pointers and unsized pointers in patterns do not behave deterministically. See https://github.com/rust-lang/rust/issues/70861 for details. +warning: 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. --> $DIR/issue-44333.rs:19:9 | LL | FOO => println!("foo"), @@ -12,7 +12,7 @@ LL | #![warn(pointer_structural_match)] = 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 #62411 -warning: function pointers and unsized pointers in patterns do not behave deterministically. See https://github.com/rust-lang/rust/issues/70861 for details. +warning: 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. --> $DIR/issue-44333.rs:21:9 | LL | BAR => println!("bar"), diff --git a/src/test/ui/rfc1445/issue-63479-match-fnptr.rs b/src/test/ui/rfc1445/issue-63479-match-fnptr.rs index 0984b1d480e..567685950e9 100644 --- a/src/test/ui/rfc1445/issue-63479-match-fnptr.rs +++ b/src/test/ui/rfc1445/issue-63479-match-fnptr.rs @@ -33,7 +33,7 @@ fn main() { let s = B(my_fn); match s { B(TEST) => println!("matched"), - //~^ WARN pointers in patterns do not behave deterministically + //~^ WARN pointers in patterns behave unpredictably //~| WARN this was previously accepted by the compiler but is being phased out _ => panic!("didn't match") }; diff --git a/src/test/ui/rfc1445/issue-63479-match-fnptr.stderr b/src/test/ui/rfc1445/issue-63479-match-fnptr.stderr index 34b9c359ca8..8cf87cc85a1 100644 --- a/src/test/ui/rfc1445/issue-63479-match-fnptr.stderr +++ b/src/test/ui/rfc1445/issue-63479-match-fnptr.stderr @@ -1,4 +1,4 @@ -warning: function pointers and unsized pointers in patterns do not behave deterministically. See https://github.com/rust-lang/rust/issues/70861 for details. +warning: 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. --> $DIR/issue-63479-match-fnptr.rs:35:7 | LL | B(TEST) => println!("matched"), From 177d0cef48879c7aea8d7dc5064407c454d36124 Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Wed, 23 Sep 2020 17:03:31 +0200 Subject: [PATCH 11/21] Deduplicate errors in const to pat conversion --- .../src/thir/pattern/const_to_pat.rs | 33 ++++++-- .../const_in_pattern/cross-crate-fail.rs | 2 - .../const_in_pattern/cross-crate-fail.stderr | 16 +--- .../const_in_pattern/no-eq-branch-fail.rs | 1 - .../const_in_pattern/no-eq-branch-fail.stderr | 8 +- .../const_in_pattern/reject_non_partial_eq.rs | 1 - .../reject_non_partial_eq.stderr | 8 +- .../const_in_pattern/reject_non_structural.rs | 10 --- .../reject_non_structural.stderr | 82 +++---------------- src/test/ui/match/issue-70972-dyn-trait.rs | 1 - .../ui/match/issue-70972-dyn-trait.stderr | 8 +- ...opaquely-typed-constant-used-in-pattern.rs | 1 - ...uely-typed-constant-used-in-pattern.stderr | 8 +- ...cant-hide-behind-direct-struct-embedded.rs | 1 - ...-hide-behind-direct-struct-embedded.stderr | 8 +- .../cant-hide-behind-direct-struct-param.rs | 1 - ...ant-hide-behind-direct-struct-param.stderr | 8 +- .../ui/rfc1445/match-forbidden-without-eq.rs | 1 - .../rfc1445/match-forbidden-without-eq.stderr | 12 +-- ...tch-nonempty-array-forbidden-without-eq.rs | 1 - ...nonempty-array-forbidden-without-eq.stderr | 8 +- .../match-requires-both-partialeq-and-eq.rs | 1 - ...atch-requires-both-partialeq-and-eq.stderr | 8 +- .../structural-match-no-leak.rs | 1 - .../structural-match-no-leak.stderr | 8 +- .../type-alias-impl-trait/structural-match.rs | 1 - .../structural-match.stderr | 8 +- src/test/ui/union/union-const-pat.rs | 1 - src/test/ui/union/union-const-pat.stderr | 8 +- 29 files changed, 55 insertions(+), 200 deletions(-) diff --git a/compiler/rustc_mir_build/src/thir/pattern/const_to_pat.rs b/compiler/rustc_mir_build/src/thir/pattern/const_to_pat.rs index 3c0dec64693..9af40acc6d6 100644 --- a/compiler/rustc_mir_build/src/thir/pattern/const_to_pat.rs +++ b/compiler/rustc_mir_build/src/thir/pattern/const_to_pat.rs @@ -252,7 +252,12 @@ impl<'a, 'tcx> ConstToPat<'a, 'tcx> { ty::Adt(adt_def, _) if adt_def.is_union() => { // Matching on union fields is unsafe, we can't hide it in constants self.saw_const_match_error.set(true); - tcx.sess.span_err(span, "cannot use unions in constant patterns"); + let msg = "cannot use unions in constant patterns"; + if self.include_lint_checks { + tcx.sess.span_err(span, msg); + } else { + tcx.sess.delay_span_bug(span, msg) + } PatKind::Wild } ty::Adt(..) @@ -267,7 +272,11 @@ impl<'a, 'tcx> ConstToPat<'a, 'tcx> { cv.ty, cv.ty, ); self.saw_const_match_error.set(true); - self.tcx().sess.span_err(self.span, &msg); + if self.include_lint_checks { + tcx.sess.span_err(self.span, &msg); + } else { + tcx.sess.delay_span_bug(self.span, &msg) + } PatKind::Wild } // If the type is not structurally comparable, just emit the constant directly, @@ -308,7 +317,11 @@ impl<'a, 'tcx> ConstToPat<'a, 'tcx> { path, path, ); self.saw_const_match_error.set(true); - tcx.sess.span_err(span, &msg); + if self.include_lint_checks { + tcx.sess.span_err(span, &msg); + } else { + tcx.sess.delay_span_bug(span, &msg) + } PatKind::Wild } ty::Adt(adt_def, substs) if adt_def.is_enum() => { @@ -340,7 +353,12 @@ impl<'a, 'tcx> ConstToPat<'a, 'tcx> { // These are not allowed and will error elsewhere anyway. ty::Dynamic(..) => { self.saw_const_match_error.set(true); - tcx.sess.span_err(span, &format!("`{}` cannot be used in patterns", cv.ty)); + let msg = format!("`{}` cannot be used in patterns", cv.ty); + if self.include_lint_checks { + tcx.sess.span_err(span, &msg); + } else { + tcx.sess.delay_span_bug(span, &msg) + } PatKind::Wild } // `&str` and `&[u8]` are represented as `ConstValue::Slice`, let's keep using this @@ -427,7 +445,12 @@ impl<'a, 'tcx> ConstToPat<'a, 'tcx> { } _ => { self.saw_const_match_error.set(true); - tcx.sess.span_err(span, &format!("`{}` cannot be used in patterns", cv.ty)); + let msg = format!("`{}` cannot be used in patterns", cv.ty); + if self.include_lint_checks { + tcx.sess.span_err(span, &msg); + } else { + tcx.sess.delay_span_bug(span, &msg) + } PatKind::Wild } }; diff --git a/src/test/ui/consts/const_in_pattern/cross-crate-fail.rs b/src/test/ui/consts/const_in_pattern/cross-crate-fail.rs index 05c53e5edcc..ab297f54dff 100644 --- a/src/test/ui/consts/const_in_pattern/cross-crate-fail.rs +++ b/src/test/ui/consts/const_in_pattern/cross-crate-fail.rs @@ -12,7 +12,6 @@ fn main() { match None { consts::SOME => panic!(), //~^ must be annotated with `#[derive(PartialEq, Eq)]` - //~| must be annotated with `#[derive(PartialEq, Eq)]` _ => {} } @@ -20,7 +19,6 @@ fn main() { match None { ::SOME => panic!(), //~^ must be annotated with `#[derive(PartialEq, Eq)]` - //~| must be annotated with `#[derive(PartialEq, Eq)]` _ => {} } diff --git a/src/test/ui/consts/const_in_pattern/cross-crate-fail.stderr b/src/test/ui/consts/const_in_pattern/cross-crate-fail.stderr index 95db19e342a..a8066a88c35 100644 --- a/src/test/ui/consts/const_in_pattern/cross-crate-fail.stderr +++ b/src/test/ui/consts/const_in_pattern/cross-crate-fail.stderr @@ -5,22 +5,10 @@ LL | consts::SOME => panic!(), | ^^^^^^^^^^^^ error: to use a constant of type `CustomEq` in a pattern, `CustomEq` must be annotated with `#[derive(PartialEq, Eq)]` - --> $DIR/cross-crate-fail.rs:21:9 + --> $DIR/cross-crate-fail.rs:20:9 | LL | ::SOME => panic!(), | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -error: to use a constant of type `CustomEq` in a pattern, `CustomEq` must be annotated with `#[derive(PartialEq, Eq)]` - --> $DIR/cross-crate-fail.rs:13:9 - | -LL | consts::SOME => panic!(), - | ^^^^^^^^^^^^ - -error: to use a constant of type `CustomEq` in a pattern, `CustomEq` must be annotated with `#[derive(PartialEq, Eq)]` - --> $DIR/cross-crate-fail.rs:21:9 - | -LL | ::SOME => panic!(), - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - -error: aborting due to 4 previous errors +error: aborting due to 2 previous errors diff --git a/src/test/ui/consts/const_in_pattern/no-eq-branch-fail.rs b/src/test/ui/consts/const_in_pattern/no-eq-branch-fail.rs index c7f02c615a0..fc80d51c72d 100644 --- a/src/test/ui/consts/const_in_pattern/no-eq-branch-fail.rs +++ b/src/test/ui/consts/const_in_pattern/no-eq-branch-fail.rs @@ -20,7 +20,6 @@ fn main() { match Foo::Qux(NoEq) { BAR_BAZ => panic!(), //~^ ERROR must be annotated with `#[derive(PartialEq, Eq)]` - //~| ERROR must be annotated with `#[derive(PartialEq, Eq)]` _ => {} } } diff --git a/src/test/ui/consts/const_in_pattern/no-eq-branch-fail.stderr b/src/test/ui/consts/const_in_pattern/no-eq-branch-fail.stderr index ee78c6f5c3e..e505dad69be 100644 --- a/src/test/ui/consts/const_in_pattern/no-eq-branch-fail.stderr +++ b/src/test/ui/consts/const_in_pattern/no-eq-branch-fail.stderr @@ -4,11 +4,5 @@ error: to use a constant of type `Foo` in a pattern, `Foo` must be annotated wit LL | BAR_BAZ => panic!(), | ^^^^^^^ -error: to use a constant of type `Foo` in a pattern, `Foo` must be annotated with `#[derive(PartialEq, Eq)]` - --> $DIR/no-eq-branch-fail.rs:21:9 - | -LL | BAR_BAZ => panic!(), - | ^^^^^^^ - -error: aborting due to 2 previous errors +error: aborting due to previous error diff --git a/src/test/ui/consts/const_in_pattern/reject_non_partial_eq.rs b/src/test/ui/consts/const_in_pattern/reject_non_partial_eq.rs index b2b2daa830f..a8216901c02 100644 --- a/src/test/ui/consts/const_in_pattern/reject_non_partial_eq.rs +++ b/src/test/ui/consts/const_in_pattern/reject_non_partial_eq.rs @@ -27,7 +27,6 @@ fn main() { match None { NO_PARTIAL_EQ_NONE => println!("NO_PARTIAL_EQ_NONE"), //~^ ERROR must be annotated with `#[derive(PartialEq, Eq)]` - //~| ERROR must be annotated with `#[derive(PartialEq, Eq)]` _ => panic!("whoops"), } } diff --git a/src/test/ui/consts/const_in_pattern/reject_non_partial_eq.stderr b/src/test/ui/consts/const_in_pattern/reject_non_partial_eq.stderr index dc830d36003..8462c32ea80 100644 --- a/src/test/ui/consts/const_in_pattern/reject_non_partial_eq.stderr +++ b/src/test/ui/consts/const_in_pattern/reject_non_partial_eq.stderr @@ -4,11 +4,5 @@ error: to use a constant of type `Option` in a pattern, `Option println!("NO_PARTIAL_EQ_NONE"), | ^^^^^^^^^^^^^^^^^^ -error: to use a constant of type `Option` in a pattern, `Option` must be annotated with `#[derive(PartialEq, Eq)]` - --> $DIR/reject_non_partial_eq.rs:28:9 - | -LL | NO_PARTIAL_EQ_NONE => println!("NO_PARTIAL_EQ_NONE"), - | ^^^^^^^^^^^^^^^^^^ - -error: aborting due to 2 previous errors +error: aborting due to previous error diff --git a/src/test/ui/consts/const_in_pattern/reject_non_structural.rs b/src/test/ui/consts/const_in_pattern/reject_non_structural.rs index bbeaeea1f87..7a8169bec45 100644 --- a/src/test/ui/consts/const_in_pattern/reject_non_structural.rs +++ b/src/test/ui/consts/const_in_pattern/reject_non_structural.rs @@ -39,51 +39,41 @@ fn main() { const ENUM: Derive = Derive::Some(NoDerive); match Derive::Some(NoDerive) { ENUM => dbg!(ENUM), _ => panic!("whoops"), }; //~^ ERROR must be annotated with `#[derive(PartialEq, Eq)]` - //~| ERROR must be annotated with `#[derive(PartialEq, Eq)]` const FIELD: OND = TrivialEq(Some(NoDerive)).0; match Some(NoDerive) { FIELD => dbg!(FIELD), _ => panic!("whoops"), }; //~^ ERROR must be annotated with `#[derive(PartialEq, Eq)]` - //~| ERROR must be annotated with `#[derive(PartialEq, Eq)]` const NO_DERIVE_SOME: OND = Some(NoDerive); const INDIRECT: OND = NO_DERIVE_SOME; match Some(NoDerive) {INDIRECT => dbg!(INDIRECT), _ => panic!("whoops"), }; //~^ ERROR must be annotated with `#[derive(PartialEq, Eq)]` - //~| ERROR must be annotated with `#[derive(PartialEq, Eq)]` const TUPLE: (OND, OND) = (None, Some(NoDerive)); match (None, Some(NoDerive)) { TUPLE => dbg!(TUPLE), _ => panic!("whoops"), }; //~^ ERROR must be annotated with `#[derive(PartialEq, Eq)]` - //~| ERROR must be annotated with `#[derive(PartialEq, Eq)]` const TYPE_ASCRIPTION: OND = Some(NoDerive): OND; match Some(NoDerive) { TYPE_ASCRIPTION => dbg!(TYPE_ASCRIPTION), _ => panic!("whoops"), }; //~^ ERROR must be annotated with `#[derive(PartialEq, Eq)]` - //~| ERROR must be annotated with `#[derive(PartialEq, Eq)]` const ARRAY: [OND; 2] = [None, Some(NoDerive)]; match [None, Some(NoDerive)] { ARRAY => dbg!(ARRAY), _ => panic!("whoops"), }; //~^ ERROR must be annotated with `#[derive(PartialEq, Eq)]` - //~| ERROR must be annotated with `#[derive(PartialEq, Eq)]` const REPEAT: [OND; 2] = [Some(NoDerive); 2]; match [Some(NoDerive); 2] { REPEAT => dbg!(REPEAT), _ => panic!("whoops"), }; //~^ ERROR must be annotated with `#[derive(PartialEq, Eq)]` //~| ERROR must be annotated with `#[derive(PartialEq, Eq)]` - //~| ERROR must be annotated with `#[derive(PartialEq, Eq)]` - //~| ERROR must be annotated with `#[derive(PartialEq, Eq)]` trait Trait: Sized { const ASSOC: Option; } impl Trait for NoDerive { const ASSOC: Option = Some(NoDerive); } match Some(NoDerive) { NoDerive::ASSOC => dbg!(NoDerive::ASSOC), _ => panic!("whoops"), }; //~^ ERROR must be annotated with `#[derive(PartialEq, Eq)]` - //~| ERROR must be annotated with `#[derive(PartialEq, Eq)]` const BLOCK: OND = { NoDerive; Some(NoDerive) }; match Some(NoDerive) { BLOCK => dbg!(BLOCK), _ => panic!("whoops"), }; //~^ ERROR must be annotated with `#[derive(PartialEq, Eq)]` - //~| ERROR must be annotated with `#[derive(PartialEq, Eq)]` const ADDR_OF: &OND = &Some(NoDerive); match &Some(NoDerive) { ADDR_OF => dbg!(ADDR_OF), _ => panic!("whoops"), }; diff --git a/src/test/ui/consts/const_in_pattern/reject_non_structural.stderr b/src/test/ui/consts/const_in_pattern/reject_non_structural.stderr index b1310cf101e..56405a55d69 100644 --- a/src/test/ui/consts/const_in_pattern/reject_non_structural.stderr +++ b/src/test/ui/consts/const_in_pattern/reject_non_structural.stderr @@ -5,61 +5,61 @@ LL | match Derive::Some(NoDerive) { ENUM => dbg!(ENUM), _ => panic!("whoops" | ^^^^ error: to use a constant of type `NoDerive` in a pattern, `NoDerive` must be annotated with `#[derive(PartialEq, Eq)]` - --> $DIR/reject_non_structural.rs:45:28 + --> $DIR/reject_non_structural.rs:44:28 | LL | match Some(NoDerive) { FIELD => dbg!(FIELD), _ => panic!("whoops"), }; | ^^^^^ error: to use a constant of type `NoDerive` in a pattern, `NoDerive` must be annotated with `#[derive(PartialEq, Eq)]` - --> $DIR/reject_non_structural.rs:51:27 + --> $DIR/reject_non_structural.rs:49:27 | LL | match Some(NoDerive) {INDIRECT => dbg!(INDIRECT), _ => panic!("whoops"), }; | ^^^^^^^^ error: to use a constant of type `NoDerive` in a pattern, `NoDerive` must be annotated with `#[derive(PartialEq, Eq)]` - --> $DIR/reject_non_structural.rs:56:36 + --> $DIR/reject_non_structural.rs:53:36 | LL | match (None, Some(NoDerive)) { TUPLE => dbg!(TUPLE), _ => panic!("whoops"), }; | ^^^^^ error: to use a constant of type `NoDerive` in a pattern, `NoDerive` must be annotated with `#[derive(PartialEq, Eq)]` - --> $DIR/reject_non_structural.rs:61:28 + --> $DIR/reject_non_structural.rs:57:28 | LL | match Some(NoDerive) { TYPE_ASCRIPTION => dbg!(TYPE_ASCRIPTION), _ => panic!("whoops"), }; | ^^^^^^^^^^^^^^^ error: to use a constant of type `NoDerive` in a pattern, `NoDerive` must be annotated with `#[derive(PartialEq, Eq)]` - --> $DIR/reject_non_structural.rs:66:36 + --> $DIR/reject_non_structural.rs:61:36 | LL | match [None, Some(NoDerive)] { ARRAY => dbg!(ARRAY), _ => panic!("whoops"), }; | ^^^^^ error: to use a constant of type `NoDerive` in a pattern, `NoDerive` must be annotated with `#[derive(PartialEq, Eq)]` - --> $DIR/reject_non_structural.rs:71:33 + --> $DIR/reject_non_structural.rs:65:33 | LL | match [Some(NoDerive); 2] { REPEAT => dbg!(REPEAT), _ => panic!("whoops"), }; | ^^^^^^ error: to use a constant of type `NoDerive` in a pattern, `NoDerive` must be annotated with `#[derive(PartialEq, Eq)]` - --> $DIR/reject_non_structural.rs:71:33 + --> $DIR/reject_non_structural.rs:65:33 | LL | match [Some(NoDerive); 2] { REPEAT => dbg!(REPEAT), _ => panic!("whoops"), }; | ^^^^^^ error: to use a constant of type `NoDerive` in a pattern, `NoDerive` must be annotated with `#[derive(PartialEq, Eq)]` - --> $DIR/reject_non_structural.rs:79:28 + --> $DIR/reject_non_structural.rs:71:28 | LL | match Some(NoDerive) { NoDerive::ASSOC => dbg!(NoDerive::ASSOC), _ => panic!("whoops"), }; | ^^^^^^^^^^^^^^^ error: to use a constant of type `NoDerive` in a pattern, `NoDerive` must be annotated with `#[derive(PartialEq, Eq)]` - --> $DIR/reject_non_structural.rs:84:28 + --> $DIR/reject_non_structural.rs:75:28 | LL | match Some(NoDerive) { BLOCK => dbg!(BLOCK), _ => panic!("whoops"), }; | ^^^^^ warning: to use a constant of type `NoDerive` in a pattern, `NoDerive` must be annotated with `#[derive(PartialEq, Eq)]` - --> $DIR/reject_non_structural.rs:89:29 + --> $DIR/reject_non_structural.rs:79:29 | LL | match &Some(NoDerive) { ADDR_OF => dbg!(ADDR_OF), _ => panic!("whoops"), }; | ^^^^^^^ @@ -72,65 +72,5 @@ LL | #![warn(indirect_structural_match)] = 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 #62411 -error: to use a constant of type `NoDerive` in a pattern, `NoDerive` must be annotated with `#[derive(PartialEq, Eq)]` - --> $DIR/reject_non_structural.rs:40:36 - | -LL | match Derive::Some(NoDerive) { ENUM => dbg!(ENUM), _ => panic!("whoops"), }; - | ^^^^ - -error: to use a constant of type `NoDerive` in a pattern, `NoDerive` must be annotated with `#[derive(PartialEq, Eq)]` - --> $DIR/reject_non_structural.rs:45:28 - | -LL | match Some(NoDerive) { FIELD => dbg!(FIELD), _ => panic!("whoops"), }; - | ^^^^^ - -error: to use a constant of type `NoDerive` in a pattern, `NoDerive` must be annotated with `#[derive(PartialEq, Eq)]` - --> $DIR/reject_non_structural.rs:51:27 - | -LL | match Some(NoDerive) {INDIRECT => dbg!(INDIRECT), _ => panic!("whoops"), }; - | ^^^^^^^^ - -error: to use a constant of type `NoDerive` in a pattern, `NoDerive` must be annotated with `#[derive(PartialEq, Eq)]` - --> $DIR/reject_non_structural.rs:56:36 - | -LL | match (None, Some(NoDerive)) { TUPLE => dbg!(TUPLE), _ => panic!("whoops"), }; - | ^^^^^ - -error: to use a constant of type `NoDerive` in a pattern, `NoDerive` must be annotated with `#[derive(PartialEq, Eq)]` - --> $DIR/reject_non_structural.rs:61:28 - | -LL | match Some(NoDerive) { TYPE_ASCRIPTION => dbg!(TYPE_ASCRIPTION), _ => panic!("whoops"), }; - | ^^^^^^^^^^^^^^^ - -error: to use a constant of type `NoDerive` in a pattern, `NoDerive` must be annotated with `#[derive(PartialEq, Eq)]` - --> $DIR/reject_non_structural.rs:66:36 - | -LL | match [None, Some(NoDerive)] { ARRAY => dbg!(ARRAY), _ => panic!("whoops"), }; - | ^^^^^ - -error: to use a constant of type `NoDerive` in a pattern, `NoDerive` must be annotated with `#[derive(PartialEq, Eq)]` - --> $DIR/reject_non_structural.rs:71:33 - | -LL | match [Some(NoDerive); 2] { REPEAT => dbg!(REPEAT), _ => panic!("whoops"), }; - | ^^^^^^ - -error: to use a constant of type `NoDerive` in a pattern, `NoDerive` must be annotated with `#[derive(PartialEq, Eq)]` - --> $DIR/reject_non_structural.rs:71:33 - | -LL | match [Some(NoDerive); 2] { REPEAT => dbg!(REPEAT), _ => panic!("whoops"), }; - | ^^^^^^ - -error: to use a constant of type `NoDerive` in a pattern, `NoDerive` must be annotated with `#[derive(PartialEq, Eq)]` - --> $DIR/reject_non_structural.rs:79:28 - | -LL | match Some(NoDerive) { NoDerive::ASSOC => dbg!(NoDerive::ASSOC), _ => panic!("whoops"), }; - | ^^^^^^^^^^^^^^^ - -error: to use a constant of type `NoDerive` in a pattern, `NoDerive` must be annotated with `#[derive(PartialEq, Eq)]` - --> $DIR/reject_non_structural.rs:84:28 - | -LL | match Some(NoDerive) { BLOCK => dbg!(BLOCK), _ => panic!("whoops"), }; - | ^^^^^ - -error: aborting due to 20 previous errors; 1 warning emitted +error: aborting due to 10 previous errors; 1 warning emitted diff --git a/src/test/ui/match/issue-70972-dyn-trait.rs b/src/test/ui/match/issue-70972-dyn-trait.rs index b69e2dab265..97d161c59ec 100644 --- a/src/test/ui/match/issue-70972-dyn-trait.rs +++ b/src/test/ui/match/issue-70972-dyn-trait.rs @@ -5,7 +5,6 @@ fn main() { match a { F => panic!(), //~^ ERROR `&dyn Send` cannot be used in patterns - //~| ERROR `&dyn Send` cannot be used in patterns _ => {} } } diff --git a/src/test/ui/match/issue-70972-dyn-trait.stderr b/src/test/ui/match/issue-70972-dyn-trait.stderr index 985799b3c83..7581070ebc1 100644 --- a/src/test/ui/match/issue-70972-dyn-trait.stderr +++ b/src/test/ui/match/issue-70972-dyn-trait.stderr @@ -4,11 +4,5 @@ error: `&dyn Send` cannot be used in patterns LL | F => panic!(), | ^ -error: `&dyn Send` cannot be used in patterns - --> $DIR/issue-70972-dyn-trait.rs:6:9 - | -LL | F => panic!(), - | ^ - -error: aborting due to 2 previous errors +error: aborting due to previous error diff --git a/src/test/ui/pattern/issue-71042-opaquely-typed-constant-used-in-pattern.rs b/src/test/ui/pattern/issue-71042-opaquely-typed-constant-used-in-pattern.rs index 0c38b533a16..427f4cd8c78 100644 --- a/src/test/ui/pattern/issue-71042-opaquely-typed-constant-used-in-pattern.rs +++ b/src/test/ui/pattern/issue-71042-opaquely-typed-constant-used-in-pattern.rs @@ -5,7 +5,6 @@ fn main() { const C: impl Copy = 0; match C { C => {} //~ ERROR: `impl Copy` cannot be used in patterns - //~^ ERROR: `impl Copy` cannot be used in patterns _ => {} } } diff --git a/src/test/ui/pattern/issue-71042-opaquely-typed-constant-used-in-pattern.stderr b/src/test/ui/pattern/issue-71042-opaquely-typed-constant-used-in-pattern.stderr index ad6cc0aa3e3..c22e6eb9443 100644 --- a/src/test/ui/pattern/issue-71042-opaquely-typed-constant-used-in-pattern.stderr +++ b/src/test/ui/pattern/issue-71042-opaquely-typed-constant-used-in-pattern.stderr @@ -4,11 +4,5 @@ error: `impl Copy` cannot be used in patterns LL | C => {} | ^ -error: `impl Copy` cannot be used in patterns - --> $DIR/issue-71042-opaquely-typed-constant-used-in-pattern.rs:7:9 - | -LL | C => {} - | ^ - -error: aborting due to 2 previous errors +error: aborting due to previous error diff --git a/src/test/ui/rfc1445/cant-hide-behind-direct-struct-embedded.rs b/src/test/ui/rfc1445/cant-hide-behind-direct-struct-embedded.rs index c663535e533..4a8a0949379 100644 --- a/src/test/ui/rfc1445/cant-hide-behind-direct-struct-embedded.rs +++ b/src/test/ui/rfc1445/cant-hide-behind-direct-struct-embedded.rs @@ -21,7 +21,6 @@ fn main() { match WRAP_DIRECT_INLINE { WRAP_DIRECT_INLINE => { panic!("WRAP_DIRECT_INLINE matched itself"); } //~^ ERROR must be annotated with `#[derive(PartialEq, Eq)]` - //~| ERROR must be annotated with `#[derive(PartialEq, Eq)]` _ => { println!("WRAP_DIRECT_INLINE did not match itself"); } } } diff --git a/src/test/ui/rfc1445/cant-hide-behind-direct-struct-embedded.stderr b/src/test/ui/rfc1445/cant-hide-behind-direct-struct-embedded.stderr index 9c7d1f3a18f..c73a6cf1326 100644 --- a/src/test/ui/rfc1445/cant-hide-behind-direct-struct-embedded.stderr +++ b/src/test/ui/rfc1445/cant-hide-behind-direct-struct-embedded.stderr @@ -4,11 +4,5 @@ error: to use a constant of type `NoDerive` in a pattern, `NoDerive` must be ann LL | WRAP_DIRECT_INLINE => { panic!("WRAP_DIRECT_INLINE matched itself"); } | ^^^^^^^^^^^^^^^^^^ -error: to use a constant of type `NoDerive` in a pattern, `NoDerive` must be annotated with `#[derive(PartialEq, Eq)]` - --> $DIR/cant-hide-behind-direct-struct-embedded.rs:22:9 - | -LL | WRAP_DIRECT_INLINE => { panic!("WRAP_DIRECT_INLINE matched itself"); } - | ^^^^^^^^^^^^^^^^^^ - -error: aborting due to 2 previous errors +error: aborting due to previous error diff --git a/src/test/ui/rfc1445/cant-hide-behind-direct-struct-param.rs b/src/test/ui/rfc1445/cant-hide-behind-direct-struct-param.rs index 872bf5a63ff..93022a23dbf 100644 --- a/src/test/ui/rfc1445/cant-hide-behind-direct-struct-param.rs +++ b/src/test/ui/rfc1445/cant-hide-behind-direct-struct-param.rs @@ -21,7 +21,6 @@ fn main() { match WRAP_DIRECT_PARAM { WRAP_DIRECT_PARAM => { panic!("WRAP_DIRECT_PARAM matched itself"); } //~^ ERROR must be annotated with `#[derive(PartialEq, Eq)]` - //~| ERROR must be annotated with `#[derive(PartialEq, Eq)]` _ => { println!("WRAP_DIRECT_PARAM did not match itself"); } } } diff --git a/src/test/ui/rfc1445/cant-hide-behind-direct-struct-param.stderr b/src/test/ui/rfc1445/cant-hide-behind-direct-struct-param.stderr index 6f49a8a0c9d..6fdf9db89b8 100644 --- a/src/test/ui/rfc1445/cant-hide-behind-direct-struct-param.stderr +++ b/src/test/ui/rfc1445/cant-hide-behind-direct-struct-param.stderr @@ -4,11 +4,5 @@ error: to use a constant of type `NoDerive` in a pattern, `NoDerive` must be ann LL | WRAP_DIRECT_PARAM => { panic!("WRAP_DIRECT_PARAM matched itself"); } | ^^^^^^^^^^^^^^^^^ -error: to use a constant of type `NoDerive` in a pattern, `NoDerive` must be annotated with `#[derive(PartialEq, Eq)]` - --> $DIR/cant-hide-behind-direct-struct-param.rs:22:9 - | -LL | WRAP_DIRECT_PARAM => { panic!("WRAP_DIRECT_PARAM matched itself"); } - | ^^^^^^^^^^^^^^^^^ - -error: aborting due to 2 previous errors +error: aborting due to previous error diff --git a/src/test/ui/rfc1445/match-forbidden-without-eq.rs b/src/test/ui/rfc1445/match-forbidden-without-eq.rs index 59141eac3e8..1cca2752061 100644 --- a/src/test/ui/rfc1445/match-forbidden-without-eq.rs +++ b/src/test/ui/rfc1445/match-forbidden-without-eq.rs @@ -12,7 +12,6 @@ fn main() { match y { FOO => { } //~^ ERROR must be annotated with `#[derive(PartialEq, Eq)]` - //~| ERROR must be annotated with `#[derive(PartialEq, Eq)]` _ => { } } diff --git a/src/test/ui/rfc1445/match-forbidden-without-eq.stderr b/src/test/ui/rfc1445/match-forbidden-without-eq.stderr index 1f26f0f11dc..02fa2398189 100644 --- a/src/test/ui/rfc1445/match-forbidden-without-eq.stderr +++ b/src/test/ui/rfc1445/match-forbidden-without-eq.stderr @@ -5,7 +5,7 @@ LL | FOO => { } | ^^^ warning: floating-point types cannot be used in patterns - --> $DIR/match-forbidden-without-eq.rs:21:9 + --> $DIR/match-forbidden-without-eq.rs:20:9 | LL | f32::INFINITY => { } | ^^^^^^^^^^^^^ @@ -14,14 +14,8 @@ LL | f32::INFINITY => { } = 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 #41620 -error: to use a constant of type `Foo` in a pattern, `Foo` must be annotated with `#[derive(PartialEq, Eq)]` - --> $DIR/match-forbidden-without-eq.rs:13:9 - | -LL | FOO => { } - | ^^^ - warning: floating-point types cannot be used in patterns - --> $DIR/match-forbidden-without-eq.rs:21:9 + --> $DIR/match-forbidden-without-eq.rs:20:9 | LL | f32::INFINITY => { } | ^^^^^^^^^^^^^ @@ -29,5 +23,5 @@ LL | f32::INFINITY => { } = 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 #41620 -error: aborting due to 2 previous errors; 2 warnings emitted +error: aborting due to previous error; 2 warnings emitted diff --git a/src/test/ui/rfc1445/match-nonempty-array-forbidden-without-eq.rs b/src/test/ui/rfc1445/match-nonempty-array-forbidden-without-eq.rs index 4112e8f4517..151a475c919 100644 --- a/src/test/ui/rfc1445/match-nonempty-array-forbidden-without-eq.rs +++ b/src/test/ui/rfc1445/match-nonempty-array-forbidden-without-eq.rs @@ -15,6 +15,5 @@ fn main() { match [B(1)] { FOO => { } //~^ ERROR must be annotated with `#[derive(PartialEq, Eq)]` - //~| ERROR must be annotated with `#[derive(PartialEq, Eq)]` } } diff --git a/src/test/ui/rfc1445/match-nonempty-array-forbidden-without-eq.stderr b/src/test/ui/rfc1445/match-nonempty-array-forbidden-without-eq.stderr index 7e354bf9ade..371f8a0aa1d 100644 --- a/src/test/ui/rfc1445/match-nonempty-array-forbidden-without-eq.stderr +++ b/src/test/ui/rfc1445/match-nonempty-array-forbidden-without-eq.stderr @@ -4,11 +4,5 @@ error: to use a constant of type `B` in a pattern, `B` must be annotated with `# LL | FOO => { } | ^^^ -error: to use a constant of type `B` in a pattern, `B` must be annotated with `#[derive(PartialEq, Eq)]` - --> $DIR/match-nonempty-array-forbidden-without-eq.rs:16:9 - | -LL | FOO => { } - | ^^^ - -error: aborting due to 2 previous errors +error: aborting due to previous error diff --git a/src/test/ui/rfc1445/match-requires-both-partialeq-and-eq.rs b/src/test/ui/rfc1445/match-requires-both-partialeq-and-eq.rs index 9530a1ffec4..6b7d94603b5 100644 --- a/src/test/ui/rfc1445/match-requires-both-partialeq-and-eq.rs +++ b/src/test/ui/rfc1445/match-requires-both-partialeq-and-eq.rs @@ -16,7 +16,6 @@ fn main() { match y { FOO => { } //~^ ERROR must be annotated with `#[derive(PartialEq, Eq)]` - //~| ERROR must be annotated with `#[derive(PartialEq, Eq)]` _ => { } } } diff --git a/src/test/ui/rfc1445/match-requires-both-partialeq-and-eq.stderr b/src/test/ui/rfc1445/match-requires-both-partialeq-and-eq.stderr index 7ef082852ba..4157cf65283 100644 --- a/src/test/ui/rfc1445/match-requires-both-partialeq-and-eq.stderr +++ b/src/test/ui/rfc1445/match-requires-both-partialeq-and-eq.stderr @@ -4,11 +4,5 @@ error: to use a constant of type `Foo` in a pattern, `Foo` must be annotated wit LL | FOO => { } | ^^^ -error: to use a constant of type `Foo` in a pattern, `Foo` must be annotated with `#[derive(PartialEq, Eq)]` - --> $DIR/match-requires-both-partialeq-and-eq.rs:17:9 - | -LL | FOO => { } - | ^^^ - -error: aborting due to 2 previous errors +error: aborting due to previous error diff --git a/src/test/ui/type-alias-impl-trait/structural-match-no-leak.rs b/src/test/ui/type-alias-impl-trait/structural-match-no-leak.rs index 54fc4956b9b..d50835608fa 100644 --- a/src/test/ui/type-alias-impl-trait/structural-match-no-leak.rs +++ b/src/test/ui/type-alias-impl-trait/structural-match-no-leak.rs @@ -13,7 +13,6 @@ fn leak_free_test() { match todo!() { LEAK_FREE => (), //~^ `impl Send` cannot be used in patterns - //~| `impl Send` cannot be used in patterns _ => (), } } diff --git a/src/test/ui/type-alias-impl-trait/structural-match-no-leak.stderr b/src/test/ui/type-alias-impl-trait/structural-match-no-leak.stderr index 90b44f6598d..889c4fd4b04 100644 --- a/src/test/ui/type-alias-impl-trait/structural-match-no-leak.stderr +++ b/src/test/ui/type-alias-impl-trait/structural-match-no-leak.stderr @@ -4,11 +4,5 @@ error: `impl Send` cannot be used in patterns LL | LEAK_FREE => (), | ^^^^^^^^^ -error: `impl Send` cannot be used in patterns - --> $DIR/structural-match-no-leak.rs:14:9 - | -LL | LEAK_FREE => (), - | ^^^^^^^^^ - -error: aborting due to 2 previous errors +error: aborting due to previous error diff --git a/src/test/ui/type-alias-impl-trait/structural-match.rs b/src/test/ui/type-alias-impl-trait/structural-match.rs index 5fe5bb4bdea..a3ff4ad1d47 100644 --- a/src/test/ui/type-alias-impl-trait/structural-match.rs +++ b/src/test/ui/type-alias-impl-trait/structural-match.rs @@ -14,7 +14,6 @@ fn test() { match todo!() { VALUE => (), //~^ `impl Send` cannot be used in patterns - //~| `impl Send` cannot be used in patterns _ => (), } } diff --git a/src/test/ui/type-alias-impl-trait/structural-match.stderr b/src/test/ui/type-alias-impl-trait/structural-match.stderr index 7aca3ba8640..262fd072613 100644 --- a/src/test/ui/type-alias-impl-trait/structural-match.stderr +++ b/src/test/ui/type-alias-impl-trait/structural-match.stderr @@ -4,11 +4,5 @@ error: `impl Send` cannot be used in patterns LL | VALUE => (), | ^^^^^ -error: `impl Send` cannot be used in patterns - --> $DIR/structural-match.rs:15:9 - | -LL | VALUE => (), - | ^^^^^ - -error: aborting due to 2 previous errors +error: aborting due to previous error diff --git a/src/test/ui/union/union-const-pat.rs b/src/test/ui/union/union-const-pat.rs index cb2248cc6d1..e7cb248a201 100644 --- a/src/test/ui/union/union-const-pat.rs +++ b/src/test/ui/union/union-const-pat.rs @@ -8,7 +8,6 @@ const C: U = U { a: 10 }; fn main() { match C { C => {} //~ ERROR cannot use unions in constant patterns - //~| ERROR cannot use unions in constant patterns _ => {} } } diff --git a/src/test/ui/union/union-const-pat.stderr b/src/test/ui/union/union-const-pat.stderr index bec720401b9..dc87f4de521 100644 --- a/src/test/ui/union/union-const-pat.stderr +++ b/src/test/ui/union/union-const-pat.stderr @@ -4,11 +4,5 @@ error: cannot use unions in constant patterns LL | C => {} | ^ -error: cannot use unions in constant patterns - --> $DIR/union-const-pat.rs:10:9 - | -LL | C => {} - | ^ - -error: aborting due to 2 previous errors +error: aborting due to previous error From 9a7e66aeafe8debdee9b3b5e128c47034864f2b1 Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Wed, 23 Sep 2020 17:52:37 +0200 Subject: [PATCH 12/21] Make sure we don't hide errors just because a lint has been emitted --- .../src/thir/pattern/const_to_pat.rs | 27 ++++++++++++++----- src/test/ui/consts/match_ice.rs | 6 ++--- src/test/ui/consts/match_ice.stderr | 17 ++++-------- ...nt-hide-behind-doubly-indirect-embedded.rs | 2 +- .../cant-hide-behind-doubly-indirect-param.rs | 2 +- ...nt-hide-behind-indirect-struct-embedded.rs | 2 +- .../cant-hide-behind-indirect-struct-param.rs | 2 +- ...2307-match-ref-ref-forbidden-without-eq.rs | 4 +-- 8 files changed, 34 insertions(+), 28 deletions(-) diff --git a/compiler/rustc_mir_build/src/thir/pattern/const_to_pat.rs b/compiler/rustc_mir_build/src/thir/pattern/const_to_pat.rs index 9af40acc6d6..f80c60b181e 100644 --- a/compiler/rustc_mir_build/src/thir/pattern/const_to_pat.rs +++ b/compiler/rustc_mir_build/src/thir/pattern/const_to_pat.rs @@ -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, + // 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, + // For backcompat we need to keep allowing non-structurally-eq types behind references. // See also all the `cant-hide-behind` tests. behind_reference: Cell, @@ -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), } } @@ -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, @@ -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)]`", @@ -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."; @@ -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 \ diff --git a/src/test/ui/consts/match_ice.rs b/src/test/ui/consts/match_ice.rs index db76e230070..73ff15f2122 100644 --- a/src/test/ui/consts/match_ice.rs +++ b/src/test/ui/consts/match_ice.rs @@ -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 { diff --git a/src/test/ui/consts/match_ice.stderr b/src/test/ui/consts/match_ice.stderr index 6cc79dbca7c..915111b3ce4 100644 --- a/src/test/ui/consts/match_ice.stderr +++ b/src/test/ui/consts/match_ice.stderr @@ -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 => {} | ^ @@ -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 -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`. diff --git a/src/test/ui/rfc1445/cant-hide-behind-doubly-indirect-embedded.rs b/src/test/ui/rfc1445/cant-hide-behind-doubly-indirect-embedded.rs index f6947819695..fe62774d220 100644 --- a/src/test/ui/rfc1445/cant-hide-behind-doubly-indirect-embedded.rs +++ b/src/test/ui/rfc1445/cant-hide-behind-doubly-indirect-embedded.rs @@ -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"); } } } diff --git a/src/test/ui/rfc1445/cant-hide-behind-doubly-indirect-param.rs b/src/test/ui/rfc1445/cant-hide-behind-doubly-indirect-param.rs index 1c29d67b655..c3a30674ea3 100644 --- a/src/test/ui/rfc1445/cant-hide-behind-doubly-indirect-param.rs +++ b/src/test/ui/rfc1445/cant-hide-behind-doubly-indirect-param.rs @@ -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"); } } } diff --git a/src/test/ui/rfc1445/cant-hide-behind-indirect-struct-embedded.rs b/src/test/ui/rfc1445/cant-hide-behind-indirect-struct-embedded.rs index 1a41dbb55c2..4d0e80d5af3 100644 --- a/src/test/ui/rfc1445/cant-hide-behind-indirect-struct-embedded.rs +++ b/src/test/ui/rfc1445/cant-hide-behind-indirect-struct-embedded.rs @@ -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"); } } } diff --git a/src/test/ui/rfc1445/cant-hide-behind-indirect-struct-param.rs b/src/test/ui/rfc1445/cant-hide-behind-indirect-struct-param.rs index 46032c4b0eb..432f196ec81 100644 --- a/src/test/ui/rfc1445/cant-hide-behind-indirect-struct-param.rs +++ b/src/test/ui/rfc1445/cant-hide-behind-indirect-struct-param.rs @@ -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"); } } } diff --git a/src/test/ui/rfc1445/issue-62307-match-ref-ref-forbidden-without-eq.rs b/src/test/ui/rfc1445/issue-62307-match-ref-ref-forbidden-without-eq.rs index b5e19611da8..46d8ee3b6be 100644 --- a/src/test/ui/rfc1445/issue-62307-match-ref-ref-forbidden-without-eq.rs +++ b/src/test/ui/rfc1445/issue-62307-match-ref-ref-forbidden-without-eq.rs @@ -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 _ => { } } } From 6a33de017007233346c26a6f7b20c3e35e6b9e90 Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Wed, 23 Sep 2020 17:55:14 +0200 Subject: [PATCH 13/21] Name function correctly --- compiler/rustc_mir_build/src/thir/pattern/const_to_pat.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/compiler/rustc_mir_build/src/thir/pattern/const_to_pat.rs b/compiler/rustc_mir_build/src/thir/pattern/const_to_pat.rs index f80c60b181e..78b0a5a82eb 100644 --- a/compiler/rustc_mir_build/src/thir/pattern/const_to_pat.rs +++ b/compiler/rustc_mir_build/src/thir/pattern/const_to_pat.rs @@ -168,7 +168,7 @@ impl<'a, 'tcx> ConstToPat<'a, 'tcx> { } }); - if !self.type_has_partial_eq_impl(cv.ty) { + if !self.type_may_have_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 && !self.saw_const_match_lint.get() { @@ -190,7 +190,7 @@ impl<'a, 'tcx> ConstToPat<'a, 'tcx> { inlined_const_as_pat } - fn type_has_partial_eq_impl(&self, ty: Ty<'tcx>) -> bool { + fn type_may_have_partial_eq_impl(&self, ty: Ty<'tcx>) -> bool { // double-check there even *is* a semantic `PartialEq` to dispatch to. // // (If there isn't, then we can safely issue a hard @@ -267,7 +267,7 @@ impl<'a, 'tcx> ConstToPat<'a, 'tcx> { PatKind::Wild } ty::Adt(..) - if !self.type_has_partial_eq_impl(cv.ty) + if !self.type_may_have_partial_eq_impl(cv.ty) // 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() => From 017423179a9ea3267d0fab7bd20fd73b0cd8a528 Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Wed, 23 Sep 2020 18:04:44 +0200 Subject: [PATCH 14/21] Make sure we report a future incompat error in all cases --- .../src/thir/pattern/const_to_pat.rs | 20 ++++++++++++++++++- src/test/ui/consts/match_ice.stderr | 6 +++--- ...ide-behind-doubly-indirect-embedded.stderr | 10 +++++++--- ...t-hide-behind-doubly-indirect-param.stderr | 10 +++++++--- ...-match-ref-ref-forbidden-without-eq.stderr | 12 +++++------ 5 files changed, 42 insertions(+), 16 deletions(-) diff --git a/compiler/rustc_mir_build/src/thir/pattern/const_to_pat.rs b/compiler/rustc_mir_build/src/thir/pattern/const_to_pat.rs index 78b0a5a82eb..cf731a076ff 100644 --- a/compiler/rustc_mir_build/src/thir/pattern/const_to_pat.rs +++ b/compiler/rustc_mir_build/src/thir/pattern/const_to_pat.rs @@ -408,7 +408,25 @@ impl<'a, 'tcx> ConstToPat<'a, 'tcx> { // this pattern to a `PartialEq::eq` comparison and `PartialEq::eq` takes a // reference. This makes the rest of the matching logic simpler as it doesn't have // to figure out how to get a reference again. - ty::Adt(..) if !self.type_marked_structural(pointee_ty) => { + ty::Adt(adt_def, _) if !self.type_marked_structural(pointee_ty) => { + 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 path = self.tcx().def_path_str(adt_def.did); + let msg = format!( + "to use a constant of type `{}` in a pattern, \ + `{}` must be annotated with `#[derive(PartialEq, Eq)]`", + path, path, + ); + self.tcx().struct_span_lint_hir( + lint::builtin::INDIRECT_STRUCTURAL_MATCH, + self.id, + self.span, + |lint| lint.build(&msg).emit(), + ); + } PatKind::Constant { value: cv } } // All other references are converted into deref patterns and then recursively diff --git a/src/test/ui/consts/match_ice.stderr b/src/test/ui/consts/match_ice.stderr index 915111b3ce4..c46f2c2e972 100644 --- a/src/test/ui/consts/match_ice.stderr +++ b/src/test/ui/consts/match_ice.stderr @@ -1,12 +1,12 @@ -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)]` +warning: 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 | C => {} | ^ | - = note: `#[warn(nontrivial_structural_match)]` on by default + = note: `#[warn(indirect_structural_match)]` on by default = 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 + = note: for more information, see issue #62411 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 diff --git a/src/test/ui/rfc1445/cant-hide-behind-doubly-indirect-embedded.stderr b/src/test/ui/rfc1445/cant-hide-behind-doubly-indirect-embedded.stderr index eb13eed6ec1..659a9812672 100644 --- a/src/test/ui/rfc1445/cant-hide-behind-doubly-indirect-embedded.stderr +++ b/src/test/ui/rfc1445/cant-hide-behind-doubly-indirect-embedded.stderr @@ -1,12 +1,16 @@ -warning: to use a constant of type `&&WrapInline` in a pattern, the constant's initializer must be trivial or all types in the constant must be annotated with `#[derive(PartialEq, Eq)]` +warning: to use a constant of type `NoDerive` in a pattern, `NoDerive` must be annotated with `#[derive(PartialEq, Eq)]` --> $DIR/cant-hide-behind-doubly-indirect-embedded.rs:24:9 | LL | WRAP_DOUBLY_INDIRECT_INLINE => { panic!("WRAP_DOUBLY_INDIRECT_INLINE matched itself"); } | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ | - = note: `#[warn(nontrivial_structural_match)]` on by default +note: the lint level is defined here + --> $DIR/cant-hide-behind-doubly-indirect-embedded.rs:7:9 + | +LL | #![warn(indirect_structural_match)] + | ^^^^^^^^^^^^^^^^^^^^^^^^^ = 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 + = note: for more information, see issue #62411 warning: 1 warning emitted diff --git a/src/test/ui/rfc1445/cant-hide-behind-doubly-indirect-param.stderr b/src/test/ui/rfc1445/cant-hide-behind-doubly-indirect-param.stderr index ddee99e76bb..c8c36510542 100644 --- a/src/test/ui/rfc1445/cant-hide-behind-doubly-indirect-param.stderr +++ b/src/test/ui/rfc1445/cant-hide-behind-doubly-indirect-param.stderr @@ -1,12 +1,16 @@ -warning: to use a constant of type `&&WrapParam` in a pattern, the constant's initializer must be trivial or all types in the constant must be annotated with `#[derive(PartialEq, Eq)]` +warning: to use a constant of type `NoDerive` in a pattern, `NoDerive` must be annotated with `#[derive(PartialEq, Eq)]` --> $DIR/cant-hide-behind-doubly-indirect-param.rs:24:9 | LL | WRAP_DOUBLY_INDIRECT_PARAM => { panic!("WRAP_DOUBLY_INDIRECT_PARAM matched itself"); } | ^^^^^^^^^^^^^^^^^^^^^^^^^^ | - = note: `#[warn(nontrivial_structural_match)]` on by default +note: the lint level is defined here + --> $DIR/cant-hide-behind-doubly-indirect-param.rs:7:9 + | +LL | #![warn(indirect_structural_match)] + | ^^^^^^^^^^^^^^^^^^^^^^^^^ = 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 + = note: for more information, see issue #62411 warning: 1 warning emitted diff --git a/src/test/ui/rfc1445/issue-62307-match-ref-ref-forbidden-without-eq.stderr b/src/test/ui/rfc1445/issue-62307-match-ref-ref-forbidden-without-eq.stderr index 7f4b4923332..a50093a5b11 100644 --- a/src/test/ui/rfc1445/issue-62307-match-ref-ref-forbidden-without-eq.stderr +++ b/src/test/ui/rfc1445/issue-62307-match-ref-ref-forbidden-without-eq.stderr @@ -1,25 +1,25 @@ -warning: to use a constant of type `&&B` in a pattern, the constant's initializer must be trivial or all types in the constant must be annotated with `#[derive(PartialEq, Eq)]` +warning: to use a constant of type `B` in a pattern, `B` must be annotated with `#[derive(PartialEq, Eq)]` --> $DIR/issue-62307-match-ref-ref-forbidden-without-eq.rs:31:9 | LL | RR_B1 => { println!("CLAIM RR0: {:?} matches {:?}", RR_B1, RR_B0); } | ^^^^^ | note: the lint level is defined here - --> $DIR/issue-62307-match-ref-ref-forbidden-without-eq.rs:13:36 + --> $DIR/issue-62307-match-ref-ref-forbidden-without-eq.rs:13:9 | LL | #![warn(indirect_structural_match, nontrivial_structural_match)] - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^^^^^^^^^^^^^^^^^^^^^ = 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 + = note: for more information, see issue #62411 -warning: to use a constant of type `&&B` in a pattern, the constant's initializer must be trivial or all types in the constant must be annotated with `#[derive(PartialEq, Eq)]` +warning: to use a constant of type `B` in a pattern, `B` must be annotated with `#[derive(PartialEq, Eq)]` --> $DIR/issue-62307-match-ref-ref-forbidden-without-eq.rs:38:9 | LL | RR_B1 => { println!("CLAIM RR1: {:?} matches {:?}", RR_B1, RR_B1); } | ^^^^^ | = 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 + = note: for more information, see issue #62411 warning: 2 warnings emitted From da217644a1e6fdf97ea745d5f3bc527477e4a3f6 Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Wed, 23 Sep 2020 18:36:53 +0200 Subject: [PATCH 15/21] Make sure we keep emitting a hard error --- .../src/thir/pattern/const_to_pat.rs | 54 ++++++++++++------- src/test/ui/consts/match_ice.rs | 2 - src/test/ui/consts/match_ice.stderr | 12 +---- 3 files changed, 37 insertions(+), 31 deletions(-) diff --git a/compiler/rustc_mir_build/src/thir/pattern/const_to_pat.rs b/compiler/rustc_mir_build/src/thir/pattern/const_to_pat.rs index cf731a076ff..5025bacafa1 100644 --- a/compiler/rustc_mir_build/src/thir/pattern/const_to_pat.rs +++ b/compiler/rustc_mir_build/src/thir/pattern/const_to_pat.rs @@ -409,25 +409,43 @@ impl<'a, 'tcx> ConstToPat<'a, 'tcx> { // reference. This makes the rest of the matching logic simpler as it doesn't have // to figure out how to get a reference again. ty::Adt(adt_def, _) if !self.type_marked_structural(pointee_ty) => { - 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 path = self.tcx().def_path_str(adt_def.did); - let msg = format!( - "to use a constant of type `{}` in a pattern, \ - `{}` must be annotated with `#[derive(PartialEq, Eq)]`", - path, path, - ); - self.tcx().struct_span_lint_hir( - lint::builtin::INDIRECT_STRUCTURAL_MATCH, - self.id, - self.span, - |lint| lint.build(&msg).emit(), - ); + if self.behind_reference.get() { + 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 path = self.tcx().def_path_str(adt_def.did); + let msg = format!( + "to use a constant of type `{}` in a pattern, \ + `{}` must be annotated with `#[derive(PartialEq, Eq)]`", + path, path, + ); + self.tcx().struct_span_lint_hir( + lint::builtin::INDIRECT_STRUCTURAL_MATCH, + self.id, + self.span, + |lint| lint.build(&msg).emit(), + ); + } + PatKind::Constant { value: cv } + } else { + if !self.saw_const_match_error.get() { + self.saw_const_match_error.set(true); + let path = self.tcx().def_path_str(adt_def.did); + let msg = format!( + "to use a constant of type `{}` in a pattern, \ + `{}` must be annotated with `#[derive(PartialEq, Eq)]`", + path, path, + ); + if self.include_lint_checks { + tcx.sess.span_err(span, &msg); + } else { + tcx.sess.delay_span_bug(span, &msg) + } + } + PatKind::Wild } - PatKind::Constant { value: cv } } // All other references are converted into deref patterns and then recursively // convert the dereferenced constant to a pattern that is the sub-pattern of the diff --git a/src/test/ui/consts/match_ice.rs b/src/test/ui/consts/match_ice.rs index 73ff15f2122..632335c841e 100644 --- a/src/test/ui/consts/match_ice.rs +++ b/src/test/ui/consts/match_ice.rs @@ -10,8 +10,6 @@ fn main() { match C { C => {} //~^ ERROR must be annotated with `#[derive(PartialEq, Eq)]` - //~| WARN must be annotated - //~| WARN previously accepted } const K: &T = &T; match K { diff --git a/src/test/ui/consts/match_ice.stderr b/src/test/ui/consts/match_ice.stderr index c46f2c2e972..699b4a5e200 100644 --- a/src/test/ui/consts/match_ice.stderr +++ b/src/test/ui/consts/match_ice.stderr @@ -1,18 +1,8 @@ -warning: 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 | C => {} - | ^ - | - = note: `#[warn(indirect_structural_match)]` on by default - = 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 #62411 - 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 | C => {} | ^ -error: aborting due to previous error; 1 warning emitted +error: aborting due to previous error From 9aa1c0934cffb5a6f83cdd3943873e6b973382f9 Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Wed, 23 Sep 2020 18:55:27 +0200 Subject: [PATCH 16/21] Update documentation tests --- compiler/rustc_session/src/lint/builtin.rs | 22 ++++++---------------- 1 file changed, 6 insertions(+), 16 deletions(-) diff --git a/compiler/rustc_session/src/lint/builtin.rs b/compiler/rustc_session/src/lint/builtin.rs index b39e01a009e..8e1c843ae61 100644 --- a/compiler/rustc_session/src/lint/builtin.rs +++ b/compiler/rustc_session/src/lint/builtin.rs @@ -2239,23 +2239,13 @@ declare_lint! { /// ```rust,compile_fail /// #![deny(nontrivial_structural_match)] /// - /// struct Plus(i32, i32); - /// const ONE_PLUS_TWO: &&Plus = &&Plus(1, 2); - /// - /// impl PartialEq for Plus { - /// fn eq(&self, other: &Self) -> bool { - /// self.0 + self.1 == other.0 + other.1 - /// } - /// } - /// - /// impl Eq for Plus {} - /// + /// #[derive(Copy, Clone, Debug)] + /// struct NoDerive(u32); + /// impl PartialEq for NoDerive { fn eq(&self, _: &Self) -> bool { false } } + /// impl Eq for NoDerive { } /// fn main() { - /// if let ONE_PLUS_TWO = &&Plus(3, 0) { - /// println!("semantic!"); - /// } else { - /// println!("structural!"); - /// } + /// const INDEX: Option = [None, Some(NoDerive(10))][0]; + /// match None { Some(_) => panic!("whoops"), INDEX => dbg!(INDEX), }; /// } /// ``` /// From 2bc54d427377d3f5d949434a9eb8a9ba71db69a4 Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Thu, 24 Sep 2020 09:43:10 +0200 Subject: [PATCH 17/21] Don't talk about determinism --- compiler/rustc_session/src/lint/builtin.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/compiler/rustc_session/src/lint/builtin.rs b/compiler/rustc_session/src/lint/builtin.rs index 8e1c843ae61..13a4057a35b 100644 --- a/compiler/rustc_session/src/lint/builtin.rs +++ b/compiler/rustc_session/src/lint/builtin.rs @@ -2191,8 +2191,8 @@ declare_lint! { } declare_lint! { - /// The `pointer_structural_match` lint detects pointers used in patterns that do not - /// behave deterministically across optimizations. + /// The `pointer_structural_match` lint detects pointers used in patterns whose behaviour + /// cannot be relied upon across compiler versions and optimization levels. /// /// ### Example /// From e4928d77a1eceea9f53e30bd6af9fdf5be205fae Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Thu, 24 Sep 2020 10:06:07 +0200 Subject: [PATCH 18/21] Use correct type in diagnostics again --- .../src/thir/pattern/const_to_pat.rs | 93 +++++++++---------- .../custom-eq-branch-warn.stderr | 2 +- .../reject_non_partial_eq.stderr | 2 +- .../const_in_pattern/warn_corner_cases.stderr | 6 +- 4 files changed, 50 insertions(+), 53 deletions(-) diff --git a/compiler/rustc_mir_build/src/thir/pattern/const_to_pat.rs b/compiler/rustc_mir_build/src/thir/pattern/const_to_pat.rs index 5025bacafa1..3c8671486bf 100644 --- a/compiler/rustc_mir_build/src/thir/pattern/const_to_pat.rs +++ b/compiler/rustc_mir_build/src/thir/pattern/const_to_pat.rs @@ -89,11 +89,42 @@ impl<'a, 'tcx> ConstToPat<'a, 'tcx> { self.infcx.tcx } - fn search_for_structural_match_violation( - &self, - ty: Ty<'tcx>, - ) -> Option> { - traits::search_for_structural_match_violation(self.id, self.span, self.tcx(), ty) + fn search_for_structural_match_violation(&self, ty: Ty<'tcx>) -> Option { + traits::search_for_structural_match_violation(self.id, self.span, self.tcx(), ty).map( + |non_sm_ty| { + with_no_trimmed_paths(|| match non_sm_ty { + traits::NonStructuralMatchTy::Adt(adt_def) => { + let path = self.tcx().def_path_str(adt_def.did); + format!( + "to use a constant of type `{}` in a pattern, \ + `{}` must be annotated with `#[derive(PartialEq, Eq)]`", + path, path, + ) + } + traits::NonStructuralMatchTy::Dynamic => { + "trait objects cannot be used in patterns".to_string() + } + traits::NonStructuralMatchTy::Opaque => { + "opaque types cannot be used in patterns".to_string() + } + traits::NonStructuralMatchTy::Generator => { + "generators cannot be used in patterns".to_string() + } + traits::NonStructuralMatchTy::Closure => { + "closures cannot be used in patterns".to_string() + } + traits::NonStructuralMatchTy::Param => { + bug!("use of a constant whose type is a parameter inside a pattern") + } + traits::NonStructuralMatchTy::Projection => { + bug!("use of a constant whose type is a projection inside a pattern") + } + traits::NonStructuralMatchTy::Foreign => { + bug!("use of a value of a foreign type inside a pattern") + } + }) + }, + ) } fn type_marked_structural(&self, ty: Ty<'tcx>) -> bool { @@ -135,39 +166,7 @@ impl<'a, 'tcx> ConstToPat<'a, 'tcx> { return inlined_const_as_pat; } - if let Some(non_sm_ty) = structural { - let msg = with_no_trimmed_paths(|| match non_sm_ty { - traits::NonStructuralMatchTy::Adt(adt_def) => { - let path = self.tcx().def_path_str(adt_def.did); - format!( - "to use a constant of type `{}` in a pattern, \ - `{}` must be annotated with `#[derive(PartialEq, Eq)]`", - path, path, - ) - } - traits::NonStructuralMatchTy::Dynamic => { - "trait objects cannot be used in patterns".to_string() - } - traits::NonStructuralMatchTy::Opaque => { - "opaque types cannot be used in patterns".to_string() - } - traits::NonStructuralMatchTy::Generator => { - "generators cannot be used in patterns".to_string() - } - traits::NonStructuralMatchTy::Closure => { - "closures cannot be used in patterns".to_string() - } - traits::NonStructuralMatchTy::Param => { - bug!("use of a constant whose type is a parameter inside a pattern") - } - traits::NonStructuralMatchTy::Projection => { - bug!("use of a constant whose type is a projection inside a pattern") - } - traits::NonStructuralMatchTy::Foreign => { - bug!("use of a value of a foreign type inside a pattern") - } - }); - + if let Some(msg) = structural { if !self.type_may_have_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); @@ -272,11 +271,9 @@ impl<'a, 'tcx> ConstToPat<'a, 'tcx> { // `search_for_structural_match_violation` and then remove this condition. && self.search_for_structural_match_violation(cv.ty).is_some() => { - let msg = format!( - "to use a constant of type `{}` in a pattern, \ - `{}` must be annotated with `#[derive(PartialEq, Eq)]`", - cv.ty, cv.ty, - ); + // Obtain the actual type that isn't annotated. If we just looked at `cv.ty` we + // could get `Option`, even though `Option` is annotated with derive. + let msg = self.search_for_structural_match_violation(cv.ty).unwrap(); self.saw_const_match_error.set(true); if self.include_lint_checks { tcx.sess.span_err(self.span, &msg); @@ -512,11 +509,11 @@ impl<'a, 'tcx> ConstToPat<'a, 'tcx> { && self.search_for_structural_match_violation(cv.ty).is_some() { 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 \ - in the constant must be annotated with `#[derive(PartialEq, Eq)]`", - cv.ty, + // Obtain the actual type that isn't annotated. If we just looked at `cv.ty` we + // could get `Option`, even though `Option` is annotated with derive. + let msg = self.search_for_structural_match_violation(cv.ty).unwrap().replace( + "in a pattern,", + "in a pattern, the constant's initializer must be trivial or", ); tcx.struct_span_lint_hir( lint::builtin::NONTRIVIAL_STRUCTURAL_MATCH, diff --git a/src/test/ui/consts/const_in_pattern/custom-eq-branch-warn.stderr b/src/test/ui/consts/const_in_pattern/custom-eq-branch-warn.stderr index fd6732195e4..e51d6f91649 100644 --- a/src/test/ui/consts/const_in_pattern/custom-eq-branch-warn.stderr +++ b/src/test/ui/consts/const_in_pattern/custom-eq-branch-warn.stderr @@ -1,4 +1,4 @@ -warning: to use a constant of type `Foo` in a pattern, the constant's initializer must be trivial or all types in the constant must be annotated with `#[derive(PartialEq, Eq)]` +warning: to use a constant of type `CustomEq` in a pattern, the constant's initializer must be trivial or `CustomEq` must be annotated with `#[derive(PartialEq, Eq)]` --> $DIR/custom-eq-branch-warn.rs:29:9 | LL | BAR_BAZ => panic!(), diff --git a/src/test/ui/consts/const_in_pattern/reject_non_partial_eq.stderr b/src/test/ui/consts/const_in_pattern/reject_non_partial_eq.stderr index 8462c32ea80..95cfa4a9ebe 100644 --- a/src/test/ui/consts/const_in_pattern/reject_non_partial_eq.stderr +++ b/src/test/ui/consts/const_in_pattern/reject_non_partial_eq.stderr @@ -1,4 +1,4 @@ -error: to use a constant of type `Option` in a pattern, `Option` must be annotated with `#[derive(PartialEq, Eq)]` +error: to use a constant of type `NoPartialEq` in a pattern, `NoPartialEq` must be annotated with `#[derive(PartialEq, Eq)]` --> $DIR/reject_non_partial_eq.rs:28:9 | LL | NO_PARTIAL_EQ_NONE => println!("NO_PARTIAL_EQ_NONE"), diff --git a/src/test/ui/consts/const_in_pattern/warn_corner_cases.stderr b/src/test/ui/consts/const_in_pattern/warn_corner_cases.stderr index a4feaff55b2..a24c8d18184 100644 --- a/src/test/ui/consts/const_in_pattern/warn_corner_cases.stderr +++ b/src/test/ui/consts/const_in_pattern/warn_corner_cases.stderr @@ -1,4 +1,4 @@ -warning: to use a constant of type `Option` in a pattern, the constant's initializer must be trivial or all types in the constant must be annotated with `#[derive(PartialEq, Eq)]` +warning: to use a constant of type `NoDerive` in a pattern, the constant's initializer must be trivial or `NoDerive` must be annotated with `#[derive(PartialEq, Eq)]` --> $DIR/warn_corner_cases.rs:26:47 | LL | match None { Some(_) => panic!("whoops"), INDEX => dbg!(INDEX), }; @@ -8,7 +8,7 @@ LL | match None { Some(_) => panic!("whoops"), INDEX => dbg!(INDEX), }; = 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 -warning: to use a constant of type `Option` in a pattern, the constant's initializer must be trivial or all types in the constant must be annotated with `#[derive(PartialEq, Eq)]` +warning: to use a constant of type `NoDerive` in a pattern, the constant's initializer must be trivial or `NoDerive` must be annotated with `#[derive(PartialEq, Eq)]` --> $DIR/warn_corner_cases.rs:32:47 | LL | match None { Some(_) => panic!("whoops"), CALL => dbg!(CALL), }; @@ -17,7 +17,7 @@ LL | match None { Some(_) => panic!("whoops"), CALL => dbg!(CALL), }; = 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 -warning: to use a constant of type `Option` in a pattern, the constant's initializer must be trivial or all types in the constant must be annotated with `#[derive(PartialEq, Eq)]` +warning: to use a constant of type `NoDerive` in a pattern, the constant's initializer must be trivial or `NoDerive` must be annotated with `#[derive(PartialEq, Eq)]` --> $DIR/warn_corner_cases.rs:38:47 | LL | match None { Some(_) => panic!("whoops"), METHOD_CALL => dbg!(METHOD_CALL), }; From 9550ca624264e3f9b78a95d4454ec30dafd9487b Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Thu, 24 Sep 2020 10:18:51 +0200 Subject: [PATCH 19/21] Deduplicate the "needs partialeq derive" message creation sites --- .../src/thir/pattern/const_to_pat.rs | 34 +++++++------------ 1 file changed, 13 insertions(+), 21 deletions(-) diff --git a/compiler/rustc_mir_build/src/thir/pattern/const_to_pat.rs b/compiler/rustc_mir_build/src/thir/pattern/const_to_pat.rs index 3c8671486bf..37f5d5f6cc3 100644 --- a/compiler/rustc_mir_build/src/thir/pattern/const_to_pat.rs +++ b/compiler/rustc_mir_build/src/thir/pattern/const_to_pat.rs @@ -3,7 +3,7 @@ use rustc_index::vec::Idx; use rustc_infer::infer::{InferCtxt, TyCtxtInferExt}; use rustc_middle::mir::Field; use rustc_middle::ty::print::with_no_trimmed_paths; -use rustc_middle::ty::{self, Ty, TyCtxt}; +use rustc_middle::ty::{self, AdtDef, Ty, TyCtxt}; use rustc_session::lint; use rustc_span::Span; use rustc_trait_selection::traits::predicate_for_trait_def; @@ -89,18 +89,20 @@ impl<'a, 'tcx> ConstToPat<'a, 'tcx> { self.infcx.tcx } + fn adt_derive_msg(&self, adt_def: &AdtDef) -> String { + let path = self.tcx().def_path_str(adt_def.did); + format!( + "to use a constant of type `{}` in a pattern, \ + `{}` must be annotated with `#[derive(PartialEq, Eq)]`", + path, path, + ) + } + fn search_for_structural_match_violation(&self, ty: Ty<'tcx>) -> Option { traits::search_for_structural_match_violation(self.id, self.span, self.tcx(), ty).map( |non_sm_ty| { with_no_trimmed_paths(|| match non_sm_ty { - traits::NonStructuralMatchTy::Adt(adt_def) => { - let path = self.tcx().def_path_str(adt_def.did); - format!( - "to use a constant of type `{}` in a pattern, \ - `{}` must be annotated with `#[derive(PartialEq, Eq)]`", - path, path, - ) - } + traits::NonStructuralMatchTy::Adt(adt) => self.adt_derive_msg(adt), traits::NonStructuralMatchTy::Dynamic => { "trait objects cannot be used in patterns".to_string() } @@ -412,12 +414,7 @@ impl<'a, 'tcx> ConstToPat<'a, 'tcx> { && !self.saw_const_match_lint.get() { self.saw_const_match_lint.set(true); - let path = self.tcx().def_path_str(adt_def.did); - let msg = format!( - "to use a constant of type `{}` in a pattern, \ - `{}` must be annotated with `#[derive(PartialEq, Eq)]`", - path, path, - ); + let msg = self.adt_derive_msg(adt_def); self.tcx().struct_span_lint_hir( lint::builtin::INDIRECT_STRUCTURAL_MATCH, self.id, @@ -429,12 +426,7 @@ impl<'a, 'tcx> ConstToPat<'a, 'tcx> { } else { if !self.saw_const_match_error.get() { self.saw_const_match_error.set(true); - let path = self.tcx().def_path_str(adt_def.did); - let msg = format!( - "to use a constant of type `{}` in a pattern, \ - `{}` must be annotated with `#[derive(PartialEq, Eq)]`", - path, path, - ); + let msg = self.adt_derive_msg(adt_def); if self.include_lint_checks { tcx.sess.span_err(span, &msg); } else { From fc9f2947dab6ad7856986ce98ebd3b75f5293da0 Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Thu, 24 Sep 2020 17:01:03 +0200 Subject: [PATCH 20/21] Document `FallbackToConstRef` and make sure we don't accidentally use it --- .../src/thir/pattern/const_to_pat.rs | 23 +++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-) diff --git a/compiler/rustc_mir_build/src/thir/pattern/const_to_pat.rs b/compiler/rustc_mir_build/src/thir/pattern/const_to_pat.rs index 37f5d5f6cc3..a203b3a1428 100644 --- a/compiler/rustc_mir_build/src/thir/pattern/const_to_pat.rs +++ b/compiler/rustc_mir_build/src/thir/pattern/const_to_pat.rs @@ -63,8 +63,23 @@ struct ConstToPat<'a, 'tcx> { include_lint_checks: bool, } -#[derive(Debug)] -struct FallbackToConstRef; +mod fallback_to_const_ref { + #[derive(Debug)] + /// This error type signals that we encountered a non-struct-eq situation behind a reference. + /// We bubble this up in order to get back to the reference destructuring and make that emit + /// a const pattern instead of a deref pattern. This allows us to simply call `PartialEq::eq` + /// on such patterns (since that function takes a reference) and not have to jump through any + /// hoops to get a reference to the value. + pub(super) struct FallbackToConstRef(()); + + pub(super) fn fallback_to_const_ref<'a, 'tcx>( + c2p: &super::ConstToPat<'a, 'tcx>, + ) -> FallbackToConstRef { + assert!(c2p.behind_reference.get()); + FallbackToConstRef(()) + } +} +use fallback_to_const_ref::{fallback_to_const_ref, FallbackToConstRef}; impl<'a, 'tcx> ConstToPat<'a, 'tcx> { fn new( @@ -314,7 +329,7 @@ impl<'a, 'tcx> ConstToPat<'a, 'tcx> { // Since we are behind a reference, we can just bubble the error up so we get a // constant at reference type, making it easy to let the fallback call // `PartialEq::eq` on it. - return Err(FallbackToConstRef); + return Err(fallback_to_const_ref(self)); } ty::Adt(adt_def, _) if !self.type_marked_structural(cv.ty) => { debug!("adt_def {:?} has !type_marked_structural for cv.ty: {:?}", adt_def, cv.ty); @@ -447,7 +462,7 @@ impl<'a, 'tcx> ConstToPat<'a, 'tcx> { // very hard to invoke `PartialEq::eq` on it as a fallback. let val = match self.recur(tcx.deref_const(self.param_env.and(cv)), false) { Ok(subpattern) => PatKind::Deref { subpattern }, - Err(FallbackToConstRef) => PatKind::Constant { value: cv }, + Err(_) => PatKind::Constant { value: cv }, }; self.behind_reference.set(old); val From daf976f6129e5fb16effc48bf91853548774e235 Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Thu, 24 Sep 2020 17:11:47 +0200 Subject: [PATCH 21/21] Revert a test change to make sure it's still testing the original issue --- .../issue-71042-opaquely-typed-constant-used-in-pattern.rs | 2 +- .../issue-71042-opaquely-typed-constant-used-in-pattern.stderr | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/test/ui/pattern/issue-71042-opaquely-typed-constant-used-in-pattern.rs b/src/test/ui/pattern/issue-71042-opaquely-typed-constant-used-in-pattern.rs index 427f4cd8c78..65f27cf78f1 100644 --- a/src/test/ui/pattern/issue-71042-opaquely-typed-constant-used-in-pattern.rs +++ b/src/test/ui/pattern/issue-71042-opaquely-typed-constant-used-in-pattern.rs @@ -4,7 +4,7 @@ fn main() { const C: impl Copy = 0; match C { - C => {} //~ ERROR: `impl Copy` cannot be used in patterns + C | //~ ERROR: `impl Copy` cannot be used in patterns _ => {} } } diff --git a/src/test/ui/pattern/issue-71042-opaquely-typed-constant-used-in-pattern.stderr b/src/test/ui/pattern/issue-71042-opaquely-typed-constant-used-in-pattern.stderr index c22e6eb9443..62dc856be82 100644 --- a/src/test/ui/pattern/issue-71042-opaquely-typed-constant-used-in-pattern.stderr +++ b/src/test/ui/pattern/issue-71042-opaquely-typed-constant-used-in-pattern.stderr @@ -1,7 +1,7 @@ error: `impl Copy` cannot be used in patterns --> $DIR/issue-71042-opaquely-typed-constant-used-in-pattern.rs:7:9 | -LL | C => {} +LL | C | | ^ error: aborting due to previous error