From bec88ad4aa004a22d719a0ccacf60bb3b75799f7 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Thu, 19 Oct 2023 08:36:24 +0200 Subject: [PATCH 1/5] patterns: reject raw pointers that are not just integers --- .../src/const_eval/valtrees.rs | 37 ++++++++++++++----- compiler/rustc_lint_defs/src/builtin.rs | 5 ++- compiler/rustc_mir_build/messages.ftl | 2 +- .../src/thir/pattern/const_to_pat.rs | 33 +++++++++++++---- .../issue-34784-match-on-non-int-raw-ptr.rs} | 10 ++--- ...ssue-34784-match-on-non-int-raw-ptr.stderr | 25 +++++++++++++ .../ui/consts/const_in_pattern/issue-44333.rs | 4 +- .../const_in_pattern/issue-44333.stderr | 4 +- .../issue-63479-match-fnptr.rs | 2 +- .../issue-63479-match-fnptr.stderr | 2 +- 10 files changed, 92 insertions(+), 32 deletions(-) rename tests/ui/consts/{issue-34784.rs => const_in_pattern/issue-34784-match-on-non-int-raw-ptr.rs} (50%) create mode 100644 tests/ui/consts/const_in_pattern/issue-34784-match-on-non-int-raw-ptr.stderr diff --git a/compiler/rustc_const_eval/src/const_eval/valtrees.rs b/compiler/rustc_const_eval/src/const_eval/valtrees.rs index d6dc1a62f4d..76bd9a82836 100644 --- a/compiler/rustc_const_eval/src/const_eval/valtrees.rs +++ b/compiler/rustc_const_eval/src/const_eval/valtrees.rs @@ -97,11 +97,27 @@ pub(crate) fn const_to_valtree_inner<'tcx>( Ok(ty::ValTree::Leaf(val.assert_int())) } - // Raw pointers are not allowed in type level constants, as we cannot properly test them for - // equality at compile-time (see `ptr_guaranteed_cmp`). + ty::RawPtr(_) => { + // Not all raw pointers are allowed, as we cannot properly test them for + // equality at compile-time (see `ptr_guaranteed_cmp`). + // However we allow those that are just integers in disguise. + // (We could allow wide raw pointers where both sides are integers in the future, + // but for now we reject them.) + let Ok(val) = ecx.read_scalar(place) else { + return Err(ValTreeCreationError::Other); + }; + // We are in the CTFE machine, so ptr-to-int casts will fail. + // This can only be `Ok` if `val` already is an integer. + let Ok(val) = val.try_to_int() else { + return Err(ValTreeCreationError::Other); + }; + // It's just a ScalarInt! + Ok(ty::ValTree::Leaf(val)) + } + // Technically we could allow function pointers (represented as `ty::Instance`), but this is not guaranteed to // agree with runtime equality tests. - ty::FnPtr(_) | ty::RawPtr(_) => Err(ValTreeCreationError::NonSupportedType), + ty::FnPtr(_) => Err(ValTreeCreationError::NonSupportedType), ty::Ref(_, _, _) => { let Ok(derefd_place)= ecx.deref_pointer(place) else { @@ -222,12 +238,14 @@ pub fn valtree_to_const_value<'tcx>( assert!(valtree.unwrap_branch().is_empty()); mir::ConstValue::ZeroSized } - ty::Bool | ty::Int(_) | ty::Uint(_) | ty::Float(_) | ty::Char => match valtree { - ty::ValTree::Leaf(scalar_int) => mir::ConstValue::Scalar(Scalar::Int(scalar_int)), - ty::ValTree::Branch(_) => bug!( - "ValTrees for Bool, Int, Uint, Float or Char should have the form ValTree::Leaf" - ), - }, + ty::Bool | ty::Int(_) | ty::Uint(_) | ty::Float(_) | ty::Char | ty::RawPtr(_) => { + match valtree { + ty::ValTree::Leaf(scalar_int) => mir::ConstValue::Scalar(Scalar::Int(scalar_int)), + ty::ValTree::Branch(_) => bug!( + "ValTrees for Bool, Int, Uint, Float, Char or RawPtr should have the form ValTree::Leaf" + ), + } + } ty::Ref(_, inner_ty, _) => { let mut ecx = mk_eval_cx(tcx, DUMMY_SP, param_env, CanAccessStatics::No); let imm = valtree_to_ref(&mut ecx, valtree, *inner_ty); @@ -281,7 +299,6 @@ pub fn valtree_to_const_value<'tcx>( | ty::Coroutine(..) | ty::CoroutineWitness(..) | ty::FnPtr(_) - | ty::RawPtr(_) | ty::Str | ty::Slice(_) | ty::Dynamic(..) => bug!("no ValTree should have been created for type {:?}", ty.kind()), diff --git a/compiler/rustc_lint_defs/src/builtin.rs b/compiler/rustc_lint_defs/src/builtin.rs index 918e484b9f4..f033504c523 100644 --- a/compiler/rustc_lint_defs/src/builtin.rs +++ b/compiler/rustc_lint_defs/src/builtin.rs @@ -2217,13 +2217,14 @@ declare_lint! { /// /// ### Explanation /// - /// Previous versions of Rust allowed function pointers and wide raw pointers in patterns. + /// Previous versions of Rust allowed function pointers and all 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. + /// in different crates and not deduplicated again via LTO. Pointer identity for memory + /// created by `const` is similarly unreliable. pub POINTER_STRUCTURAL_MATCH, Allow, "pointers are not structural-match", diff --git a/compiler/rustc_mir_build/messages.ftl b/compiler/rustc_mir_build/messages.ftl index 32711c23dc4..dfd0bc238c1 100644 --- a/compiler/rustc_mir_build/messages.ftl +++ b/compiler/rustc_mir_build/messages.ftl @@ -242,7 +242,7 @@ mir_build_overlapping_range_endpoints = multiple patterns overlap on their endpo mir_build_pattern_not_covered = refutable pattern in {$origin} .pattern_ty = the matched value is of type `{$pattern_ty}` -mir_build_pointer_pattern = 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. +mir_build_pointer_pattern = function pointers and raw pointers not derived from integers in patterns behave unpredictably and should not be relied upon. See https://github.com/rust-lang/rust/issues/70861 for details. mir_build_privately_uninhabited = pattern `{$witness_1}` is currently uninhabited, but this variant contains private fields which may become inhabited in the future 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 fc03f7891a8..ad386b129ce 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 @@ -123,6 +123,8 @@ impl<'tcx> ConstToPat<'tcx> { }); debug!(?check_body_for_struct_match_violation, ?mir_structural_match_violation); + let have_valtree = + matches!(cv, mir::Const::Ty(c) if matches!(c.kind(), ty::ConstKind::Value(_))); let inlined_const_as_pat = match cv { mir::Const::Ty(c) => match c.kind() { ty::ConstKind::Param(_) @@ -238,7 +240,9 @@ impl<'tcx> ConstToPat<'tcx> { } } else if !self.saw_const_match_lint.get() { match cv.ty().kind() { - ty::RawPtr(pointee) if pointee.ty.is_sized(self.tcx(), self.param_env) => {} + ty::RawPtr(..) if have_valtree => { + // This is a good raw pointer, it was accepted by valtree construction. + } ty::FnPtr(..) | ty::RawPtr(..) => { self.tcx().emit_spanned_lint( lint::builtin::POINTER_STRUCTURAL_MATCH, @@ -389,11 +393,19 @@ impl<'tcx> ConstToPat<'tcx> { subpatterns: self .field_pats(cv.unwrap_branch().iter().copied().zip(fields.iter()))?, }, - ty::Adt(def, args) => PatKind::Leaf { - subpatterns: self.field_pats(cv.unwrap_branch().iter().copied().zip( - def.non_enum_variant().fields.iter().map(|field| field.ty(self.tcx(), args)), - ))?, - }, + ty::Adt(def, args) => { + assert!(!def.is_union()); // Valtree construction would never succeed for unions. + PatKind::Leaf { + subpatterns: self.field_pats( + cv.unwrap_branch().iter().copied().zip( + def.non_enum_variant() + .fields + .iter() + .map(|field| field.ty(self.tcx(), args)), + ), + )?, + } + } ty::Slice(elem_ty) => PatKind::Slice { prefix: cv .unwrap_branch() @@ -480,10 +492,15 @@ impl<'tcx> ConstToPat<'tcx> { } } }, - ty::Bool | ty::Char | ty::Int(_) | ty::Uint(_) => { + ty::Bool | ty::Char | ty::Int(_) | ty::Uint(_) | ty::RawPtr(..) => { + // The raw pointers we see here have been "vetted" by valtree construction to be + // just integers, so we simply allow them. PatKind::Constant { value: mir::Const::Ty(ty::Const::new_value(tcx, cv, ty)) } } - ty::FnPtr(..) | ty::RawPtr(..) => unreachable!(), + ty::FnPtr(..) => { + // Valtree construction would never succeed for these, so this is unreachable. + unreachable!() + } _ => { let err = InvalidPattern { span, non_sm_ty: ty }; let e = tcx.sess.emit_err(err); diff --git a/tests/ui/consts/issue-34784.rs b/tests/ui/consts/const_in_pattern/issue-34784-match-on-non-int-raw-ptr.rs similarity index 50% rename from tests/ui/consts/issue-34784.rs rename to tests/ui/consts/const_in_pattern/issue-34784-match-on-non-int-raw-ptr.rs index 98d943470a7..bbb98a162c3 100644 --- a/tests/ui/consts/issue-34784.rs +++ b/tests/ui/consts/const_in_pattern/issue-34784-match-on-non-int-raw-ptr.rs @@ -1,12 +1,11 @@ -// run-pass - -#![warn(pointer_structural_match)] +#![deny(pointer_structural_match)] #![allow(dead_code)] const C: *const u8 = &0; fn foo(x: *const u8) { match x { - C => {} + C => {} //~ERROR: behave unpredictably + //~| previously accepted _ => {} } } @@ -15,7 +14,8 @@ const D: *const [u8; 4] = b"abcd"; fn main() { match D { - D => {} + D => {} //~ERROR: behave unpredictably + //~| previously accepted _ => {} } } diff --git a/tests/ui/consts/const_in_pattern/issue-34784-match-on-non-int-raw-ptr.stderr b/tests/ui/consts/const_in_pattern/issue-34784-match-on-non-int-raw-ptr.stderr new file mode 100644 index 00000000000..af7caaf5b74 --- /dev/null +++ b/tests/ui/consts/const_in_pattern/issue-34784-match-on-non-int-raw-ptr.stderr @@ -0,0 +1,25 @@ +error: function pointers and raw pointers not derived from integers in patterns behave unpredictably and should not be relied upon. See https://github.com/rust-lang/rust/issues/70861 for details. + --> $DIR/issue-34784-match-on-non-int-raw-ptr.rs:7:9 + | +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 #62411 +note: the lint level is defined here + --> $DIR/issue-34784-match-on-non-int-raw-ptr.rs:1:9 + | +LL | #![deny(pointer_structural_match)] + | ^^^^^^^^^^^^^^^^^^^^^^^^ + +error: function pointers and raw pointers not derived from integers in patterns behave unpredictably and should not be relied upon. See https://github.com/rust-lang/rust/issues/70861 for details. + --> $DIR/issue-34784-match-on-non-int-raw-ptr.rs:17:9 + | +LL | D => {} + | ^ + | + = 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: aborting due to 2 previous errors + diff --git a/tests/ui/consts/const_in_pattern/issue-44333.rs b/tests/ui/consts/const_in_pattern/issue-44333.rs index 96e8795e52d..aaf1edb6fe6 100644 --- a/tests/ui/consts/const_in_pattern/issue-44333.rs +++ b/tests/ui/consts/const_in_pattern/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 behave unpredictably + FOO => println!("foo"), //~ WARN behave unpredictably //~^ WARN will become a hard error - BAR => println!("bar"), //~ WARN pointers in patterns behave unpredictably + BAR => println!("bar"), //~ WARN behave unpredictably //~^ WARN will become a hard error _ => unreachable!(), } diff --git a/tests/ui/consts/const_in_pattern/issue-44333.stderr b/tests/ui/consts/const_in_pattern/issue-44333.stderr index 731ef509cca..441aeecbc6d 100644 --- a/tests/ui/consts/const_in_pattern/issue-44333.stderr +++ b/tests/ui/consts/const_in_pattern/issue-44333.stderr @@ -1,4 +1,4 @@ -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. +warning: function pointers and raw pointers not derived from integers 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 @@ note: the lint level is defined here LL | #![warn(pointer_structural_match)] | ^^^^^^^^^^^^^^^^^^^^^^^^ -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. +warning: function pointers and raw pointers not derived from integers 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/tests/ui/rfcs/rfc-1445-restrict-constants-in-patterns/issue-63479-match-fnptr.rs b/tests/ui/rfcs/rfc-1445-restrict-constants-in-patterns/issue-63479-match-fnptr.rs index 567685950e9..767d9754299 100644 --- a/tests/ui/rfcs/rfc-1445-restrict-constants-in-patterns/issue-63479-match-fnptr.rs +++ b/tests/ui/rfcs/rfc-1445-restrict-constants-in-patterns/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 behave unpredictably + //~^ WARN behave unpredictably //~| WARN this was previously accepted by the compiler but is being phased out _ => panic!("didn't match") }; diff --git a/tests/ui/rfcs/rfc-1445-restrict-constants-in-patterns/issue-63479-match-fnptr.stderr b/tests/ui/rfcs/rfc-1445-restrict-constants-in-patterns/issue-63479-match-fnptr.stderr index d6afc0255ec..1aa627a8035 100644 --- a/tests/ui/rfcs/rfc-1445-restrict-constants-in-patterns/issue-63479-match-fnptr.stderr +++ b/tests/ui/rfcs/rfc-1445-restrict-constants-in-patterns/issue-63479-match-fnptr.stderr @@ -1,4 +1,4 @@ -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. +warning: function pointers and raw pointers not derived from integers 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 af6c7e0ca154fe4c14b29405e1970b824095920d Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Fri, 20 Oct 2023 08:35:36 +0200 Subject: [PATCH 2/5] also lint against fn ptr and raw ptr nested inside the const --- .../src/thir/pattern/const_to_pat.rs | 22 ++++++++----------- .../issue-34784-match-on-non-int-raw-ptr.rs | 11 ++++++++++ ...ssue-34784-match-on-non-int-raw-ptr.stderr | 15 ++++++++++--- .../issue-63479-match-fnptr.rs | 9 +++++++- .../issue-63479-match-fnptr.stderr | 13 +++++++++-- 5 files changed, 51 insertions(+), 19 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 ad386b129ce..53823c9226c 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 @@ -239,19 +239,15 @@ impl<'tcx> ConstToPat<'tcx> { } } } else if !self.saw_const_match_lint.get() { - match cv.ty().kind() { - ty::RawPtr(..) if have_valtree => { - // This is a good raw pointer, it was accepted by valtree construction. - } - ty::FnPtr(..) | ty::RawPtr(..) => { - self.tcx().emit_spanned_lint( - lint::builtin::POINTER_STRUCTURAL_MATCH, - self.id, - self.span, - PointerPattern, - ); - } - _ => {} + if !have_valtree { + // The only way valtree construction can fail without the structural match + // checker finding a violation is if there is a pointer somewhere. + self.tcx().emit_spanned_lint( + lint::builtin::POINTER_STRUCTURAL_MATCH, + self.id, + self.span, + PointerPattern, + ); } } diff --git a/tests/ui/consts/const_in_pattern/issue-34784-match-on-non-int-raw-ptr.rs b/tests/ui/consts/const_in_pattern/issue-34784-match-on-non-int-raw-ptr.rs index bbb98a162c3..2491071d1e1 100644 --- a/tests/ui/consts/const_in_pattern/issue-34784-match-on-non-int-raw-ptr.rs +++ b/tests/ui/consts/const_in_pattern/issue-34784-match-on-non-int-raw-ptr.rs @@ -1,6 +1,9 @@ #![deny(pointer_structural_match)] #![allow(dead_code)] + const C: *const u8 = &0; +// Make sure we also find pointers nested in other types. +const C_INNER: (*const u8, u8) = (C, 0); fn foo(x: *const u8) { match x { @@ -10,6 +13,14 @@ fn foo(x: *const u8) { } } +fn foo2(x: *const u8) { + match (x, 1) { + C_INNER => {} //~ERROR: behave unpredictably + //~| previously accepted + _ => {} + } +} + const D: *const [u8; 4] = b"abcd"; fn main() { diff --git a/tests/ui/consts/const_in_pattern/issue-34784-match-on-non-int-raw-ptr.stderr b/tests/ui/consts/const_in_pattern/issue-34784-match-on-non-int-raw-ptr.stderr index af7caaf5b74..ab53346b5ee 100644 --- a/tests/ui/consts/const_in_pattern/issue-34784-match-on-non-int-raw-ptr.stderr +++ b/tests/ui/consts/const_in_pattern/issue-34784-match-on-non-int-raw-ptr.stderr @@ -1,5 +1,5 @@ error: function pointers and raw pointers not derived from integers in patterns behave unpredictably and should not be relied upon. See https://github.com/rust-lang/rust/issues/70861 for details. - --> $DIR/issue-34784-match-on-non-int-raw-ptr.rs:7:9 + --> $DIR/issue-34784-match-on-non-int-raw-ptr.rs:10:9 | LL | C => {} | ^ @@ -13,7 +13,16 @@ LL | #![deny(pointer_structural_match)] | ^^^^^^^^^^^^^^^^^^^^^^^^ error: function pointers and raw pointers not derived from integers in patterns behave unpredictably and should not be relied upon. See https://github.com/rust-lang/rust/issues/70861 for details. - --> $DIR/issue-34784-match-on-non-int-raw-ptr.rs:17:9 + --> $DIR/issue-34784-match-on-non-int-raw-ptr.rs:18:9 + | +LL | C_INNER => {} + | ^^^^^^^ + | + = 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: function pointers and raw pointers not derived from integers in patterns behave unpredictably and should not be relied upon. See https://github.com/rust-lang/rust/issues/70861 for details. + --> $DIR/issue-34784-match-on-non-int-raw-ptr.rs:28:9 | LL | D => {} | ^ @@ -21,5 +30,5 @@ LL | D => {} = 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: aborting due to 2 previous errors +error: aborting due to 3 previous errors diff --git a/tests/ui/rfcs/rfc-1445-restrict-constants-in-patterns/issue-63479-match-fnptr.rs b/tests/ui/rfcs/rfc-1445-restrict-constants-in-patterns/issue-63479-match-fnptr.rs index 767d9754299..b05b8c8da1f 100644 --- a/tests/ui/rfcs/rfc-1445-restrict-constants-in-patterns/issue-63479-match-fnptr.rs +++ b/tests/ui/rfcs/rfc-1445-restrict-constants-in-patterns/issue-63479-match-fnptr.rs @@ -26,6 +26,7 @@ fn my_fn(_args: &[A]) { } const TEST: Fn = my_fn; +const TEST2: (Fn, u8) = (TEST, 0); struct B(Fn); @@ -33,8 +34,14 @@ fn main() { let s = B(my_fn); match s { B(TEST) => println!("matched"), - //~^ WARN behave unpredictably + //~^ WARN behave unpredictably //~| WARN this was previously accepted by the compiler but is being phased out _ => panic!("didn't match") }; + match (s.0, 0) { + TEST2 => println!("matched"), + //~^ WARN behave unpredictably + //~| WARN this was previously accepted by the compiler but is being phased out + _ => panic!("didn't match") + } } diff --git a/tests/ui/rfcs/rfc-1445-restrict-constants-in-patterns/issue-63479-match-fnptr.stderr b/tests/ui/rfcs/rfc-1445-restrict-constants-in-patterns/issue-63479-match-fnptr.stderr index 1aa627a8035..4fdfce60bb8 100644 --- a/tests/ui/rfcs/rfc-1445-restrict-constants-in-patterns/issue-63479-match-fnptr.stderr +++ b/tests/ui/rfcs/rfc-1445-restrict-constants-in-patterns/issue-63479-match-fnptr.stderr @@ -1,5 +1,5 @@ warning: function pointers and raw pointers not derived from integers 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 + --> $DIR/issue-63479-match-fnptr.rs:36:7 | LL | B(TEST) => println!("matched"), | ^^^^ @@ -12,5 +12,14 @@ note: the lint level is defined here LL | #![warn(pointer_structural_match)] | ^^^^^^^^^^^^^^^^^^^^^^^^ -warning: 1 warning emitted +warning: function pointers and raw pointers not derived from integers 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:42:5 + | +LL | TEST2 => println!("matched"), + | ^^^^^ + | + = 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 From 70a8e157ab485a775c1f648ad7967ffc3535c5b9 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Fri, 20 Oct 2023 08:39:54 +0200 Subject: [PATCH 3/5] make pointer_structural_match warn-by-default --- compiler/rustc_lint_defs/src/builtin.rs | 2 +- library/core/src/marker.rs | 1 + .../match/match-edge-cases_1.rs | 3 +- .../match/match-edge-cases_1.stderr | 12 +++ tests/ui/pattern/usefulness/consts-opaque.rs | 24 +++-- .../pattern/usefulness/consts-opaque.stderr | 84 +++++++++++++++-- .../fn-ptr-is-structurally-matchable.rs | 30 ++++-- .../fn-ptr-is-structurally-matchable.stderr | 93 +++++++++++++++++++ 8 files changed, 223 insertions(+), 26 deletions(-) create mode 100644 tests/ui/closures/2229_closure_analysis/match/match-edge-cases_1.stderr create mode 100644 tests/ui/rfcs/rfc-1445-restrict-constants-in-patterns/fn-ptr-is-structurally-matchable.stderr diff --git a/compiler/rustc_lint_defs/src/builtin.rs b/compiler/rustc_lint_defs/src/builtin.rs index f033504c523..bef9f469cc6 100644 --- a/compiler/rustc_lint_defs/src/builtin.rs +++ b/compiler/rustc_lint_defs/src/builtin.rs @@ -2226,7 +2226,7 @@ declare_lint! { /// in different crates and not deduplicated again via LTO. Pointer identity for memory /// created by `const` is similarly unreliable. pub POINTER_STRUCTURAL_MATCH, - Allow, + Warn, "pointers are not structural-match", @future_incompatible = FutureIncompatibleInfo { reason: FutureIncompatibilityReason::FutureReleaseErrorDontReportInDeps, diff --git a/library/core/src/marker.rs b/library/core/src/marker.rs index f1594501d40..9e49f146f73 100644 --- a/library/core/src/marker.rs +++ b/library/core/src/marker.rs @@ -247,6 +247,7 @@ marker_impls! { /// /// const CFN: Wrap = Wrap(higher_order); /// +/// #[allow(pointer_structural_match)] /// fn main() { /// match CFN { /// CFN => {} diff --git a/tests/ui/closures/2229_closure_analysis/match/match-edge-cases_1.rs b/tests/ui/closures/2229_closure_analysis/match/match-edge-cases_1.rs index 914ebbe26a5..106485e04ee 100644 --- a/tests/ui/closures/2229_closure_analysis/match/match-edge-cases_1.rs +++ b/tests/ui/closures/2229_closure_analysis/match/match-edge-cases_1.rs @@ -26,7 +26,8 @@ pub fn edge_case_str(event: String) { pub fn edge_case_raw_ptr(event: *const i32) { let _ = || { match event { - NUMBER_POINTER => (), + NUMBER_POINTER => (), //~WARN behave unpredictably + //~| previously accepted _ => (), }; }; diff --git a/tests/ui/closures/2229_closure_analysis/match/match-edge-cases_1.stderr b/tests/ui/closures/2229_closure_analysis/match/match-edge-cases_1.stderr new file mode 100644 index 00000000000..c83ba41976b --- /dev/null +++ b/tests/ui/closures/2229_closure_analysis/match/match-edge-cases_1.stderr @@ -0,0 +1,12 @@ +warning: function pointers and raw pointers not derived from integers in patterns behave unpredictably and should not be relied upon. See https://github.com/rust-lang/rust/issues/70861 for details. + --> $DIR/match-edge-cases_1.rs:29:13 + | +LL | NUMBER_POINTER => (), + | ^^^^^^^^^^^^^^ + | + = 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: `#[warn(pointer_structural_match)]` on by default + +warning: 1 warning emitted + diff --git a/tests/ui/pattern/usefulness/consts-opaque.rs b/tests/ui/pattern/usefulness/consts-opaque.rs index 6dc1425cf03..27e305a3972 100644 --- a/tests/ui/pattern/usefulness/consts-opaque.rs +++ b/tests/ui/pattern/usefulness/consts-opaque.rs @@ -95,8 +95,10 @@ fn main() { const QUUX: Quux = quux; match QUUX { - QUUX => {} - QUUX => {} + QUUX => {} //~WARN behave unpredictably + //~| previously accepted + QUUX => {} //~WARN behave unpredictably + //~| previously accepted _ => {} } @@ -105,14 +107,17 @@ fn main() { const WRAPQUUX: Wrap = Wrap(quux); match WRAPQUUX { - WRAPQUUX => {} - WRAPQUUX => {} + WRAPQUUX => {} //~WARN behave unpredictably + //~| previously accepted + WRAPQUUX => {} //~WARN behave unpredictably + //~| previously accepted Wrap(_) => {} } match WRAPQUUX { Wrap(_) => {} - WRAPQUUX => {} + WRAPQUUX => {} //~WARN behave unpredictably + //~| previously accepted } match WRAPQUUX { @@ -121,7 +126,8 @@ fn main() { match WRAPQUUX { //~^ ERROR: non-exhaustive patterns: `Wrap(_)` not covered - WRAPQUUX => {} + WRAPQUUX => {} //~WARN behave unpredictably + //~| previously accepted } #[derive(PartialEq, Eq)] @@ -132,9 +138,11 @@ fn main() { const WHOKNOWSQUUX: WhoKnows = WhoKnows::Yay(quux); match WHOKNOWSQUUX { - WHOKNOWSQUUX => {} + WHOKNOWSQUUX => {} //~WARN behave unpredictably + //~| previously accepted WhoKnows::Yay(_) => {} - WHOKNOWSQUUX => {} + WHOKNOWSQUUX => {} //~WARN behave unpredictably + //~| previously accepted WhoKnows::Nope => {} } } diff --git a/tests/ui/pattern/usefulness/consts-opaque.stderr b/tests/ui/pattern/usefulness/consts-opaque.stderr index 51f2f276bbe..09f72ba927e 100644 --- a/tests/ui/pattern/usefulness/consts-opaque.stderr +++ b/tests/ui/pattern/usefulness/consts-opaque.stderr @@ -91,24 +91,96 @@ LL | BAZ => {} = note: the traits must be derived, manual `impl`s are not sufficient = note: see https://doc.rust-lang.org/stable/std/marker/trait.StructuralEq.html for details +warning: function pointers and raw pointers not derived from integers in patterns behave unpredictably and should not be relied upon. See https://github.com/rust-lang/rust/issues/70861 for details. + --> $DIR/consts-opaque.rs:98:9 + | +LL | QUUX => {} + | ^^^^ + | + = 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: `#[warn(pointer_structural_match)]` on by default + +warning: function pointers and raw pointers not derived from integers in patterns behave unpredictably and should not be relied upon. See https://github.com/rust-lang/rust/issues/70861 for details. + --> $DIR/consts-opaque.rs:100:9 + | +LL | QUUX => {} + | ^^^^ + | + = 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 raw pointers not derived from integers in patterns behave unpredictably and should not be relied upon. See https://github.com/rust-lang/rust/issues/70861 for details. + --> $DIR/consts-opaque.rs:110:9 + | +LL | WRAPQUUX => {} + | ^^^^^^^^ + | + = 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 raw pointers not derived from integers in patterns behave unpredictably and should not be relied upon. See https://github.com/rust-lang/rust/issues/70861 for details. + --> $DIR/consts-opaque.rs:112:9 + | +LL | WRAPQUUX => {} + | ^^^^^^^^ + | + = 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 raw pointers not derived from integers in patterns behave unpredictably and should not be relied upon. See https://github.com/rust-lang/rust/issues/70861 for details. + --> $DIR/consts-opaque.rs:119:9 + | +LL | WRAPQUUX => {} + | ^^^^^^^^ + | + = 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 raw pointers not derived from integers in patterns behave unpredictably and should not be relied upon. See https://github.com/rust-lang/rust/issues/70861 for details. + --> $DIR/consts-opaque.rs:129:9 + | +LL | WRAPQUUX => {} + | ^^^^^^^^ + | + = 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 raw pointers not derived from integers in patterns behave unpredictably and should not be relied upon. See https://github.com/rust-lang/rust/issues/70861 for details. + --> $DIR/consts-opaque.rs:141:9 + | +LL | WHOKNOWSQUUX => {} + | ^^^^^^^^^^^^ + | + = 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 raw pointers not derived from integers in patterns behave unpredictably and should not be relied upon. See https://github.com/rust-lang/rust/issues/70861 for details. + --> $DIR/consts-opaque.rs:144:9 + | +LL | WHOKNOWSQUUX => {} + | ^^^^^^^^^^^^ + | + = 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[E0004]: non-exhaustive patterns: `Wrap(_)` not covered - --> $DIR/consts-opaque.rs:122:11 + --> $DIR/consts-opaque.rs:127:11 | LL | match WRAPQUUX { | ^^^^^^^^ pattern `Wrap(_)` not covered | note: `Wrap usize>` defined here - --> $DIR/consts-opaque.rs:104:12 + --> $DIR/consts-opaque.rs:106:12 | LL | struct Wrap(T); | ^^^^ = note: the matched value is of type `Wrap usize>` help: ensure that all possible cases are being handled by adding a match arm with a wildcard pattern or an explicit pattern as shown | -LL ~ WRAPQUUX => {}, -LL + Wrap(_) => todo!() - | +LL | WRAPQUUX => {}, Wrap(_) => todo!() + | ++++++++++++++++++++ -error: aborting due to 10 previous errors; 1 warning emitted +error: aborting due to 10 previous errors; 9 warnings emitted For more information about this error, try `rustc --explain E0004`. diff --git a/tests/ui/rfcs/rfc-1445-restrict-constants-in-patterns/fn-ptr-is-structurally-matchable.rs b/tests/ui/rfcs/rfc-1445-restrict-constants-in-patterns/fn-ptr-is-structurally-matchable.rs index 2b3fbd2a4d2..e591b2a93e1 100644 --- a/tests/ui/rfcs/rfc-1445-restrict-constants-in-patterns/fn-ptr-is-structurally-matchable.rs +++ b/tests/ui/rfcs/rfc-1445-restrict-constants-in-patterns/fn-ptr-is-structurally-matchable.rs @@ -40,7 +40,8 @@ fn main() { const CFN1: Wrap = Wrap(trivial); let input: Wrap = Wrap(trivial); match Wrap(input) { - Wrap(CFN1) => count += 1, + Wrap(CFN1) => count += 1, //~WARN behave unpredictably + //~| previously accepted Wrap(_) => {} }; @@ -48,7 +49,8 @@ fn main() { const CFN2: Wrap = Wrap(sm_to); let input: Wrap = Wrap(sm_to); match Wrap(input) { - Wrap(CFN2) => count += 1, + Wrap(CFN2) => count += 1, //~WARN behave unpredictably + //~| previously accepted Wrap(_) => {} }; @@ -56,7 +58,8 @@ fn main() { const CFN3: Wrap SM> = Wrap(to_sm); let input: Wrap SM> = Wrap(to_sm); match Wrap(input) { - Wrap(CFN3) => count += 1, + Wrap(CFN3) => count += 1, //~WARN behave unpredictably + //~| previously accepted Wrap(_) => {} }; @@ -64,7 +67,8 @@ fn main() { const CFN4: Wrap = Wrap(not_sm_to); let input: Wrap = Wrap(not_sm_to); match Wrap(input) { - Wrap(CFN4) => count += 1, + Wrap(CFN4) => count += 1, //~WARN behave unpredictably + //~| previously accepted Wrap(_) => {} }; @@ -72,7 +76,8 @@ fn main() { const CFN5: Wrap NotSM> = Wrap(to_not_sm); let input: Wrap NotSM> = Wrap(to_not_sm); match Wrap(input) { - Wrap(CFN5) => count += 1, + Wrap(CFN5) => count += 1, //~WARN behave unpredictably + //~| previously accepted Wrap(_) => {} }; @@ -80,7 +85,8 @@ fn main() { const CFN6: Wrap = Wrap(r_sm_to); let input: Wrap = Wrap(r_sm_to); match Wrap(input) { - Wrap(CFN6) => count += 1, + Wrap(CFN6) => count += 1, //~WARN behave unpredictably + //~| previously accepted Wrap(_) => {} }; @@ -88,7 +94,8 @@ fn main() { const CFN7: Wrap &SM> = Wrap(r_to_r_sm); let input: Wrap &SM> = Wrap(r_to_r_sm); match Wrap(input) { - Wrap(CFN7) => count += 1, + Wrap(CFN7) => count += 1, //~WARN behave unpredictably + //~| previously accepted Wrap(_) => {} }; @@ -96,7 +103,8 @@ fn main() { const CFN8: Wrap = Wrap(r_not_sm_to); let input: Wrap = Wrap(r_not_sm_to); match Wrap(input) { - Wrap(CFN8) => count += 1, + Wrap(CFN8) => count += 1, //~WARN behave unpredictably + //~| previously accepted Wrap(_) => {} }; @@ -104,7 +112,8 @@ fn main() { const CFN9: Wrap &NotSM> = Wrap(r_to_r_not_sm); let input: Wrap &NotSM> = Wrap(r_to_r_not_sm); match Wrap(input) { - Wrap(CFN9) => count += 1, + Wrap(CFN9) => count += 1, //~WARN behave unpredictably + //~| previously accepted Wrap(_) => {} }; @@ -126,7 +135,8 @@ fn main() { let input = Foo { alpha: not_sm_to, beta: to_not_sm, gamma: sm_to, delta: to_sm }; match input { - CFOO => count += 1, + CFOO => count += 1, //~WARN behave unpredictably + //~| previously accepted Foo { .. } => {} }; diff --git a/tests/ui/rfcs/rfc-1445-restrict-constants-in-patterns/fn-ptr-is-structurally-matchable.stderr b/tests/ui/rfcs/rfc-1445-restrict-constants-in-patterns/fn-ptr-is-structurally-matchable.stderr new file mode 100644 index 00000000000..080bf5885ba --- /dev/null +++ b/tests/ui/rfcs/rfc-1445-restrict-constants-in-patterns/fn-ptr-is-structurally-matchable.stderr @@ -0,0 +1,93 @@ +warning: function pointers and raw pointers not derived from integers in patterns behave unpredictably and should not be relied upon. See https://github.com/rust-lang/rust/issues/70861 for details. + --> $DIR/fn-ptr-is-structurally-matchable.rs:43:14 + | +LL | Wrap(CFN1) => count += 1, + | ^^^^ + | + = 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: `#[warn(pointer_structural_match)]` on by default + +warning: function pointers and raw pointers not derived from integers in patterns behave unpredictably and should not be relied upon. See https://github.com/rust-lang/rust/issues/70861 for details. + --> $DIR/fn-ptr-is-structurally-matchable.rs:52:14 + | +LL | Wrap(CFN2) => count += 1, + | ^^^^ + | + = 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 raw pointers not derived from integers in patterns behave unpredictably and should not be relied upon. See https://github.com/rust-lang/rust/issues/70861 for details. + --> $DIR/fn-ptr-is-structurally-matchable.rs:61:14 + | +LL | Wrap(CFN3) => count += 1, + | ^^^^ + | + = 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 raw pointers not derived from integers in patterns behave unpredictably and should not be relied upon. See https://github.com/rust-lang/rust/issues/70861 for details. + --> $DIR/fn-ptr-is-structurally-matchable.rs:70:14 + | +LL | Wrap(CFN4) => count += 1, + | ^^^^ + | + = 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 raw pointers not derived from integers in patterns behave unpredictably and should not be relied upon. See https://github.com/rust-lang/rust/issues/70861 for details. + --> $DIR/fn-ptr-is-structurally-matchable.rs:79:14 + | +LL | Wrap(CFN5) => count += 1, + | ^^^^ + | + = 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 raw pointers not derived from integers in patterns behave unpredictably and should not be relied upon. See https://github.com/rust-lang/rust/issues/70861 for details. + --> $DIR/fn-ptr-is-structurally-matchable.rs:88:14 + | +LL | Wrap(CFN6) => count += 1, + | ^^^^ + | + = 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 raw pointers not derived from integers in patterns behave unpredictably and should not be relied upon. See https://github.com/rust-lang/rust/issues/70861 for details. + --> $DIR/fn-ptr-is-structurally-matchable.rs:97:14 + | +LL | Wrap(CFN7) => count += 1, + | ^^^^ + | + = 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 raw pointers not derived from integers in patterns behave unpredictably and should not be relied upon. See https://github.com/rust-lang/rust/issues/70861 for details. + --> $DIR/fn-ptr-is-structurally-matchable.rs:106:14 + | +LL | Wrap(CFN8) => count += 1, + | ^^^^ + | + = 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 raw pointers not derived from integers in patterns behave unpredictably and should not be relied upon. See https://github.com/rust-lang/rust/issues/70861 for details. + --> $DIR/fn-ptr-is-structurally-matchable.rs:115:14 + | +LL | Wrap(CFN9) => count += 1, + | ^^^^ + | + = 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 raw pointers not derived from integers in patterns behave unpredictably and should not be relied upon. See https://github.com/rust-lang/rust/issues/70861 for details. + --> $DIR/fn-ptr-is-structurally-matchable.rs:138:9 + | +LL | CFOO => count += 1, + | ^^^^ + | + = 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: 10 warnings emitted + From 03b24f275607672fce4577a6a3edbb18d9b1977d Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 30 Oct 2023 09:11:52 +0100 Subject: [PATCH 4/5] remove some dead code --- .../rustc_mir_build/src/thir/pattern/const_to_pat.rs | 10 ---------- 1 file changed, 10 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 53823c9226c..a5e7cae6934 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 @@ -211,16 +211,6 @@ impl<'tcx> ConstToPat<'tcx> { } else if !self.saw_const_match_lint.get() { if let Some(mir_structural_match_violation) = mir_structural_match_violation { match non_sm_ty.kind() { - ty::RawPtr(pointee) - if pointee.ty.is_sized(self.tcx(), self.param_env) => {} - ty::FnPtr(..) | ty::RawPtr(..) => { - self.tcx().emit_spanned_lint( - lint::builtin::POINTER_STRUCTURAL_MATCH, - self.id, - self.span, - PointerPattern, - ); - } ty::Adt(..) if mir_structural_match_violation => { self.tcx().emit_spanned_lint( lint::builtin::INDIRECT_STRUCTURAL_MATCH, From 30588657b753b9ae500b3514d58da18a4a155b08 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Wed, 8 Nov 2023 20:37:08 +0100 Subject: [PATCH 5/5] avoid unnecessary nested conditionals --- .../src/thir/pattern/const_to_pat.rs | 20 +++++++++---------- 1 file changed, 9 insertions(+), 11 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 a5e7cae6934..48a590f5d37 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 @@ -228,17 +228,15 @@ impl<'tcx> ConstToPat<'tcx> { } } } - } else if !self.saw_const_match_lint.get() { - if !have_valtree { - // The only way valtree construction can fail without the structural match - // checker finding a violation is if there is a pointer somewhere. - self.tcx().emit_spanned_lint( - lint::builtin::POINTER_STRUCTURAL_MATCH, - self.id, - self.span, - PointerPattern, - ); - } + } else if !have_valtree && !self.saw_const_match_lint.get() { + // The only way valtree construction can fail without the structural match + // checker finding a violation is if there is a pointer somewhere. + self.tcx().emit_spanned_lint( + lint::builtin::POINTER_STRUCTURAL_MATCH, + self.id, + self.span, + PointerPattern, + ); } // Always check for `PartialEq`, even if we emitted other lints. (But not if there were