Skip to content

Commit 191ff2d

Browse files
committed
Auto merge of #30651 - nagisa:mir-fix-equality-checks, r=eddyb
This is not a fix to checks themselves per se (though we still use `Eq` MIR test instead of calling `PartialEq::eq`), but rather how we handle items we encounter in pattern position. Previously we would just call `PartialEq` with the constant and the matchee, but now we essentially inline the constant instead. E.g. these two snippets are functionally equivalent at MIR level: ``` match val { Some(42) => true, _ => false } ``` and ``` const SECRET: Option<u8> = Some(42); match val { SECRET => true, _ => false } ``` This approach also allows for more optimizations of matches. I.e. It can now exploit `SwitchInt` to switch on number inside a `Some` regardless of whether the value being an item or not. This is based on @tsion’s already approved PR so I could reuse the file for more tests. r? @eddyb cc @nikomatsakis @tsion
2 parents 99e59de + add7410 commit 191ff2d

File tree

7 files changed

+101
-159
lines changed

7 files changed

+101
-159
lines changed

src/librustc_mir/build/matches/mod.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -259,7 +259,7 @@ enum TestKind<'tcx> {
259259

260260
// test for equality
261261
Eq {
262-
value: Literal<'tcx>,
262+
value: ConstVal,
263263
ty: Ty<'tcx>,
264264
},
265265

src/librustc_mir/build/matches/test.rs

+11-44
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ impl<'a,'tcx> Builder<'a,'tcx> {
3737
}
3838
}
3939

