Skip to content
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

Improve MIR match generation for ranges #56810

Merged
merged 6 commits into from
Dec 17, 2018
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
120 changes: 118 additions & 2 deletions src/librustc_mir/build/matches/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
use build::Builder;
use build::matches::{Candidate, MatchPair, Test, TestKind};
use hair::*;
use hair::pattern::compare_const_vals;
use rustc_data_structures::bit_set::BitSet;
use rustc_data_structures::fx::FxHashMap;
use rustc::ty::{self, Ty};
Expand Down Expand Up @@ -136,7 +137,15 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
PatternKind::Variant { .. } => {
panic!("you should have called add_variants_to_switch instead!");
}
PatternKind::Range { .. } |
PatternKind::Range { ty, lo, hi, end } => {
indices
sinkuu marked this conversation as resolved.
Show resolved Hide resolved
.keys()
.all(|value| {
!self
.const_range_contains(ty, lo, hi, end, value)
.unwrap_or(true)
})
}
PatternKind::Slice { .. } |
PatternKind::Array { .. } |
PatternKind::Wild |
Expand Down Expand Up @@ -529,6 +538,28 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
resulting_candidates[index].push(new_candidate);
true
}

(&TestKind::SwitchInt { switch_ty: _, ref options, ref indices },
&PatternKind::Range { ty, lo, hi, end }) => {
let not_contained = indices
sinkuu marked this conversation as resolved.
Show resolved Hide resolved
.keys()
.all(|value| {
!self
.const_range_contains(ty, lo, hi, end, value)
.unwrap_or(true)
});

if not_contained {
// No values are contained in the pattern range,
// so the pattern can be matched only if this test fails.
let otherwise = options.len();
resulting_candidates[otherwise].push(candidate.clone());
true
} else {
false
}
}

(&TestKind::SwitchInt { .. }, _) => false,


Expand Down Expand Up @@ -607,8 +638,70 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
}
}

(&TestKind::Range {
lo: test_lo, hi: test_hi, ty: test_ty, end: test_end,
}, &PatternKind::Range {
lo: pat_lo, hi: pat_hi, ty: _, end: pat_end,
}) => {
if (test_lo, test_hi, test_end) == (pat_lo, pat_hi, pat_end) {
sinkuu marked this conversation as resolved.
Show resolved Hide resolved
resulting_candidates[0]
.push(self.candidate_without_match_pair(
match_pair_index,
candidate,
));
return true;
}

let no_overlap = (|| {
Copy link
Contributor

Choose a reason for hiding this comment

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

we can use the try syntax inside the compiler.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

try syntax seems to require not only #![feature(try_block)] but also --edition 2018 for try keyword.

use std::cmp::Ordering::*;
use rustc::hir::RangeEnd::*;

let param_env = ty::ParamEnv::empty().and(test_ty);
let tcx = self.hir.tcx();

let lo = compare_const_vals(tcx, test_lo, pat_hi, param_env)?;
let hi = compare_const_vals(tcx, test_hi, pat_lo, param_env)?;

match (test_end, pat_end, lo, hi) {
// pat < test
(_, _, Greater, _) |
(_, Excluded, Equal, _) |
// pat > test
(_, _, _, Less) |
(Excluded, _, _, Equal) => Some(true),
_ => Some(false),
}
})();

if no_overlap == Some(true) {
// Testing range does not overlap with pattern range,
// so the pattern can be matched only if this test fails.
resulting_candidates[1].push(candidate.clone());
true
} else {
false
}
}

(&TestKind::Range {
lo, hi, ty, end
}, &PatternKind::Constant {
ref value
}) => {
if self.const_range_contains(ty, lo, hi, end, value) == Some(false) {
// `value` is not contained in the testing range,
// so `value` can be matched only if this test fails.
resulting_candidates[1].push(candidate.clone());
true
} else {
false
}
}

(&TestKind::Range { .. }, _) => false,


(&TestKind::Eq { .. }, _) |
(&TestKind::Range { .. }, _) |
(&TestKind::Len { .. }, _) => {
// These are all binary tests.
//
Expand Down Expand Up @@ -719,6 +812,29 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
"simplifyable pattern found: {:?}",
match_pair.pattern)
}

fn const_range_contains(
&self,
ty: Ty<'tcx>,
lo: &'tcx ty::Const<'tcx>,
hi: &'tcx ty::Const<'tcx>,
end: RangeEnd,
value: &'tcx ty::Const<'tcx>,
) -> Option<bool> {
use std::cmp::Ordering::*;

let param_env = ty::ParamEnv::empty().and(ty);
let tcx = self.hir.tcx();

let a = compare_const_vals(tcx, lo, value, param_env)?;
let b = compare_const_vals(tcx, value, hi, param_env)?;

match (b, end) {
(Less, _) |
(Equal, RangeEnd::Included) if a != Greater => Some(true),
_ => Some(false),
}
}
}

