diff options
author | bors[bot] <26634292+bors[bot]@users.noreply.github.com> | 2020-06-18 20:43:05 +0100 |
---|---|---|
committer | GitHub <[email protected]> | 2020-06-18 20:43:05 +0100 |
commit | 902a9c6da7939abec74bb4e4be9d1d16dfb15daa (patch) | |
tree | 2c9787b68d1ec457ec3df49c41db537dcb6a8737 /crates/ra_hir_ty | |
parent | 1e35c7405560b2c0badaa9ee4945a131b733bda8 (diff) | |
parent | 4d6d7aec50e81d2d6da01e4f4fc1353430e79067 (diff) |
Merge #4930
4930: Avoid all unchecked indexing in match checking r=flodiebold a=jonas-schievink
Fixes https://github.com/rust-analyzer/rust-analyzer/issues/4416, but replaces it with a false positive.
r? @flodiebold
Co-authored-by: Jonas Schievink <[email protected]>
Diffstat (limited to 'crates/ra_hir_ty')
-rw-r--r-- | crates/ra_hir_ty/src/_match.rs | 77 |
1 files changed, 49 insertions, 28 deletions
diff --git a/crates/ra_hir_ty/src/_match.rs b/crates/ra_hir_ty/src/_match.rs index 02a7a61f1..5495ce284 100644 --- a/crates/ra_hir_ty/src/_match.rs +++ b/crates/ra_hir_ty/src/_match.rs | |||
@@ -312,20 +312,16 @@ impl PatStack { | |||
312 | Self(v) | 312 | Self(v) |
313 | } | 313 | } |
314 | 314 | ||
315 | fn is_empty(&self) -> bool { | ||
316 | self.0.is_empty() | ||
317 | } | ||
318 | |||
319 | fn head(&self) -> PatIdOrWild { | ||
320 | self.0[0] | ||
321 | } | ||
322 | |||
323 | fn get_head(&self) -> Option<PatIdOrWild> { | 315 | fn get_head(&self) -> Option<PatIdOrWild> { |
324 | self.0.first().copied() | 316 | self.0.first().copied() |
325 | } | 317 | } |
326 | 318 | ||
319 | fn tail(&self) -> &[PatIdOrWild] { | ||
320 | self.0.get(1..).unwrap_or(&[]) | ||
321 | } | ||
322 | |||
327 | fn to_tail(&self) -> PatStack { | 323 | fn to_tail(&self) -> PatStack { |
328 | Self::from_slice(&self.0[1..]) | 324 | Self::from_slice(self.tail()) |
329 | } | 325 | } |
330 | 326 | ||
331 | fn replace_head_with<I, T>(&self, pats: I) -> PatStack | 327 | fn replace_head_with<I, T>(&self, pats: I) -> PatStack |
@@ -347,7 +343,7 @@ impl PatStack { | |||
347 | /// | 343 | /// |
348 | /// See the module docs and the associated documentation in rustc for details. | 344 | /// See the module docs and the associated documentation in rustc for details. |
349 | fn specialize_wildcard(&self, cx: &MatchCheckCtx) -> Option<PatStack> { | 345 | fn specialize_wildcard(&self, cx: &MatchCheckCtx) -> Option<PatStack> { |
350 | if matches!(self.head().as_pat(cx), Pat::Wild) { | 346 | if matches!(self.get_head()?.as_pat(cx), Pat::Wild) { |
351 | Some(self.to_tail()) | 347 | Some(self.to_tail()) |
352 | } else { | 348 | } else { |
353 | None | 349 | None |
@@ -362,11 +358,12 @@ impl PatStack { | |||
362 | cx: &MatchCheckCtx, | 358 | cx: &MatchCheckCtx, |
363 | constructor: &Constructor, | 359 | constructor: &Constructor, |
364 | ) -> MatchCheckResult<Option<PatStack>> { | 360 | ) -> MatchCheckResult<Option<PatStack>> { |
365 | if self.is_empty() { | 361 | let head = match self.get_head() { |
366 | return Ok(None); | 362 | Some(head) => head, |
367 | } | 363 | None => return Ok(None), |
364 | }; | ||
368 | 365 | ||
369 | let head_pat = self.head().as_pat(cx); | 366 | let head_pat = head.as_pat(cx); |
370 | let result = match (head_pat, constructor) { | 367 | let result = match (head_pat, constructor) { |
371 | (Pat::Tuple { args: ref pat_ids, ellipsis }, Constructor::Tuple { arity: _ }) => { | 368 | (Pat::Tuple { args: ref pat_ids, ellipsis }, Constructor::Tuple { arity: _ }) => { |
372 | if ellipsis.is_some() { | 369 | if ellipsis.is_some() { |
@@ -394,7 +391,7 @@ impl PatStack { | |||
394 | (Pat::Wild, constructor) => Some(self.expand_wildcard(cx, constructor)?), | 391 | (Pat::Wild, constructor) => Some(self.expand_wildcard(cx, constructor)?), |
395 | (Pat::Path(_), Constructor::Enum(constructor)) => { | 392 | (Pat::Path(_), Constructor::Enum(constructor)) => { |
396 | // unit enum variants become `Pat::Path` | 393 | // unit enum variants become `Pat::Path` |
397 | let pat_id = self.head().as_id().expect("we know this isn't a wild"); | 394 | let pat_id = head.as_id().expect("we know this isn't a wild"); |
398 | if !enum_variant_matches(cx, pat_id, *constructor) { | 395 | if !enum_variant_matches(cx, pat_id, *constructor) { |
399 | None | 396 | None |
400 | } else { | 397 | } else { |
@@ -405,7 +402,7 @@ impl PatStack { | |||
405 | Pat::TupleStruct { args: ref pat_ids, ellipsis, .. }, | 402 | Pat::TupleStruct { args: ref pat_ids, ellipsis, .. }, |
406 | Constructor::Enum(enum_constructor), | 403 | Constructor::Enum(enum_constructor), |
407 | ) => { | 404 | ) => { |
408 | let pat_id = self.head().as_id().expect("we know this isn't a wild"); | 405 | let pat_id = head.as_id().expect("we know this isn't a wild"); |
409 | if !enum_variant_matches(cx, pat_id, *enum_constructor) { | 406 | if !enum_variant_matches(cx, pat_id, *enum_constructor) { |
410 | None | 407 | None |
411 | } else { | 408 | } else { |
@@ -445,7 +442,7 @@ impl PatStack { | |||
445 | } | 442 | } |
446 | } | 443 | } |
447 | (Pat::Record { args: ref arg_patterns, .. }, Constructor::Enum(e)) => { | 444 | (Pat::Record { args: ref arg_patterns, .. }, Constructor::Enum(e)) => { |
448 | let pat_id = self.head().as_id().expect("we know this isn't a wild"); | 445 | let pat_id = head.as_id().expect("we know this isn't a wild"); |
449 | if !enum_variant_matches(cx, pat_id, *e) { | 446 | if !enum_variant_matches(cx, pat_id, *e) { |
450 | None | 447 | None |
451 | } else { | 448 | } else { |
@@ -491,7 +488,7 @@ impl PatStack { | |||
491 | ) -> MatchCheckResult<PatStack> { | 488 | ) -> MatchCheckResult<PatStack> { |
492 | assert_eq!( | 489 | assert_eq!( |
493 | Pat::Wild, | 490 | Pat::Wild, |
494 | self.head().as_pat(cx), | 491 | self.get_head().expect("expand_wildcard called on empty PatStack").as_pat(cx), |
495 | "expand_wildcard must only be called on PatStack with wild at head", | 492 | "expand_wildcard must only be called on PatStack with wild at head", |
496 | ); | 493 | ); |
497 | 494 | ||
@@ -509,7 +506,6 @@ impl PatStack { | |||
509 | } | 506 | } |
510 | } | 507 | } |
511 | 508 | ||
512 | #[derive(Debug)] | ||
513 | /// A collection of PatStack. | 509 | /// A collection of PatStack. |
514 | /// | 510 | /// |
515 | /// This type is modeled from the struct of the same name in `rustc`. | 511 | /// This type is modeled from the struct of the same name in `rustc`. |
@@ -623,13 +619,16 @@ pub(crate) fn is_useful( | |||
623 | _ => (), | 619 | _ => (), |
624 | } | 620 | } |
625 | 621 | ||
626 | if v.is_empty() { | 622 | let head = match v.get_head() { |
627 | let result = if matrix.is_empty() { Usefulness::Useful } else { Usefulness::NotUseful }; | 623 | Some(head) => head, |
624 | None => { | ||
625 | let result = if matrix.is_empty() { Usefulness::Useful } else { Usefulness::NotUseful }; | ||
628 | 626 | ||
629 | return Ok(result); | 627 | return Ok(result); |
630 | } | 628 | } |
629 | }; | ||
631 | 630 | ||
632 | if let Pat::Or(pat_ids) = v.head().as_pat(cx) { | 631 | if let Pat::Or(pat_ids) = head.as_pat(cx) { |
633 | let mut found_unimplemented = false; | 632 | let mut found_unimplemented = false; |
634 | let any_useful = pat_ids.iter().any(|&pat_id| { | 633 | let any_useful = pat_ids.iter().any(|&pat_id| { |
635 | let v = PatStack::from_pattern(pat_id); | 634 | let v = PatStack::from_pattern(pat_id); |
@@ -653,7 +652,7 @@ pub(crate) fn is_useful( | |||
653 | }; | 652 | }; |
654 | } | 653 | } |
655 | 654 | ||
656 | if let Some(constructor) = pat_constructor(cx, v.head())? { | 655 | if let Some(constructor) = pat_constructor(cx, head)? { |
657 | let matrix = matrix.specialize_constructor(&cx, &constructor)?; | 656 | let matrix = matrix.specialize_constructor(&cx, &constructor)?; |
658 | let v = v | 657 | let v = v |
659 | .specialize_constructor(&cx, &constructor)? | 658 | .specialize_constructor(&cx, &constructor)? |
@@ -854,10 +853,10 @@ mod tests { | |||
854 | } | 853 | } |
855 | 854 | ||
856 | pub(super) fn check_no_diagnostic(ra_fixture: &str) { | 855 | pub(super) fn check_no_diagnostic(ra_fixture: &str) { |
857 | let diagnostic_count = | 856 | let (s, diagnostic_count) = |
858 | TestDB::with_single_file(ra_fixture).0.diagnostic::<MissingMatchArms>().1; | 857 | TestDB::with_single_file(ra_fixture).0.diagnostic::<MissingMatchArms>(); |
859 | 858 | ||
860 | assert_eq!(0, diagnostic_count, "expected no diagnostic, found one"); | 859 | assert_eq!(0, diagnostic_count, "expected no diagnostic, found one: {}", s); |
861 | } | 860 | } |
862 | 861 | ||
863 | #[test] | 862 | #[test] |
@@ -2014,6 +2013,28 @@ mod tests { | |||
2014 | ", | 2013 | ", |
2015 | ); | 2014 | ); |
2016 | } | 2015 | } |
2016 | |||
2017 | #[test] | ||
2018 | fn or_pattern_panic_2() { | ||
2019 | // FIXME: This is a false positive, but the code used to cause a panic in the match checker, | ||
2020 | // so this acts as a regression test for that. | ||
2021 | check_diagnostic( | ||
2022 | r" | ||
2023 | pub enum Category { | ||
2024 | Infinity, | ||
2025 | Zero, | ||
2026 | } | ||
2027 | |||
2028 | fn panic(a: Category, b: Category) { | ||
2029 | match (a, b) { | ||
2030 | (Category::Infinity, Category::Infinity) | (Category::Zero, Category::Zero) => {} | ||
2031 | |||
2032 | (Category::Infinity | Category::Zero, _) => {} | ||
2033 | } | ||
2034 | } | ||
2035 | ", | ||
2036 | ); | ||
2037 | } | ||
2017 | } | 2038 | } |
2018 | 2039 | ||
2019 | #[cfg(test)] | 2040 | #[cfg(test)] |