40-
PatternKind::Constant { value: Literal::Value { .. } }
40+
PatternKind::Constant { .. }
4141
if is_switch_ty(match_pair.pattern.ty) => {
4242
// for integers, we use a SwitchInt match, which allows
4343
// us to handle more cases
@@ -55,12 +55,11 @@ impl<'a,'tcx> Builder<'a,'tcx> {
5555
}
5656

5757
PatternKind::Constant { ref value } => {
58-
// for other types, we use an equality comparison
5958
Test {
6059
span: match_pair.pattern.span,
6160
kind: TestKind::Eq {
6261
value: value.clone(),
63-
ty: match_pair.pattern.ty.clone(),
62+
ty: match_pair.pattern.ty.clone()
6463
}
6564
}
6665
}
@@ -113,7 +112,7 @@ impl<'a,'tcx> Builder<'a,'tcx> {
113112
};
114113

115114
match *match_pair.pattern.kind {
116-
PatternKind::Constant { value: Literal::Value { ref value } } => {
115+
PatternKind::Constant { ref value } => {
117116
// if the lvalues match, the type should match
118117
assert_eq!(match_pair.pattern.ty, switch_ty);
119118

@@ -126,7 +125,6 @@ impl<'a,'tcx> Builder<'a,'tcx> {
126125
}
127126

128127
PatternKind::Range { .. } |
129-
PatternKind::Constant { .. } |
130128
PatternKind::Variant { .. } |
131129
PatternKind::Slice { .. } |
132130
PatternKind::Array { .. } |
@@ -177,11 +175,13 @@ impl<'a,'tcx> Builder<'a,'tcx> {
177175
}
178176

179177
TestKind::Eq { ref value, ty } => {
180-
// call PartialEq::eq(discrim, constant)
181-
let constant = self.literal_operand(test.span, ty.clone(), value.clone());
182-
let item_ref = self.hir.partial_eq(ty);
183-
self.call_comparison_fn(block, test.span, item_ref,
184-
Operand::Consume(lvalue.clone()), constant)
178+
let expect = self.literal_operand(test.span, ty.clone(), Literal::Value {
179+
value: value.clone()
180+
});
181+
let val = Operand::Consume(lvalue.clone());
182+
let fail = self.cfg.start_new_block();
183+
let block = self.compare(block, fail, test.span, BinOp::Eq, expect, val.clone());
184+
vec![block, fail]
185185
}
186186

187187
TestKind::Range { ref lo, ref hi, ty } => {
@@ -251,39 +251,6 @@ impl<'a,'tcx> Builder<'a,'tcx> {
251251
target_block
252252
}
253253

254-
fn call_comparison_fn(&mut self,
255-
block: BasicBlock,
256-
span: Span,
257-
item_ref: ItemRef<'tcx>,
258-
lvalue1: Operand<'tcx>,
259-
lvalue2: Operand<'tcx>)
260-
-> Vec<BasicBlock> {
261-
let target_blocks = vec![self.cfg.start_new_block(), self.cfg.start_new_block()];
262-
263-
let bool_ty = self.hir.bool_ty();
264-
let eq_result = self.temp(bool_ty);
265-
let func = self.item_ref_operand(span, item_ref);
266-
let call_blocks = (self.cfg.start_new_block(), self.diverge_cleanup());
267-
self.cfg.terminate(block,
268-
Terminator::Call {
269-
data: CallData {
270-
destination: eq_result.clone(),
271-
func: func,
272-
args: vec![lvalue1, lvalue2],
273-
},
274-
targets: call_blocks,
275-
});
276-
277-
// check the result
278-
self.cfg.terminate(call_blocks.0,
279-
Terminator::If {
280-
cond: Operand::Consume(eq_result),
281-
targets: (target_blocks[0], target_blocks[1]),
282-
});
283-
284-
target_blocks
285-
}
286-
287254
/// Given that we are performing `test` against `test_lvalue`,
288255
/// this job sorts out what the status of `candidate` will be
289256
/// after the test. The `resulting_candidates` vector stores, for
@@ -368,7 +335,7 @@ impl<'a,'tcx> Builder<'a,'tcx> {
368335
// things out here, in some cases.
369336
TestKind::SwitchInt { switch_ty: _, options: _, ref indices } => {
370337
match *match_pair.pattern.kind {
371-
PatternKind::Constant { value: Literal::Value { ref value } }
338+
PatternKind::Constant { ref value }
372339
if is_switch_ty(match_pair.pattern.ty) => {
373340
let index = indices[value];
374341
let new_candidate = self.candidate_without_match_pair(match_pair_index,

src/librustc_mir/hair/cx/mod.rs

-37
Original file line numberDiff line numberDiff line change
@@ -19,12 +19,9 @@ use hair::*;
1919
use rustc::mir::repr::*;
2020

2121
use rustc::middle::const_eval::{self, ConstVal};
22-
use rustc::middle::def_id::DefId;
2322
use rustc::middle::infer::InferCtxt;
24-
use rustc::middle::subst::{Subst, Substs};
2523
use rustc::middle::ty::{self, Ty};
2624
use syntax::codemap::Span;
27-
use syntax::parse::token;
2825
use rustc_front::hir;
2926

3027
#[derive(Copy, Clone)]
@@ -83,11 +80,6 @@ impl<'a,'tcx:'a> Cx<'a, 'tcx> {
8380
.map(|v| Literal::Value { value: v })
8481
}
8582

86-
pub fn partial_eq(&mut self, ty: Ty<'tcx>) -> ItemRef<'tcx> {
87-
let eq_def_id = self.tcx.lang_items.eq_trait().unwrap();
88-
self.cmp_method_ref(eq_def_id, "eq", ty)
89-
}
90-
9183
pub fn num_variants(&mut self, adt_def: ty::AdtDef<'tcx>) -> usize {
9284
adt_def.variants.len()
9385
}
@@ -118,35 +110,6 @@ impl<'a,'tcx:'a> Cx<'a, 'tcx> {
118110
pub fn tcx(&self) -> &'a ty::ctxt<'tcx> {
119111
self.tcx
120112
}
121-
122-
fn cmp_method_ref(&mut self,
123-
trait_def_id: DefId,
124-
method_name: &str,
125-
arg_ty: Ty<'tcx>)
126-
-> ItemRef<'tcx> {
127-
let method_name = token::intern(method_name);
128-
let substs = Substs::new_trait(vec![arg_ty], vec![], arg_ty);
129-
for trait_item in self.tcx.trait_items(trait_def_id).iter() {
130-
match *trait_item {
131-
ty::ImplOrTraitItem::MethodTraitItem(ref method) => {
132-
if method.name == method_name {
133-
let method_ty = self.tcx.lookup_item_type(method.def_id);
134-
let method_ty = method_ty.ty.subst(self.tcx, &substs);
135-
return ItemRef {
136-
ty: method_ty,
137-
kind: ItemKind::Method,
138-
def_id: method.def_id,
139-
substs: self.tcx.mk_substs(substs),
140-
};
141-
}
142-
}
143-
ty::ImplOrTraitItem::ConstTraitItem(..) |
144-
ty::ImplOrTraitItem::TypeTraitItem(..) => {}
145-
}
146-
}
147-
148-
self.tcx.sess.bug(&format!("found no method `{}` in `{:?}`", method_name, trait_def_id));
149-
}
150113
}
151114

152115
mod block;

src/librustc_mir/hair/cx/pattern.rs

+32-46
Original file line numberDiff line numberDiff line change
@@ -14,11 +14,11 @@ use rustc_data_structures::fnv::FnvHashMap;
1414
use rustc::middle::const_eval;
1515
use rustc::middle::def;
1616
use rustc::middle::pat_util::{pat_is_resolved_const, pat_is_binding};
17-
use rustc::middle::subst::Substs;
1817
use rustc::middle::ty::{self, Ty};
1918
use rustc::mir::repr::*;
2019
use rustc_front::hir;
2120
use syntax::ast;
21+
use syntax::codemap::Span;
2222
use syntax::ptr::P;
2323

2424
/// When there are multiple patterns in a single arm, each one has its
@@ -40,15 +40,15 @@ struct PatCx<'patcx, 'cx: 'patcx, 'tcx: 'cx> {
4040
}
4141

4242
impl<'cx, 'tcx> Cx<'cx, 'tcx> {
43-
pub fn irrefutable_pat(&mut self, pat: &'tcx hir::Pat) -> Pattern<'tcx> {
44-
PatCx::new(self, None).to_pat(pat)
43+
pub fn irrefutable_pat(&mut self, pat: &hir::Pat) -> Pattern<'tcx> {
44+
PatCx::new(self, None).to_pattern(pat)
4545
}
4646

4747
pub fn refutable_pat(&mut self,
4848
binding_map: Option<&FnvHashMap<ast::Name, ast::NodeId>>,
49-
pat: &'tcx hir::Pat)
49+
pat: &hir::Pat)
5050
-> Pattern<'tcx> {
51-
PatCx::new(self, binding_map).to_pat(pat)
51+
PatCx::new(self, binding_map).to_pattern(pat)
5252
}
5353
}
5454

@@ -62,13 +62,12 @@ impl<'patcx, 'cx, 'tcx> PatCx<'patcx, 'cx, 'tcx> {
6262
}
6363
}
6464

65-
fn to_pat(&mut self, pat: &'tcx hir::Pat) -> Pattern<'tcx> {
65+
fn to_pattern(&mut self, pat: &hir::Pat) -> Pattern<'tcx> {
6666
let kind = match pat.node {
6767
hir::PatWild => PatternKind::Wild,
6868

6969
hir::PatLit(ref value) => {
7070
let value = const_eval::eval_const_expr(self.cx.tcx, value);
71-
let value = Literal::Value { value: value };
7271
PatternKind::Constant { value: value }
7372
}
7473

@@ -88,22 +87,9 @@ impl<'patcx, 'cx, 'tcx> PatCx<'patcx, 'cx, 'tcx> {
8887
def::DefConst(def_id) | def::DefAssociatedConst(def_id) =>
8988
match const_eval::lookup_const_by_id(self.cx.tcx, def_id, Some(pat.id)) {
9089
Some(const_expr) => {
91-
let opt_value =
92-
const_eval::eval_const_expr_partial(
93-
self.cx.tcx, const_expr,
94-
const_eval::EvalHint::ExprTypeChecked,
95-
None);
96-
let literal = if let Ok(value) = opt_value {
97-
Literal::Value { value: value }
98-
} else {
99-
let substs = self.cx.tcx.mk_substs(Substs::empty());
100-
Literal::Item {
101-
def_id: def_id,
102-
kind: ItemKind::Constant,
103-
substs: substs
104-
}
105-
};
106-
PatternKind::Constant { value: literal }
90+
let pat = const_eval::const_expr_to_pat(self.cx.tcx, const_expr,
91+
pat.span);
92+
return self.to_pattern(&*pat);
10793
}
10894
None => {
10995
self.cx.tcx.sess.span_bug(
@@ -120,7 +106,7 @@ impl<'patcx, 'cx, 'tcx> PatCx<'patcx, 'cx, 'tcx> {
120106

121107
hir::PatRegion(ref subpattern, _) |
122108
hir::PatBox(ref subpattern) => {
123-
PatternKind::Deref { subpattern: self.to_pat(subpattern) }
109+
PatternKind::Deref { subpattern: self.to_pattern(subpattern) }
124110
}
125111

126112
hir::PatVec(ref prefix, ref slice, ref suffix) => {
@@ -131,14 +117,14 @@ impl<'patcx, 'cx, 'tcx> PatCx<'patcx, 'cx, 'tcx> {
131117
subpattern: Pattern {
132118
ty: mt.ty,
133119
span: pat.span,
134-
kind: Box::new(self.slice_or_array_pattern(pat, mt.ty, prefix,
120+
kind: Box::new(self.slice_or_array_pattern(pat.span, mt.ty, prefix,
135121
slice, suffix)),
136122
},
137123
},
138124

139125
ty::TySlice(..) |
140126
ty::TyArray(..) =>
141-
self.slice_or_array_pattern(pat, ty, prefix, slice, suffix),
127+
self.slice_or_array_pattern(pat.span, ty, prefix, slice, suffix),
142128

143129
ref sty =>
144130
self.cx.tcx.sess.span_bug(
@@ -153,7 +139,7 @@ impl<'patcx, 'cx, 'tcx> PatCx<'patcx, 'cx, 'tcx> {
153139
.enumerate()
154140
.map(|(i, subpattern)| FieldPattern {
155141
field: Field::new(i),
156-
pattern: self.to_pat(subpattern),
142+
pattern: self.to_pattern(subpattern),
157143
})
158144
.collect();
159145

@@ -188,7 +174,7 @@ impl<'patcx, 'cx, 'tcx> PatCx<'patcx, 'cx, 'tcx> {
188174
name: ident.node.name,
189175
var: id,
190176
ty: var_ty,
191-
subpattern: self.to_opt_pat(sub),
177+
subpattern: self.to_opt_pattern(sub),
192178
}
193179
}
194180

@@ -203,7 +189,7 @@ impl<'patcx, 'cx, 'tcx> PatCx<'patcx, 'cx, 'tcx> {
203189
.enumerate()
204190
.map(|(i, field)| FieldPattern {
205191
field: Field::new(i),
206-
pattern: self.to_pat(field),
192+
pattern: self.to_pattern(field),
207193
})
208194
.collect();
209195
self.variant_or_leaf(pat, subpatterns)
@@ -234,7 +220,7 @@ impl<'patcx, 'cx, 'tcx> PatCx<'patcx, 'cx, 'tcx> {
234220
});
235221
FieldPattern {
236222
field: Field::new(index),
237-
pattern: self.to_pat(&field.node.pat),
223+
pattern: self.to_pattern(&field.node.pat),
238224
}
239225
})
240226
.collect();
@@ -256,49 +242,49 @@ impl<'patcx, 'cx, 'tcx> PatCx<'patcx, 'cx, 'tcx> {
256242
}
257243
}
258244

259-
fn to_pats(&mut self, pats: &'tcx [P<hir::Pat>]) -> Vec<Pattern<'tcx>> {
260-
pats.iter().map(|p| self.to_pat(p)).collect()
245+
fn to_patterns(&mut self, pats: &[P<hir::Pat>]) -> Vec<Pattern<'tcx>> {
246+
pats.iter().map(|p| self.to_pattern(p)).collect()
261247
}
262248

263-
fn to_opt_pat(&mut self, pat: &'tcx Option<P<hir::Pat>>) -> Option<Pattern<'tcx>> {
264-
pat.as_ref().map(|p| self.to_pat(p))
249+
fn to_opt_pattern(&mut self, pat: &Option<P<hir::Pat>>) -> Option<Pattern<'tcx>> {
250+
pat.as_ref().map(|p| self.to_pattern(p))
265251
}
266252

267253
fn slice_or_array_pattern(&mut self,
268-
pat: &'tcx hir::Pat,
254+
span: Span,
269255
ty: Ty<'tcx>,
270-
prefix: &'tcx [P<hir::Pat>],
271-
slice: &'tcx Option<P<hir::Pat>>,
272-
suffix: &'tcx [P<hir::Pat>])
256+
prefix: &[P<hir::Pat>],
257+
slice: &Option<P<hir::Pat>>,
258+
suffix: &[P<hir::Pat>])
273259
-> PatternKind<'tcx> {
274260
match ty.sty {
275261
ty::TySlice(..) => {
276262
// matching a slice or fixed-length array
277263
PatternKind::Slice {
278-
prefix: self.to_pats(prefix),
279-
slice: self.to_opt_pat(slice),
280-
suffix: self.to_pats(suffix),
264+
prefix: self.to_patterns(prefix),
265+
slice: self.to_opt_pattern(slice),
266+
suffix: self.to_patterns(suffix),
281267
}
282268
}
283269

284270
ty::TyArray(_, len) => {
285271
// fixed-length array
286272
assert!(len >= prefix.len() + suffix.len());
287273
PatternKind::Array {
288-
prefix: self.to_pats(prefix),
289-
slice: self.to_opt_pat(slice),
290-
suffix: self.to_pats(suffix),
274+
prefix: self.to_patterns(prefix),
275+
slice: self.to_opt_pattern(slice),
276+
suffix: self.to_patterns(suffix),
291277
}
292278
}
293279

294280
_ => {
295-
self.cx.tcx.sess.span_bug(pat.span, "unexpanded macro or bad constant etc");
281+
self.cx.tcx.sess.span_bug(span, "unexpanded macro or bad constant etc");
296282
}
297283
}
298284
}
299285

300286
fn variant_or_leaf(&mut self,
301-
pat: &'tcx hir::Pat,
287+
pat: &hir::Pat,
302288
subpatterns: Vec<FieldPattern<'tcx>>)
303289
-> PatternKind<'tcx> {
304290
let def = self.cx.tcx.def_map.borrow().get(&pat.id).unwrap().full_def();

src/librustc_mir/hair/mod.rs

+2-1
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
//! structures.
1616
1717
use rustc::mir::repr::{BinOp, BorrowKind, Field, Literal, Mutability, UnOp, ItemKind};
18+
use rustc::middle::const_eval::ConstVal;
1819
use rustc::middle::def_id::DefId;
1920
use rustc::middle::region::CodeExtent;
2021
use rustc::middle::subst::Substs;
@@ -305,7 +306,7 @@ pub enum PatternKind<'tcx> {
305306
}, // box P, &P, &mut P, etc
306307

307308
Constant {
308-
value: Literal<'tcx>,
309+
value: ConstVal,
309310
},
310311

311312
Range {

0 commit comments

Comments
 (0)