Skip to content

Don't depend on Allocation sizes for pattern length #56540

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 8 commits into from
Dec 15, 2018
Merged

Don't depend on Allocation sizes for pattern length #56540

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
a5a7fcb
Don't depend on `Allocation` sizes for pattern length
oli-obk Dec 5, 2018
8f2ce3d
Document and simplify pattern matching with constants as patterns
oli-obk Dec 7, 2018
8a9414a
Add a few more tests of edge cases
oli-obk Dec 7, 2018
7d1b19c
Diagnostics improvement!
oli-obk Dec 11, 2018
46c1da6
Doc comment format
oli-obk Dec 13, 2018
25a4f76
Fix indent
oli-obk Dec 13, 2018
5df6af4
Update tests to show diagnostics
oli-obk Dec 13, 2018
1c2a29e
Expand on a few comments
oli-obk Dec 13, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
186 changes: 133 additions & 53 deletions src/librustc_mir/hair/pattern/_match.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -178,11 +178,11 @@ use super::{PatternFoldable, PatternFolder, compare_const_vals};

use rustc::hir::def_id::DefId;
use rustc::hir::RangeEnd;
use rustc::ty::{self, Ty, TyCtxt, TypeFoldable};
use rustc::ty::layout::{Integer, IntegerExt, VariantIdx};
use rustc::ty::{self, Ty, TyCtxt, TypeFoldable, Const};
use rustc::ty::layout::{Integer, IntegerExt, VariantIdx, Size};

use rustc::mir::Field;
use rustc::mir::interpret::ConstValue;
use rustc::mir::interpret::{ConstValue, Pointer, Scalar};
use rustc::util::common::ErrorReported;

