aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorJosh Mcguigan <[email protected]>2020-04-05 02:02:27 +0100
committerJosh Mcguigan <[email protected]>2020-04-07 13:12:08 +0100
commitb87b7a088f34ff794fc19e57ee2ae1cfe81a12df (patch)
treedcfd92910610f457a6ae598068c847bcd49867cc
parent8c378af72117e92bc894fd4a79e978ef0d1c0cc7 (diff)
remove panics
-rw-r--r--crates/ra_hir_ty/src/_match.rs273
-rw-r--r--crates/ra_hir_ty/src/expr.rs7
2 files changed, 214 insertions, 66 deletions
diff --git a/crates/ra_hir_ty/src/_match.rs b/crates/ra_hir_ty/src/_match.rs
index ac66dd415..de291c1f6 100644
--- a/crates/ra_hir_ty/src/_match.rs
+++ b/crates/ra_hir_ty/src/_match.rs
@@ -42,6 +42,10 @@ impl From<PatId> for PatIdOrWild {
42 } 42 }
43} 43}
44 44
45#[derive(Debug, Clone, Copy, PartialEq)]
46pub struct MatchCheckNotImplemented;
47pub type MatchCheckResult<T> = Result<T, MatchCheckNotImplemented>;
48
45type PatStackInner = SmallVec<[PatIdOrWild; 2]>; 49type PatStackInner = SmallVec<[PatIdOrWild; 2]>;
46#[derive(Debug)] 50#[derive(Debug)]
47pub(crate) struct PatStack(PatStackInner); 51pub(crate) struct PatStack(PatStackInner);
@@ -104,42 +108,49 @@ impl PatStack {
104 &self, 108 &self,
105 cx: &MatchCheckCtx, 109 cx: &MatchCheckCtx,
106 constructor: &Constructor, 110 constructor: &Constructor,
107 ) -> Option<PatStack> { 111 ) -> MatchCheckResult<Option<PatStack>> {
108 match (self.head().as_pat(cx), constructor) { 112 let result = match (self.head().as_pat(cx), constructor) {
109 (Pat::Tuple(ref pat_ids), Constructor::Tuple { arity }) => { 113 (Pat::Tuple(ref pat_ids), Constructor::Tuple { arity }) => {
110 if pat_ids.len() != *arity { 114 if pat_ids.len() != *arity {
111 return None; 115 None
116 } else {
117 Some(self.replace_head_with(pat_ids))
112 } 118 }
113
114 Some(self.replace_head_with(pat_ids))
115 } 119 }
116 (Pat::Lit(_), Constructor::Bool(_)) => { 120 (Pat::Lit(_), Constructor::Bool(_)) => {
117 // for now we only support bool literals 121 // for now we only support bool literals
118 Some(self.to_tail()) 122 Some(self.to_tail())
119 } 123 }
120 (Pat::Wild, constructor) => Some(self.expand_wildcard(cx, constructor)), 124 (Pat::Wild, constructor) => Some(self.expand_wildcard(cx, constructor)?),
121 (Pat::Path(_), Constructor::Enum(constructor)) => { 125 (Pat::Path(_), Constructor::Enum(constructor)) => {
126 // enums with no associated data become `Pat::Path`
122 let pat_id = self.head().as_id().expect("we know this isn't a wild"); 127 let pat_id = self.head().as_id().expect("we know this isn't a wild");
123 if !enum_variant_matches(cx, pat_id, *constructor) { 128 if !enum_variant_matches(cx, pat_id, *constructor) {
124 return None; 129 None
130 } else {
131 Some(self.to_tail())
125 } 132 }
126 // enums with no associated data become `Pat::Path`
127 Some(self.to_tail())
128 } 133 }
129 (Pat::TupleStruct { args: ref pat_ids, .. }, Constructor::Enum(constructor)) => { 134 (Pat::TupleStruct { args: ref pat_ids, .. }, Constructor::Enum(constructor)) => {
130 let pat_id = self.head().as_id().expect("we know this isn't a wild"); 135 let pat_id = self.head().as_id().expect("we know this isn't a wild");
131 if !enum_variant_matches(cx, pat_id, *constructor) { 136 if !enum_variant_matches(cx, pat_id, *constructor) {
132 return None; 137 None
138 } else {
139 Some(self.replace_head_with(pat_ids))
133 } 140 }
134
135 Some(self.replace_head_with(pat_ids))
136 } 141 }
137 (Pat::Or(_), _) => unreachable!("we desugar or patterns so this should never happen"), 142 (Pat::Or(_), _) => unreachable!("we desugar or patterns so this should never happen"),
138 (a, b) => unimplemented!("{:?}, {:?}", a, b), 143 (_, _) => return Err(MatchCheckNotImplemented),
139 } 144 };
145
146 Ok(result)
140 } 147 }
141 148
142 fn expand_wildcard(&self, cx: &MatchCheckCtx, constructor: &Constructor) -> PatStack { 149 fn expand_wildcard(
150 &self,
151 cx: &MatchCheckCtx,
152 constructor: &Constructor,
153 ) -> MatchCheckResult<PatStack> {
143 assert_eq!( 154 assert_eq!(
144 Pat::Wild, 155 Pat::Wild,
145 self.head().as_pat(cx), 156 self.head().as_pat(cx),
@@ -154,7 +165,7 @@ impl PatStack {
154 match cx.db.enum_data(e.parent).variants[e.local_id].variant_data.as_ref() { 165 match cx.db.enum_data(e.parent).variants[e.local_id].variant_data.as_ref() {
155 VariantData::Tuple(struct_field_data) => struct_field_data.len(), 166 VariantData::Tuple(struct_field_data) => struct_field_data.len(),
156 VariantData::Unit => 0, 167 VariantData::Unit => 0,
157 x => unimplemented!("{:?}", x), 168 _ => return Err(MatchCheckNotImplemented),
158 } 169 }
159 } 170 }
160 }; 171 };
@@ -167,7 +178,7 @@ impl PatStack {
167 patterns.push(*pat); 178 patterns.push(*pat);
168 } 179 }
169 180
170 PatStack::from_vec(patterns) 181 Ok(PatStack::from_vec(patterns))
171 } 182 }
172} 183}
173 184
@@ -204,8 +215,19 @@ impl Matrix {
204 } 215 }
205 216
206 // Computes `S(constructor, self)`. 217 // Computes `S(constructor, self)`.
207 fn specialize_constructor(&self, cx: &MatchCheckCtx, constructor: &Constructor) -> Self { 218 fn specialize_constructor(
208 Self::collect(cx, self.0.iter().filter_map(|r| r.specialize_constructor(cx, constructor))) 219 &self,
220 cx: &MatchCheckCtx,
221 constructor: &Constructor,
222 ) -> MatchCheckResult<Self> {
223 let mut new_matrix = Matrix::empty();
224 for pat in &self.0 {
225 if let Some(pat) = pat.specialize_constructor(cx, constructor)? {
226 new_matrix.push(cx, pat);
227 }
228 }
229
230 Ok(new_matrix)
209 } 231 }
210 232
211 fn collect<T: IntoIterator<Item = PatStack>>(cx: &MatchCheckCtx, iter: T) -> Self { 233 fn collect<T: IntoIterator<Item = PatStack>>(cx: &MatchCheckCtx, iter: T) -> Self {
@@ -239,37 +261,56 @@ pub struct MatchCheckCtx<'a> {
239// don't think we can make that assumption here. How should that be handled? 261// don't think we can make that assumption here. How should that be handled?
240// 262//
241// Perhaps check that validity before passing the patterns into this method? 263// Perhaps check that validity before passing the patterns into this method?
242pub(crate) fn is_useful(cx: &MatchCheckCtx, matrix: &Matrix, v: &PatStack) -> Usefulness { 264pub(crate) fn is_useful(
243 dbg!(matrix); 265 cx: &MatchCheckCtx,
244 dbg!(v); 266 matrix: &Matrix,
267 v: &PatStack,
268) -> MatchCheckResult<Usefulness> {
245 if v.is_empty() { 269 if v.is_empty() {
246 if matrix.is_empty() { 270 let result = if matrix.is_empty() { Usefulness::Useful } else { Usefulness::NotUseful };
247 return Usefulness::Useful; 271
248 } else { 272 return Ok(result);
249 return Usefulness::NotUseful;
250 }
251 } 273 }
252 274
253 if let Pat::Or(pat_ids) = v.head().as_pat(cx) { 275 if let Pat::Or(pat_ids) = v.head().as_pat(cx) {
276 let mut found_unimplemented = false;
254 let any_useful = pat_ids.iter().any(|&pat_id| { 277 let any_useful = pat_ids.iter().any(|&pat_id| {
255 let v = PatStack::from_pattern(pat_id); 278 let v = PatStack::from_pattern(pat_id);
256 279
257 is_useful(cx, matrix, &v) == Usefulness::Useful 280 match is_useful(cx, matrix, &v) {
281 Ok(Usefulness::Useful) => true,
282 Ok(Usefulness::NotUseful) => false,
283 _ => {
284 found_unimplemented = true;
285 false
286 }
287 }
258 }); 288 });
259 289
260 return if any_useful { Usefulness::Useful } else { Usefulness::NotUseful }; 290 return if any_useful {
291 Ok(Usefulness::Useful)
292 } else if found_unimplemented {
293 Err(MatchCheckNotImplemented)
294 } else {
295 Ok(Usefulness::NotUseful)
296 };
261 } 297 }
262 298
263 if let Some(constructor) = pat_constructor(cx, v.head()) { 299 if let Some(constructor) = pat_constructor(cx, v.head())? {
264 let matrix = matrix.specialize_constructor(&cx, &constructor); 300 let matrix = matrix.specialize_constructor(&cx, &constructor)?;
265 let v = v.specialize_constructor(&cx, &constructor).expect("todo handle this case"); 301 let v = v
302 .specialize_constructor(&cx, &constructor)?
303 .expect("we know this can't fail because we get the constructor from `v.head()` above");
266 304
267 is_useful(&cx, &matrix, &v) 305 is_useful(&cx, &matrix, &v)
268 } else { 306 } else {
269 dbg!("expanding wildcard");
270 // expanding wildcard 307 // expanding wildcard
271 let used_constructors: Vec<Constructor> = 308 let mut used_constructors: Vec<Constructor> = vec![];
272 matrix.heads().iter().filter_map(|&p| pat_constructor(cx, p)).collect(); 309 for pat in matrix.heads() {
310 if let Some(constructor) = pat_constructor(cx, pat)? {
311 used_constructors.push(constructor);
312 }
313 }
273 314
274 // We assume here that the first constructor is the "correct" type. Since we 315 // We assume here that the first constructor is the "correct" type. Since we
275 // only care about the "type" of the constructor (i.e. if it is a bool we 316 // only care about the "type" of the constructor (i.e. if it is a bool we
@@ -278,7 +319,6 @@ pub(crate) fn is_useful(cx: &MatchCheckCtx, matrix: &Matrix, v: &PatStack) -> Us
278 // this is to use the match expressions type. 319 // this is to use the match expressions type.
279 match &used_constructors.first() { 320 match &used_constructors.first() {
280 Some(constructor) if all_constructors_covered(&cx, constructor, &used_constructors) => { 321 Some(constructor) if all_constructors_covered(&cx, constructor, &used_constructors) => {
281 dbg!("all constructors are covered");
282 // If all constructors are covered, then we need to consider whether 322 // If all constructors are covered, then we need to consider whether
283 // any values are covered by this wildcard. 323 // any values are covered by this wildcard.
284 // 324 //
@@ -286,43 +326,48 @@ pub(crate) fn is_useful(cx: &MatchCheckCtx, matrix: &Matrix, v: &PatStack) -> Us
286 // constructors are covered (`Some`/`None`), so we need 326 // constructors are covered (`Some`/`None`), so we need
287 // to perform specialization to see that our wildcard will cover 327 // to perform specialization to see that our wildcard will cover
288 // the `Some(false)` case. 328 // the `Some(false)` case.
289 let constructor = 329 let mut constructor = None;
290 matrix.heads().iter().filter_map(|&pat| pat_constructor(cx, pat)).next(); 330 for pat in matrix.heads() {
331 if let Some(c) = pat_constructor(cx, pat)? {
332 constructor = Some(c);
333 break;
334 }
335 }
291 336
292 if let Some(constructor) = constructor { 337 if let Some(constructor) = constructor {
293 dbg!("found constructor {:?}, specializing..", &constructor);
294 if let Constructor::Enum(e) = constructor { 338 if let Constructor::Enum(e) = constructor {
295 // For enums we handle each variant as a distinct constructor, so 339 // For enums we handle each variant as a distinct constructor, so
296 // here we create a constructor for each variant and then check 340 // here we create a constructor for each variant and then check
297 // usefulness after specializing for that constructor. 341 // usefulness after specializing for that constructor.
298 let any_useful = cx 342 let mut found_unimplemented = false;
299 .db 343 for constructor in
300 .enum_data(e.parent) 344 cx.db.enum_data(e.parent).variants.iter().map(|(local_id, _)| {
301 .variants
302 .iter()
303 .map(|(local_id, _)| {
304 Constructor::Enum(EnumVariantId { parent: e.parent, local_id }) 345 Constructor::Enum(EnumVariantId { parent: e.parent, local_id })
305 }) 346 })
306 .any(|constructor| { 347 {
307 let matrix = matrix.specialize_constructor(&cx, &constructor); 348 let matrix = matrix.specialize_constructor(&cx, &constructor)?;
308 let v = v.expand_wildcard(&cx, &constructor); 349 let v = v.expand_wildcard(&cx, &constructor)?;
309 350
310 is_useful(&cx, &matrix, &v) == Usefulness::Useful 351 match is_useful(&cx, &matrix, &v) {
311 }); 352 Ok(Usefulness::Useful) => return Ok(Usefulness::Useful),
353 Ok(Usefulness::NotUseful) => continue,
354 _ => found_unimplemented = true,
355 };
356 }
312 357
313 if any_useful { 358 if found_unimplemented {
314 Usefulness::Useful 359 Err(MatchCheckNotImplemented)
315 } else { 360 } else {
316 Usefulness::NotUseful 361 Ok(Usefulness::NotUseful)
317 } 362 }
318 } else { 363 } else {
319 let matrix = matrix.specialize_constructor(&cx, &constructor); 364 let matrix = matrix.specialize_constructor(&cx, &constructor)?;
320 let v = v.expand_wildcard(&cx, &constructor); 365 let v = v.expand_wildcard(&cx, &constructor)?;
321 366
322 is_useful(&cx, &matrix, &v) 367 is_useful(&cx, &matrix, &v)
323 } 368 }
324 } else { 369 } else {
325 Usefulness::NotUseful 370 Ok(Usefulness::NotUseful)
326 } 371 }
327 } 372 }
328 _ => { 373 _ => {
@@ -345,30 +390,32 @@ enum Constructor {
345 Enum(EnumVariantId), 390 Enum(EnumVariantId),
346} 391}
347 392
348fn pat_constructor(cx: &MatchCheckCtx, pat: PatIdOrWild) -> Option<Constructor> { 393fn pat_constructor(cx: &MatchCheckCtx, pat: PatIdOrWild) -> MatchCheckResult<Option<Constructor>> {
349 match pat.as_pat(cx) { 394 let res = match pat.as_pat(cx) {
350 Pat::Wild => None, 395 Pat::Wild => None,
351 Pat::Tuple(pats) => Some(Constructor::Tuple { arity: pats.len() }), 396 Pat::Tuple(pats) => Some(Constructor::Tuple { arity: pats.len() }),
352 Pat::Lit(lit_expr) => { 397 Pat::Lit(lit_expr) => {
353 // for now we only support bool literals 398 // for now we only support bool literals
354 match cx.body.exprs[lit_expr] { 399 match cx.body.exprs[lit_expr] {
355 Expr::Literal(Literal::Bool(val)) => Some(Constructor::Bool(val)), 400 Expr::Literal(Literal::Bool(val)) => Some(Constructor::Bool(val)),
356 _ => unimplemented!(), 401 _ => return Err(MatchCheckNotImplemented),
357 } 402 }
358 } 403 }
359 Pat::TupleStruct { .. } | Pat::Path(_) => { 404 Pat::TupleStruct { .. } | Pat::Path(_) => {
360 let pat_id = pat.as_id().expect("we already know this pattern is not a wild"); 405 let pat_id = pat.as_id().expect("we already know this pattern is not a wild");
361 let variant_id = 406 let variant_id =
362 cx.infer.variant_resolution_for_pat(pat_id).unwrap_or_else(|| unimplemented!()); 407 cx.infer.variant_resolution_for_pat(pat_id).ok_or(MatchCheckNotImplemented)?;
363 match variant_id { 408 match variant_id {
364 VariantId::EnumVariantId(enum_variant_id) => { 409 VariantId::EnumVariantId(enum_variant_id) => {
365 Some(Constructor::Enum(enum_variant_id)) 410 Some(Constructor::Enum(enum_variant_id))
366 } 411 }
367 _ => unimplemented!(), 412 _ => return Err(MatchCheckNotImplemented),
368 } 413 }
369 } 414 }
370 x => unimplemented!("{:?} not yet implemented", x), 415 _ => return Err(MatchCheckNotImplemented),
371 } 416 };
417
418 Ok(res)
372} 419}
373 420
374fn all_constructors_covered( 421fn all_constructors_covered(
@@ -614,6 +661,34 @@ mod tests {
614 } 661 }
615 662
616 #[test] 663 #[test]
664 fn tuple_of_bools_binding_missing_arms() {
665 let content = r"
666 fn test_fn() {
667 match (false, true) {
668 (true, _x) => {},
669 }
670 }
671 ";
672
673 check_diagnostic_with_no_fix(content);
674 }
675
676 #[test]
677 fn tuple_of_bools_binding_no_diagnostic() {
678 let content = r"
679 fn test_fn() {
680 match (false, true) {
681 (true, _x) => {},
682 (false, true) => {},
683 (false, false) => {},
684 }
685 }
686 ";
687
688 check_no_diagnostic(content);
689 }
690
691 #[test]
617 fn tuple_of_tuple_and_bools_no_arms() { 692 fn tuple_of_tuple_and_bools_no_arms() {
618 let content = r" 693 let content = r"
619 fn test_fn() { 694 fn test_fn() {
@@ -941,4 +1016,74 @@ mod false_negatives {
941 // match the type of the match expression. 1016 // match the type of the match expression.
942 check_no_diagnostic(content); 1017 check_no_diagnostic(content);
943 } 1018 }
1019
1020 #[test]
1021 fn mismatched_types_with_different_arity() {
1022 let content = r"
1023 fn test_fn() {
1024 match (true, false) {
1025 (true, false, true) => (),
1026 (true) => (),
1027 }
1028 }
1029 ";
1030
1031 // This is a false negative.
1032 // We don't currently check that the match arms actually
1033 // match the type of the match expression. This test
1034 // checks to ensure we don't panic when the code we are
1035 // checking is malformed in such a way that the arity of the
1036 // constructors doesn't match.
1037 check_no_diagnostic(content);
1038 }
1039
1040 #[test]
1041 fn integers() {
1042 let content = r"
1043 fn test_fn() {
1044 match 5 {
1045 10 => (),
1046 11..20 => (),
1047 }
1048 }
1049 ";
1050
1051 // This is a false negative.
1052 // We don't currently check integer exhaustiveness.
1053 check_no_diagnostic(content);
1054 }
1055
1056 #[test]
1057 fn enum_record() {
1058 let content = r"
1059 enum Either {
1060 A { foo: u32 },
1061 B,
1062 }
1063 fn test_fn() {
1064 match Either::B {
1065 Either::A { foo: 5 } => (),
1066 }
1067 }
1068 ";
1069
1070 // This is a false negative.
1071 // We don't currently handle enum record types.
1072 check_no_diagnostic(content);
1073 }
1074
1075 #[test]
1076 fn enum_not_in_scope() {
1077 let content = r"
1078 fn test_fn() {
1079 match Foo::Bar {
1080 Foo::Baz => (),
1081 }
1082 }
1083 ";
1084
1085 // This is a false negative.
1086 // The enum is not in scope so we don't perform exhaustiveness checking.
1087 check_no_diagnostic(content);
1088 }
944} 1089}
diff --git a/crates/ra_hir_ty/src/expr.rs b/crates/ra_hir_ty/src/expr.rs
index 3caeeb394..7498d04dc 100644
--- a/crates/ra_hir_ty/src/expr.rs
+++ b/crates/ra_hir_ty/src/expr.rs
@@ -90,9 +90,12 @@ impl<'a, 'b> ExprValidator<'a, 'b> {
90 } 90 }
91 91
92 match is_useful(&cx, &seen, &PatStack::from_wild()) { 92 match is_useful(&cx, &seen, &PatStack::from_wild()) {
93 Usefulness::Useful => (), 93 Ok(Usefulness::Useful) => (),
94 // if a wildcard pattern is not useful, then all patterns are covered 94 // if a wildcard pattern is not useful, then all patterns are covered
95 Usefulness::NotUseful => return, 95 Ok(Usefulness::NotUseful) => return,
96 // this path is for unimplemented checks, so we err on the side of not
97 // reporting any errors
98 _ => return,
96 } 99 }
97 100
98 if let Ok(source_ptr) = source_map.expr_syntax(id) { 101 if let Ok(source_ptr) = source_map.expr_syntax(id) {