Skip to content

Commit d55c3fa

Browse files
committed
Fix codegen bug for mixed range and literal patterns
If there are both range and literal patterns in the same column of a match expression, rustc may generate incorrect code. This patch fixes it. Closes #18060
1 parent 2b01ef3 commit d55c3fa

File tree

3 files changed

+272
-89
lines changed

3 files changed

+272
-89
lines changed

src/librustc/middle/check_match.rs

+73-27
Original file line numberDiff line numberDiff line change
@@ -102,23 +102,39 @@ pub struct MatchCheckCtxt<'a, 'tcx: 'a> {
102102
pub param_env: ParameterEnvironment<'a, 'tcx>,
103103
}
104104

105-
#[derive(Clone, PartialEq)]
105+
#[derive(Clone)]
106106
pub enum Constructor {
107107
/// The constructor of all patterns that don't vary by constructor,
108108
/// e.g. struct patterns and fixed-length arrays.
109109
Single,
110110
/// Enum variants.
111111
Variant(ast::DefId),
112112
/// Literal values.
113-
ConstantValue(const_val),
113+
ConstantValue(NodeId, const_val),
114114
/// Ranges of literal values (2..5).
115-
ConstantRange(const_val, const_val),
115+
ConstantRange(NodeId, const_val, NodeId, const_val),
116116
/// Array patterns of length n.
117117
Slice(uint),
118118
/// Array patterns with a subslice.
119119
SliceWithSubslice(uint, uint)
120120
}
121121