use syntax::attr::{SignedInt, UnsignedInt};
Expand All @@ -200,22 +200,72 @@ use std::u128;
pub fn expand_pattern<'a, 'tcx>(cx: &MatchCheckCtxt<'a, 'tcx>, pat: Pattern<'tcx>)
-> &'a Pattern<'tcx>
{
cx.pattern_arena.alloc(LiteralExpander.fold_pattern(&pat))
cx.pattern_arena.alloc(LiteralExpander { tcx: cx.tcx }.fold_pattern(&pat))
}

struct LiteralExpander;
impl<'tcx> PatternFolder<'tcx> for LiteralExpander {
struct LiteralExpander<'a, 'tcx> {
tcx: TyCtxt<'a, 'tcx, 'tcx>
}

impl<'a, 'tcx> LiteralExpander<'a, 'tcx> {
/// Derefs `val` and potentially unsizes the value if `crty` is an array and `rty` a slice
///
/// `crty` and `rty` can differ because you can use array constants in the presence of slice
/// patterns. So the pattern may end up being a slice, but the constant is an array. We convert
/// the array to a slice in that case
Copy link
Member

Choose a reason for hiding this comment

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

Nit: end comments with full stops (here and elsewhere).

fn fold_const_value_deref(
&mut self,
val: ConstValue<'tcx>,
// the pattern's pointee type
rty: Ty<'tcx>,
// the constant's pointee type
crty: Ty<'tcx>,
) -> ConstValue<'tcx> {
match (val, &crty.sty, &rty.sty) {
// the easy case, deref a reference
(ConstValue::Scalar(Scalar::Ptr(p)), x, y) if x == y => ConstValue::ByRef(
p.alloc_id,
self.tcx.alloc_map.lock().unwrap_memory(p.alloc_id),
p.offset,
),
// unsize array to slice if pattern is array but match value or other patterns are slice
(ConstValue::Scalar(Scalar::Ptr(p)), ty::Array(t, n), ty::Slice(u)) => {
assert_eq!(t, u);
ConstValue::ScalarPair(
Scalar::Ptr(p),
n.val.try_to_scalar().unwrap(),
)
},
// fat pointers stay the same
(ConstValue::ScalarPair(..), _, _) => val,
// FIXME(oli-obk): this is reachable for `const FOO: &&&u32 = &&&42;` being used
Copy link
Member

Choose a reason for hiding this comment

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

What happens in this case in the existing code? A bug again?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Currently this is an ICE: https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=76fe5897409eefe240b587fb5ba59f6e

It will stay an ICE, but this PR is prepping the road for making it saner, because it's an explicit choice to ICE instead of just a random assert triggering.

_ => bug!("cannot deref {:#?}, {} -> {}", val, crty, rty),
}
}
}

impl<'a, 'tcx> PatternFolder<'tcx> for LiteralExpander<'a, 'tcx> {
fn fold_pattern(&mut self, pat: &Pattern<'tcx>) -> Pattern<'tcx> {
match (&pat.ty.sty, &*pat.kind) {
(&ty::Ref(_, rty, _), &PatternKind::Constant { ref value }) => {
(
&ty::Ref(_, rty, _),
&PatternKind::Constant { value: Const {
val,
ty: ty::TyS { sty: ty::Ref(_, crty, _), .. },
} },
) => {
Pattern {
ty: pat.ty,
span: pat.span,
kind: box PatternKind::Deref {
subpattern: Pattern {
ty: rty,
span: pat.span,
kind: box PatternKind::Constant { value: value.clone() },
kind: box PatternKind::Constant { value: Const::from_const_value(
self.tcx,
self.fold_const_value_deref(*val, rty, crty),
rty,
) },
}
}
}
Expand Down Expand Up @@ -732,15 +782,17 @@ fn max_slice_length<'p, 'a: 'p, 'tcx: 'a, I>(
for row in patterns {
match *row.kind {
PatternKind::Constant { value } => {
if let Some(ptr) = value.to_ptr() {
let is_array_ptr = value.ty
.builtin_deref(true)
.and_then(|t| t.ty.builtin_index())
.map_or(false, |t| t == cx.tcx.types.u8);
if is_array_ptr {
let alloc = cx.tcx.alloc_map.lock().unwrap_memory(ptr.alloc_id);
max_fixed_len = cmp::max(max_fixed_len, alloc.bytes.len() as u64);
}
// extract the length of an array/slice from a constant
match (value.val, &value.ty.sty) {
(_, ty::Array(_, n)) => max_fixed_len = cmp::max(
max_fixed_len,
n.unwrap_usize(cx.tcx),
),
(ConstValue::ScalarPair(_, n), ty::Slice(_)) => max_fixed_len = cmp::max(
max_fixed_len,
n.to_usize(&cx.tcx).unwrap(),
),
_ => {},
}
}
PatternKind::Slice { ref prefix, slice: None, ref suffix } => {
Expand Down Expand Up @@ -1348,28 +1400,56 @@ fn constructor_sub_pattern_tys<'a, 'tcx: 'a>(cx: &MatchCheckCtxt<'a, 'tcx>,
}
}

fn slice_pat_covered_by_constructor<'tcx>(
// checks whether a constant is equal to a user-written slice pattern. Only supports byte slices,
// meaning all other types will compare unequal and thus equal patterns often do not cause the
// second pattern to lint about unreachable match arms.
fn slice_pat_covered_by_const<'tcx>(
tcx: TyCtxt<'_, 'tcx, '_>,
_span: Span,
ctor: &Constructor,
const_val: &ty::Const<'tcx>,
prefix: &[Pattern<'tcx>],
slice: &Option<Pattern<'tcx>>,
suffix: &[Pattern<'tcx>]
) -> Result<bool, ErrorReported> {
let data: &[u8] = match *ctor {
ConstantValue(const_val) => {
let val = match const_val.val {
ConstValue::Unevaluated(..) |
ConstValue::ByRef(..) => bug!("unexpected ConstValue: {:?}", const_val),
ConstValue::Scalar(val) | ConstValue::ScalarPair(val, _) => val,
};
if let Ok(ptr) = val.to_ptr() {
tcx.alloc_map.lock().unwrap_memory(ptr.alloc_id).bytes.as_ref()
} else {
bug!("unexpected non-ptr ConstantValue")
let data: &[u8] = match (const_val.val, &const_val.ty.sty) {
(ConstValue::ByRef(id, alloc, offset), ty::Array(t, n)) => {
if *t != tcx.types.u8 {
// FIXME(oli-obk): can't mix const patterns with slice patterns and get
// any sort of exhaustiveness/unreachable check yet
return Ok(false);
}
}
_ => bug!()
let ptr = Pointer::new(id, offset);
let n = n.assert_usize(tcx).unwrap();
alloc.get_bytes(&tcx, ptr, Size::from_bytes(n)).unwrap()
},
// a slice fat pointer to a zero length slice
(ConstValue::ScalarPair(Scalar::Bits { .. }, n), ty::Slice(t)) => {
if *t != tcx.types.u8 {
// FIXME(oli-obk): can't mix const patterns with slice patterns and get
// any sort of exhaustiveness/unreachable check yet
return Ok(false);
}
assert_eq!(n.to_usize(&tcx).unwrap(), 0);
&[]
},
//
(ConstValue::ScalarPair(Scalar::Ptr(ptr), n), ty::Slice(t)) => {
if *t != tcx.types.u8 {
// FIXME(oli-obk): can't mix const patterns with slice patterns and get
// any sort of exhaustiveness/unreachable check yet
return Ok(false);
}
let n = n.to_usize(&tcx).unwrap();
tcx.alloc_map
.lock()
.unwrap_memory(ptr.alloc_id)
.get_bytes(&tcx, ptr, Size::from_bytes(n))
.unwrap()
},
_ => bug!(
"slice_pat_covered_by_const: {:#?}, {:#?}, {:#?}, {:#?}",
const_val, prefix, slice, suffix,
),
};

let pat_len = prefix.len() + suffix.len();
Expand Down Expand Up @@ -1675,22 +1755,23 @@ fn specialize<'p, 'a: 'p, 'tcx: 'a>(
// necessarily point to memory, they are usually just integers. The only time
// they should be pointing to memory is when they are subslices of nonzero
// slices
let (opt_ptr, n, ty) = match value.ty.builtin_deref(false).unwrap().ty.sty {
ty::TyKind::Array(t, n) => (value.to_ptr(), n.unwrap_usize(cx.tcx), t),
ty::TyKind::Slice(t) => {
match value.val {
ConstValue::ScalarPair(ptr, n) => (
ptr.to_ptr().ok(),
n.to_bits(cx.tcx.data_layout.pointer_size).unwrap() as u64,
t,
),
_ => span_bug!(
pat.span,
"slice pattern constant must be scalar pair but is {:?}",
value,
),
}
},
let (opt_ptr, n, ty) = match (value.val, &value.ty.sty) {
(ConstValue::ByRef(id, alloc, offset), ty::TyKind::Array(t, n)) => (
Some((
Pointer::new(id, offset),
alloc,
)),
n.unwrap_usize(cx.tcx),
t,
),
(ConstValue::ScalarPair(ptr, n), ty::TyKind::Slice(t)) => (
ptr.to_ptr().ok().map(|ptr| (
ptr,
cx.tcx.alloc_map.lock().unwrap_memory(ptr.alloc_id),
)),
n.to_bits(cx.tcx.data_layout.pointer_size).unwrap() as u64,
t,
),
_ => span_bug!(
pat.span,
"unexpected const-val {:?} with ctor {:?}",
Expand All @@ -1702,8 +1783,7 @@ fn specialize<'p, 'a: 'p, 'tcx: 'a>(
// convert a constant slice/array pattern to a list of patterns.
match (n, opt_ptr) {
(0, _) => Some(SmallVec::new()),
(_, Some(ptr)) => {
let alloc = cx.tcx.alloc_map.lock().unwrap_memory(ptr.alloc_id);
(_, Some((ptr, alloc))) => {
let layout = cx.tcx.layout_of(cx.param_env.and(ty)).ok()?;
(0..n).map(|i| {
let ptr = ptr.offset(layout.size * i, &cx.tcx).ok()?;
Expand Down Expand Up @@ -1766,9 +1846,9 @@ fn specialize<'p, 'a: 'p, 'tcx: 'a>(
None
}
}
ConstantValue(..) => {
match slice_pat_covered_by_constructor(
cx.tcx, pat.span, constructor, prefix, slice, suffix
ConstantValue(cv) => {
match slice_pat_covered_by_const(
cx.tcx, pat.span, cv, prefix, slice, suffix
) {
Ok(true) => Some(smallvec![]),
Ok(false) => None,
Expand Down
44 changes: 21 additions & 23 deletions src/librustc_mir/hair/pattern/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1259,34 +1259,32 @@ pub fn compare_const_vals<'a, 'tcx>(
}
}

if let ty::Ref(_, rty, _) = ty.value.sty {
if let ty::Str = rty.sty {
match (a.val, b.val) {
(
ConstValue::ScalarPair(
Scalar::Ptr(ptr_a),
len_a,
),
ConstValue::ScalarPair(
Scalar::Ptr(ptr_b),
len_b,
),
) if ptr_a.offset.bytes() == 0 && ptr_b.offset.bytes() == 0 => {
if let Ok(len_a) = len_a.to_bits(tcx.data_layout.pointer_size) {
if let Ok(len_b) = len_b.to_bits(tcx.data_layout.pointer_size) {
if len_a == len_b {
let map = tcx.alloc_map.lock();
let alloc_a = map.unwrap_memory(ptr_a.alloc_id);
let alloc_b = map.unwrap_memory(ptr_b.alloc_id);
if alloc_a.bytes.len() as u128 == len_a {
return from_bool(alloc_a == alloc_b);
}
if let ty::Str = ty.value.sty {
match (a.val, b.val) {
(
ConstValue::ScalarPair(
Scalar::Ptr(ptr_a),
len_a,
),
ConstValue::ScalarPair(
Scalar::Ptr(ptr_b),
len_b,
),
) if ptr_a.offset.bytes() == 0 && ptr_b.offset.bytes() == 0 => {
if let Ok(len_a) = len_a.to_bits(tcx.data_layout.pointer_size) {
if let Ok(len_b) = len_b.to_bits(tcx.data_layout.pointer_size) {
if len_a == len_b {
let map = tcx.alloc_map.lock();
let alloc_a = map.unwrap_memory(ptr_a.alloc_id);
let alloc_b = map.unwrap_memory(ptr_b.alloc_id);
if alloc_a.bytes.len() as u128 == len_a {
return from_bool(alloc_a == alloc_b);
}
}
}
}
_ => (),
}
_ => (),
}
}

Expand Down
3 changes: 2 additions & 1 deletion src/test/run-pass/ctfe/references.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,10 @@ fn main() {
_ => panic!("c"),
}

#[allow(unreachable_patterns)]
match &43 {
&42 => panic!(),
BOO => panic!(),
BOO => panic!(), // pattern is unreachable
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a behavioral change of this PR. We now detect that BOO is &42. This solely emits new warnings about unreachable patterns.

_ => println!("d"),
}
}
7 changes: 7 additions & 0 deletions src/test/ui/pattern/slice-pattern-const-2.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,4 +21,11 @@ fn main() {
MAGIC_TEST => (), // this should warn
_ => (),
}
const FOO: [u32; 1] = [4];
match [99] {
[0x00] => (),
[4] => (),
FOO => (), // this should warn
_ => (),
}
}
7 changes: 7 additions & 0 deletions src/test/ui/pattern/slice-pattern-const-3.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,4 +21,11 @@ fn main() {
MAGIC_TEST => (), // this should warn
_ => (),
}
const FOO: [&str; 1] = ["boo"];
match ["baa"] {
["0x00"] => (),
["boo"] => (),
FOO => (), // this should warn
_ => (),
}
}
23 changes: 23 additions & 0 deletions src/test/ui/pattern/slice-pattern-const.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,4 +21,27 @@ fn main() {
MAGIC_TEST => (), // this should warn
_ => (),
}
const FOO: [u8; 1] = [4];
match [99] {
[0x00] => (),
[4] => (),
FOO => (), // this should warn
_ => (),
}
const BAR: &[u8; 1] = &[4];
match &[99] {
[0x00] => (),
[4] => (),
BAR => (), // this should warn
b"a" => (),
_ => (),
}

const BOO: &[u8; 0] = &[];
match &[] {
[] => (),
BOO => (), // this should warn
b"" => (),
_ => (),
}
}