fn is_switch_ty<'tcx>(ty: Ty<'tcx>) -> bool {
Expand Down
9 changes: 6 additions & 3 deletions src/librustc_mir/hair/pattern/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ use hair::constant::*;
use rustc::mir::{fmt_const_val, Field, BorrowKind, Mutability};
use rustc::mir::{ProjectionElem, UserTypeAnnotation, UserTypeProjection, UserTypeProjections};
use rustc::mir::interpret::{Scalar, GlobalId, ConstValue, sign_extend};
use rustc::ty::{self, Region, TyCtxt, AdtDef, Ty};
use rustc::ty::{self, Region, TyCtxt, AdtDef, Ty, Lift};
use rustc::ty::subst::{Substs, Kind};
use rustc::ty::layout::VariantIdx;
use rustc::hir::{self, PatKind, RangeEnd};
Expand Down Expand Up @@ -1210,8 +1210,8 @@ impl<'tcx> PatternFoldable<'tcx> for PatternKind<'tcx> {
}
}

pub fn compare_const_vals<'a, 'tcx>(
tcx: TyCtxt<'a, 'tcx, 'tcx>,
pub fn compare_const_vals<'a, 'gcx, 'tcx>(
tcx: TyCtxt<'a, 'gcx, 'tcx>,
a: &'tcx ty::Const<'tcx>,
b: &'tcx ty::Const<'tcx>,
ty: ty::ParamEnvAnd<'tcx, Ty<'tcx>>,
Expand All @@ -1233,6 +1233,9 @@ pub fn compare_const_vals<'a, 'tcx>(
return fallback();
}

let tcx = tcx.global_tcx();
let (a, b, ty) = (a, b, ty).lift_to_tcx(tcx).unwrap();

// FIXME: This should use assert_bits(ty) instead of use_bits
// but triggers possibly bugs due to mismatching of arrays and slices
if let (Some(a), Some(b)) = (a.to_bits(tcx, ty), b.to_bits(tcx, ty)) {
Expand Down
83 changes: 83 additions & 0 deletions src/test/run-pass/mir/mir_match_test.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
#![feature(exclusive_range_pattern)]

// run-pass

fn main() {
let incl_range = |x, b| {
match x {
0..=5 if b => 0,
5..=10 if b => 1,
1..=4 if !b => 2,
_ => 3,
}
};
assert_eq!(incl_range(3, false), 2);
assert_eq!(incl_range(3, true), 0);
assert_eq!(incl_range(5, false), 3);
assert_eq!(incl_range(5, true), 0);

let excl_range = |x, b| {
match x {
0..5 if b => 0,
5..10 if b => 1,
1..4 if !b => 2,
_ => 3,
}
};
assert_eq!(excl_range(3, false), 2);
assert_eq!(excl_range(3, true), 0);
assert_eq!(excl_range(5, false), 3);
assert_eq!(excl_range(5, true), 1);

let incl_range_vs_const = |x, b| {
match x {
0..=5 if b => 0,
7 => 1,
3 => 2,
_ => 3,
}
};
assert_eq!(incl_range_vs_const(5, false), 3);
assert_eq!(incl_range_vs_const(5, true), 0);
assert_eq!(incl_range_vs_const(3, false), 2);
assert_eq!(incl_range_vs_const(3, true), 0);
assert_eq!(incl_range_vs_const(7, false), 1);
assert_eq!(incl_range_vs_const(7, true), 1);

let excl_range_vs_const = |x, b| {
match x {
0..5 if b => 0,
7 => 1,
3 => 2,
_ => 3,
}
};
assert_eq!(excl_range_vs_const(5, false), 3);
assert_eq!(excl_range_vs_const(5, true), 3);
assert_eq!(excl_range_vs_const(3, false), 2);
assert_eq!(excl_range_vs_const(3, true), 0);
assert_eq!(excl_range_vs_const(7, false), 1);
assert_eq!(excl_range_vs_const(7, true), 1);

let const_vs_incl_range = |x, b| {
match x {
3 if b => 0,
5..=7 => 2,
1..=4 => 1,
_ => 3,
}
};
assert_eq!(const_vs_incl_range(3, false), 1);
assert_eq!(const_vs_incl_range(3, true), 0);

let const_vs_excl_range = |x, b| {
match x {
3 if b => 0,
5..7 => 2,
1..4 => 1,
_ => 3,
}
};
assert_eq!(const_vs_excl_range(3, false), 1);
assert_eq!(const_vs_excl_range(3, true), 0);
}