122+
impl PartialEq for Constructor {
123+
fn eq(&self, other: &Constructor) -> bool {
124+
match (self, other) {
125+
(&Single, &Single) => true,
126+
(&Variant(ref id), &Variant(ref o_id)) => id == o_id,
127+
(&ConstantValue(_, ref val), &ConstantValue(_, ref o_val)) => val == o_val,
128+
(&ConstantRange(_, ref from, _, ref to),
129+
&ConstantRange(_, ref o_from, _, ref o_to)) => from == o_from && to == o_to,
130+
(&Slice(ref val), &Slice(ref o_val)) => val == o_val,
131+
(&SliceWithSubslice(ref from, ref to),
132+
&SliceWithSubslice(ref o_from, ref o_to)) => from == o_from && to == o_to,
133+
_ => false
134+
}
135+
}
136+
}
137+
122138
#[derive(Clone, PartialEq)]
123139
pub enum Usefulness {
124140
Useful,
@@ -505,7 +521,7 @@ fn construct_witness(cx: &MatchCheckCtxt, ctor: &Constructor,
505521

506522
_ => {
507523
match *ctor {
508-
ConstantValue(ref v) => ast::PatLit(const_val_to_expr(v)),
524+
ConstantValue(_, ref v) => ast::PatLit(const_val_to_expr(v)),
509525
_ => ast::PatWild(ast::PatWildSingle),
510526
}
511527
}
@@ -536,7 +552,9 @@ fn all_constructors(cx: &MatchCheckCtxt, left_ty: Ty,
536552
max_slice_length: uint) -> Vec<Constructor> {
537553
match left_ty.sty {
538554
ty::ty_bool =>
539-
[true, false].iter().map(|b| ConstantValue(const_bool(*b))).collect(),
555+
[true, false].iter()
556+
.map(|b| ConstantValue(DUMMY_NODE_ID, const_bool(*b)))
557+
.collect(),
540558

541559
ty::ty_rptr(_, ty::mt { ty, .. }) => match ty.sty {
542560
ty::ty_vec(_, None) =>
@@ -655,9 +673,9 @@ fn is_useful_specialized(cx: &MatchCheckCtxt, &Matrix(ref m): &Matrix,
655673
witness: WitnessPreference) -> Usefulness {
656674
let arity = constructor_arity(cx, &ctor, lty);
657675
let matrix = Matrix(m.iter().filter_map(|r| {
658-
specialize(cx, &r[], &ctor, 0u, arity)
676+
specialize(cx, &r[], &ctor, 0u, arity, range_covered_by_constructor)
659677
}).collect());
660-
match specialize(cx, v, &ctor, 0u, arity) {
678+
match specialize(cx, v, &ctor, 0u, arity, range_covered_by_constructor) {
661679
Some(v) => is_useful(cx, &matrix, &v[], witness),
662680
None => NotUseful
663681
}
@@ -702,9 +720,11 @@ fn pat_constructors(cx: &MatchCheckCtxt, p: &Pat,
702720
_ => vec!(Single)
703721
},
704722
ast::PatLit(ref expr) =>
705-
vec!(ConstantValue(eval_const_expr(cx.tcx, &**expr))),
723+
vec!(ConstantValue(expr.id, eval_const_expr(cx.tcx, &**expr))),
706724
ast::PatRange(ref lo, ref hi) =>
707-
vec!(ConstantRange(eval_const_expr(cx.tcx, &**lo), eval_const_expr(cx.tcx, &**hi))),
725+
vec!(ConstantRange(
726+
lo.id, eval_const_expr(cx.tcx, &**lo),
727+
hi.id, eval_const_expr(cx.tcx, &**hi))),
708728
ast::PatVec(ref before, ref slice, ref after) =>
709729
match left_ty.sty {
710730
ty::ty_vec(_, Some(_)) => vec!(Single),
@@ -737,7 +757,7 @@ pub fn constructor_arity(cx: &MatchCheckCtxt, ctor: &Constructor, ty: Ty) -> uin
737757
ty::ty_rptr(_, ty::mt { ty, .. }) => match ty.sty {
738758
ty::ty_vec(_, None) => match *ctor {
739759
Slice(length) => length,
740-
ConstantValue(_) => 0u,
760+
ConstantValue(..) => 0u,
741761
_ => unreachable!()
742762
},
743763
ty::ty_str => 0u,
@@ -756,17 +776,29 @@ pub fn constructor_arity(cx: &MatchCheckCtxt, ctor: &Constructor, ty: Ty) -> uin
756776
}
757777

758778
fn range_covered_by_constructor(ctor: &Constructor,
759-
from: &const_val, to: &const_val) -> Option<bool> {
760-
let (c_from, c_to) = match *ctor {
761-
ConstantValue(ref value) => (value, value),
762-
ConstantRange(ref from, ref to) => (from, to),
763-
Single => return Some(true),
764-
_ => unreachable!()
779+
pat_ctor: &Constructor) -> Option<bool> {
780+
let (r_from, r_to) = match (ctor, pat_ctor) {
781+
(&Single, _) => return Some(true),
782+
(&ConstantValue(_, ref val),
783+
&ConstantValue(_, ref pat_val)) =>
784+
return compare_const_vals(val, pat_val).map(|r| r == 0),
785+
786+
(&ConstantValue(_, ref val),
787+
&ConstantRange(_, ref p_from, _, ref p_to)) =>
788+
(compare_const_vals(val, p_from), compare_const_vals(val, p_to)),
789+
790+
(&ConstantRange(_, ref from, _, ref to),
791+
&ConstantValue(_, ref pat_val)) =>
792+
(compare_const_vals(from, pat_val), compare_const_vals(to, pat_val)),
793+
794+
(&ConstantRange(_, ref from, _, ref to),
795+
&ConstantRange(_, ref p_from, _, ref p_to)) =>
796+
(compare_const_vals(from, p_from), compare_const_vals(to, p_to)),
797+
798+
_ => unreachable!()
765799
};
766-
let cmp_from = compare_const_vals(c_from, from);
767-
let cmp_to = compare_const_vals(c_to, to);
768-
match (cmp_from, cmp_to) {
769-
(Some(val1), Some(val2)) => Some(val1 >= 0 && val2 <= 0),
800+
match (r_from, r_to) {
801+
(Some(r1), Some(r2)) => Some(0 <= r1 && r2 <= 0),
770802
_ => None
771803
}
772804
}
@@ -779,8 +811,21 @@ fn range_covered_by_constructor(ctor: &Constructor,
779811
/// different patterns.
780812
/// Structure patterns with a partial wild pattern (Foo { a: 42, .. }) have their missing
781813
/// fields filled with wild patterns.
782-
pub fn specialize<'a>(cx: &MatchCheckCtxt, r: &[&'a Pat],
783-
constructor: &Constructor, col: uint, arity: uint) -> Option<Vec<&'a Pat>> {
814+
///
815+
/// Both `typeck` and `codegen` use the specialization but in slightly different ways.
816+
/// For typeck's exhaustion and reachability checking, whether a range pattern covers another
817+
/// range or literal pattern is apparently significant. For codegen, however, the semantic of
818+
/// the equality of range and literal patterns depends on the generated LLVM instruction.
819+
/// The `op_range` is to deal with such distinctions. For the concrete examples of some subtle
820+
/// match expressions, check out `run-pass/issue-18060.rs`.
821+
pub fn specialize<'a, F>(cx: &MatchCheckCtxt,
822+
r: &[&'a Pat],
823+
constructor: &Constructor,
824+
col: uint,
825+
arity: uint,
826+
op_range: F) -> Option<Vec<&'a Pat>> where
827+
F: Fn(&Constructor, &Constructor) -> Option<bool>
828+
{
784829
let &Pat {
785830
id: pat_id, ref node, span: pat_span
786831
} = raw_pat(r[col]);
@@ -863,8 +908,8 @@ pub fn specialize<'a>(cx: &MatchCheckCtxt, r: &[&'a Pat],
863908
Some(vec![&**inner]),
864909

865910
ast::PatLit(ref expr) => {
866-
let expr_value = eval_const_expr(cx.tcx, &**expr);
867-
match range_covered_by_constructor(constructor, &expr_value, &expr_value) {
911+
let pat_ctor = ConstantValue(expr.id, eval_const_expr(cx.tcx, &**expr));
912+
match op_range(constructor, &pat_ctor) {
868913
Some(true) => Some(vec![]),
869914
Some(false) => None,
870915
None => {
@@ -875,9 +920,10 @@ pub fn specialize<'a>(cx: &MatchCheckCtxt, r: &[&'a Pat],
875920
}
876921

877922
ast::PatRange(ref from, ref to) => {
878-
let from_value = eval_const_expr(cx.tcx, &**from);
879-
let to_value = eval_const_expr(cx.tcx, &**to);
880-
match range_covered_by_constructor(constructor, &from_value, &to_value) {
923+
let pat_ctor = ConstantRange(
924+
from.id, eval_const_expr(cx.tcx, &**from),
925+
to.id, eval_const_expr(cx.tcx, &**to));
926+
match op_range(constructor, &pat_ctor) {
881927
Some(true) => Some(vec![]),
882928
Some(false) => None,
883929
None => {

0 commit comments

Comments
 (0)