-
Notifications
You must be signed in to change notification settings - Fork 12.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
patterns: reject raw pointers that are not just integers #116930
Changes from 4 commits
bec88ad
af6c7e0
70a8e15
03b24f2
3058865
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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(_) | ||
|
@@ -209,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, | ||
|
@@ -237,17 +229,15 @@ 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::FnPtr(..) | ty::RawPtr(..) => { | ||
self.tcx().emit_spanned_lint( | ||
lint::builtin::POINTER_STRUCTURAL_MATCH, | ||
self.id, | ||
self.span, | ||
PointerPattern, | ||
); | ||
} | ||
_ => {} | ||
if !have_valtree { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: could be part of the condition on the previous line |
||
// 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, | ||
); | ||
} | ||
} | ||
|
||
|
@@ -389,11 +379,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 +478,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); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -247,6 +247,7 @@ marker_impls! { | |
/// | ||
/// const CFN: Wrap<fn(&())> = Wrap(higher_order); | ||
/// | ||
/// #[allow(pointer_structural_match)] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. FWIW these entire trait docs are outdated; that's tracked in #115881. |
||
/// fn main() { | ||
/// match CFN { | ||
/// CFN => {} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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 <https://github.com/rust-lang/rust/issues/70861> | ||
= note: `#[warn(pointer_structural_match)]` on by default | ||
|
||
warning: 1 warning emitted | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,32 @@ | ||
#![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 { | ||
C => {} //~ERROR: behave unpredictably | ||
//~| previously accepted | ||
_ => {} | ||
} | ||
} | ||
|
||
fn foo2(x: *const u8) { | ||
match (x, 1) { | ||
C_INNER => {} //~ERROR: behave unpredictably | ||
//~| previously accepted | ||
_ => {} | ||
} | ||
} | ||
|
||
const D: *const [u8; 4] = b"abcd"; | ||
|
||
fn main() { | ||
match D { | ||
D => {} //~ERROR: behave unpredictably | ||
//~| previously accepted | ||
_ => {} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,34 @@ | ||
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:10: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 <https://github.com/rust-lang/rust/issues/70861> | ||
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: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 <https://github.com/rust-lang/rust/issues/70861> | ||
|
||
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 => {} | ||
| ^ | ||
| | ||
= 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 <https://github.com/rust-lang/rust/issues/70861> | ||
|
||
error: aborting due to 3 previous errors | ||
|
This file was deleted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't realize this lint is still allow-by-default oO. Seems high time we make it warn-by-default?