diff options
author | bors[bot] <26634292+bors[bot]@users.noreply.github.com> | 2020-04-13 16:20:17 +0100 |
---|---|---|
committer | GitHub <[email protected]> | 2020-04-13 16:20:17 +0100 |
commit | d075f49e6d342737c8eb81bd5448503bdc33bd79 (patch) | |
tree | 2616cbd7be4a0b8f29a6a6d8f977f8817cc24ce9 /crates/ra_hir_ty/src | |
parent | c388130f5ffbcbe7d3131213a24d12d02f769b87 (diff) | |
parent | ee822d19b7662a9055bc6693c4c40d8dcf752ea1 (diff) |
Merge #3960
3960: ellipsis in tuple patterns r=JoshMcguigan a=JoshMcguigan
This PR lowers ellipsis in tuple patterns. It fixes a bug in the way ellipsis were previously lowered (by replacing the ellipsis with a single `Pat::Wild` no matter how many items the `..` was taking the place of).
It also uses this new information to properly handle `..` in tuple struct patterns when perform match statement exhaustiveness checks.
While this PR provides the building blocks for match statement exhaustiveness checks for tuples, there are some additional challenges there, so that is still unimplemented (unlike tuple structs).
Co-authored-by: Josh Mcguigan <[email protected]>
Diffstat (limited to 'crates/ra_hir_ty/src')
-rw-r--r-- | crates/ra_hir_ty/src/_match.rs | 160 | ||||
-rw-r--r-- | crates/ra_hir_ty/src/infer/pat.rs | 6 |
2 files changed, 116 insertions, 50 deletions
diff --git a/crates/ra_hir_ty/src/_match.rs b/crates/ra_hir_ty/src/_match.rs index c482cf619..a64be9848 100644 --- a/crates/ra_hir_ty/src/_match.rs +++ b/crates/ra_hir_ty/src/_match.rs | |||
@@ -289,7 +289,7 @@ impl PatStack { | |||
289 | Self::from_slice(&self.0[1..]) | 289 | Self::from_slice(&self.0[1..]) |
290 | } | 290 | } |
291 | 291 | ||
292 | fn replace_head_with(&self, pat_ids: &[PatId]) -> PatStack { | 292 | fn replace_head_with<T: Into<PatIdOrWild> + Copy>(&self, pat_ids: &[T]) -> PatStack { |
293 | let mut patterns: PatStackInner = smallvec![]; | 293 | let mut patterns: PatStackInner = smallvec![]; |
294 | for pat in pat_ids { | 294 | for pat in pat_ids { |
295 | patterns.push((*pat).into()); | 295 | patterns.push((*pat).into()); |
@@ -320,12 +320,14 @@ impl PatStack { | |||
320 | constructor: &Constructor, | 320 | constructor: &Constructor, |
321 | ) -> MatchCheckResult<Option<PatStack>> { | 321 | ) -> MatchCheckResult<Option<PatStack>> { |
322 | let result = match (self.head().as_pat(cx), constructor) { | 322 | let result = match (self.head().as_pat(cx), constructor) { |
323 | (Pat::Tuple(ref pat_ids), Constructor::Tuple { arity }) => { | 323 | (Pat::Tuple { args: ref pat_ids, ellipsis }, Constructor::Tuple { arity: _ }) => { |
324 | debug_assert_eq!( | 324 | if ellipsis.is_some() { |
325 | pat_ids.len(), | 325 | // If there are ellipsis here, we should add the correct number of |
326 | *arity, | 326 | // Pat::Wild patterns to `pat_ids`. We should be able to use the |
327 | "we type check before calling this code, so we should never hit this case", | 327 | // constructors arity for this, but at the time of writing we aren't |
328 | ); | 328 | // correctly calculating this arity when ellipsis are present. |
329 | return Err(MatchCheckErr::NotImplemented); | ||
330 | } | ||
329 | 331 | ||
330 | Some(self.replace_head_with(pat_ids)) | 332 | Some(self.replace_head_with(pat_ids)) |
331 | } | 333 | } |
@@ -351,19 +353,47 @@ impl PatStack { | |||
351 | Some(self.to_tail()) | 353 | Some(self.to_tail()) |
352 | } | 354 | } |
353 | } | 355 | } |
354 | (Pat::TupleStruct { args: ref pat_ids, .. }, Constructor::Enum(enum_constructor)) => { | 356 | ( |
357 | Pat::TupleStruct { args: ref pat_ids, ellipsis, .. }, | ||
358 | Constructor::Enum(enum_constructor), | ||
359 | ) => { | ||
355 | let pat_id = self.head().as_id().expect("we know this isn't a wild"); | 360 | let pat_id = self.head().as_id().expect("we know this isn't a wild"); |
356 | if !enum_variant_matches(cx, pat_id, *enum_constructor) { | 361 | if !enum_variant_matches(cx, pat_id, *enum_constructor) { |
357 | None | 362 | None |
358 | } else { | 363 | } else { |
359 | // If the enum variant matches, then we need to confirm | 364 | let constructor_arity = constructor.arity(cx)?; |
360 | // that the number of patterns aligns with the expected | 365 | if let Some(ellipsis_position) = ellipsis { |
361 | // number of patterns for that enum variant. | 366 | // If there are ellipsis in the pattern, the ellipsis must take the place |
362 | if pat_ids.len() != constructor.arity(cx)? { | 367 | // of at least one sub-pattern, so `pat_ids` should be smaller than the |
363 | return Err(MatchCheckErr::MalformedMatchArm); | 368 | // constructor arity. |
369 | if pat_ids.len() < constructor_arity { | ||
370 | let mut new_patterns: Vec<PatIdOrWild> = vec![]; | ||
371 | |||
372 | for pat_id in &pat_ids[0..ellipsis_position] { | ||
373 | new_patterns.push((*pat_id).into()); | ||
374 | } | ||
375 | |||
376 | for _ in 0..(constructor_arity - pat_ids.len()) { | ||
377 | new_patterns.push(PatIdOrWild::Wild); | ||
378 | } | ||
379 | |||
380 | for pat_id in &pat_ids[ellipsis_position..pat_ids.len()] { | ||
381 | new_patterns.push((*pat_id).into()); | ||
382 | } | ||
383 | |||
384 | Some(self.replace_head_with(&new_patterns)) | ||
385 | } else { | ||
386 | return Err(MatchCheckErr::MalformedMatchArm); | ||
387 | } | ||
388 | } else { | ||
389 | // If there is no ellipsis in the tuple pattern, the number | ||
390 | // of patterns must equal the constructor arity. | ||
391 | if pat_ids.len() == constructor_arity { | ||
392 | Some(self.replace_head_with(pat_ids)) | ||
393 | } else { | ||
394 | return Err(MatchCheckErr::MalformedMatchArm); | ||
395 | } | ||
364 | } | 396 | } |
365 | |||
366 | Some(self.replace_head_with(pat_ids)) | ||
367 | } | 397 | } |
368 | } | 398 | } |
369 | (Pat::Or(_), _) => return Err(MatchCheckErr::NotImplemented), | 399 | (Pat::Or(_), _) => return Err(MatchCheckErr::NotImplemented), |
@@ -644,7 +674,11 @@ impl Constructor { | |||
644 | fn pat_constructor(cx: &MatchCheckCtx, pat: PatIdOrWild) -> MatchCheckResult<Option<Constructor>> { | 674 | fn pat_constructor(cx: &MatchCheckCtx, pat: PatIdOrWild) -> MatchCheckResult<Option<Constructor>> { |
645 | let res = match pat.as_pat(cx) { | 675 | let res = match pat.as_pat(cx) { |
646 | Pat::Wild => None, | 676 | Pat::Wild => None, |
647 | Pat::Tuple(pats) => Some(Constructor::Tuple { arity: pats.len() }), | 677 | // FIXME somehow create the Tuple constructor with the proper arity. If there are |
678 | // ellipsis, the arity is not equal to the number of patterns. | ||
679 | Pat::Tuple { args: pats, ellipsis } if ellipsis.is_none() => { | ||
680 | Some(Constructor::Tuple { arity: pats.len() }) | ||
681 | } | ||
648 | Pat::Lit(lit_expr) => match cx.body.exprs[lit_expr] { | 682 | Pat::Lit(lit_expr) => match cx.body.exprs[lit_expr] { |
649 | Expr::Literal(Literal::Bool(val)) => Some(Constructor::Bool(val)), | 683 | Expr::Literal(Literal::Bool(val)) => Some(Constructor::Bool(val)), |
650 | _ => return Err(MatchCheckErr::NotImplemented), | 684 | _ => return Err(MatchCheckErr::NotImplemented), |
@@ -1507,6 +1541,67 @@ mod tests { | |||
1507 | } | 1541 | } |
1508 | 1542 | ||
1509 | #[test] | 1543 | #[test] |
1544 | fn enum_tuple_partial_ellipsis_2_no_diagnostic() { | ||
1545 | let content = r" | ||
1546 | enum Either { | ||
1547 | A(bool, bool, bool, bool), | ||
1548 | B, | ||
1549 | } | ||
1550 | fn test_fn() { | ||
1551 | match Either::B { | ||
1552 | Either::A(true, .., true) => {}, | ||
1553 | Either::A(true, .., false) => {}, | ||
1554 | Either::A(.., true) => {}, | ||
1555 | Either::A(.., false) => {}, | ||
1556 | Either::B => {}, | ||
1557 | } | ||
1558 | } | ||
1559 | "; | ||
1560 | |||
1561 | check_no_diagnostic(content); | ||
1562 | } | ||
1563 | |||
1564 | #[test] | ||
1565 | fn enum_tuple_partial_ellipsis_missing_arm() { | ||
1566 | let content = r" | ||
1567 | enum Either { | ||
1568 | A(bool, bool, bool, bool), | ||
1569 | B, | ||
1570 | } | ||
1571 | fn test_fn() { | ||
1572 | match Either::B { | ||
1573 | Either::A(true, .., true) => {}, | ||
1574 | Either::A(true, .., false) => {}, | ||
1575 | Either::A(false, .., false) => {}, | ||
1576 | Either::B => {}, | ||
1577 | } | ||
1578 | } | ||
1579 | "; | ||
1580 | |||
1581 | check_diagnostic(content); | ||
1582 | } | ||
1583 | |||
1584 | #[test] | ||
1585 | fn enum_tuple_partial_ellipsis_2_missing_arm() { | ||
1586 | let content = r" | ||
1587 | enum Either { | ||
1588 | A(bool, bool, bool, bool), | ||
1589 | B, | ||
1590 | } | ||
1591 | fn test_fn() { | ||
1592 | match Either::B { | ||
1593 | Either::A(true, .., true) => {}, | ||
1594 | Either::A(true, .., false) => {}, | ||
1595 | Either::A(.., true) => {}, | ||
1596 | Either::B => {}, | ||
1597 | } | ||
1598 | } | ||
1599 | "; | ||
1600 | |||
1601 | check_diagnostic(content); | ||
1602 | } | ||
1603 | |||
1604 | #[test] | ||
1510 | fn enum_tuple_ellipsis_no_diagnostic() { | 1605 | fn enum_tuple_ellipsis_no_diagnostic() { |
1511 | let content = r" | 1606 | let content = r" |
1512 | enum Either { | 1607 | enum Either { |
@@ -1645,11 +1740,7 @@ mod false_negatives { | |||
1645 | "; | 1740 | "; |
1646 | 1741 | ||
1647 | // This is a false negative. | 1742 | // This is a false negative. |
1648 | // The `..` pattern is currently lowered to a single `Pat::Wild` | 1743 | // We don't currently handle tuple patterns with ellipsis. |
1649 | // no matter how many fields the `..` pattern is covering. This | ||
1650 | // causes the match arm in this test not to type check against | ||
1651 | // the match expression, which causes this diagnostic not to | ||
1652 | // fire. | ||
1653 | check_no_diagnostic(content); | 1744 | check_no_diagnostic(content); |
1654 | } | 1745 | } |
1655 | 1746 | ||
@@ -1664,32 +1755,7 @@ mod false_negatives { | |||
1664 | "; | 1755 | "; |
1665 | 1756 | ||
1666 | // This is a false negative. | 1757 | // This is a false negative. |
1667 | // See comments on `tuple_of_bools_with_ellipsis_at_end_missing_arm`. | 1758 | // We don't currently handle tuple patterns with ellipsis. |
1668 | check_no_diagnostic(content); | ||
1669 | } | ||
1670 | |||
1671 | #[test] | ||
1672 | fn enum_tuple_partial_ellipsis_missing_arm() { | ||
1673 | let content = r" | ||
1674 | enum Either { | ||
1675 | A(bool, bool, bool, bool), | ||
1676 | B, | ||
1677 | } | ||
1678 | fn test_fn() { | ||
1679 | match Either::B { | ||
1680 | Either::A(true, .., true) => {}, | ||
1681 | Either::A(true, .., false) => {}, | ||
1682 | Either::A(false, .., false) => {}, | ||
1683 | Either::B => {}, | ||
1684 | } | ||
1685 | } | ||
1686 | "; | ||
1687 | |||
1688 | // This is a false negative. | ||
1689 | // The `..` pattern is currently lowered to a single `Pat::Wild` | ||
1690 | // no matter how many fields the `..` pattern is covering. This | ||
1691 | // causes us to return a `MatchCheckErr::MalformedMatchArm` in | ||
1692 | // `Pat::specialize_constructor`. | ||
1693 | check_no_diagnostic(content); | 1759 | check_no_diagnostic(content); |
1694 | } | 1760 | } |
1695 | } | 1761 | } |
diff --git a/crates/ra_hir_ty/src/infer/pat.rs b/crates/ra_hir_ty/src/infer/pat.rs index 078476f76..8ec4d4ace 100644 --- a/crates/ra_hir_ty/src/infer/pat.rs +++ b/crates/ra_hir_ty/src/infer/pat.rs | |||
@@ -85,7 +85,7 @@ impl<'a> InferenceContext<'a> { | |||
85 | let body = Arc::clone(&self.body); // avoid borrow checker problem | 85 | let body = Arc::clone(&self.body); // avoid borrow checker problem |
86 | 86 | ||
87 | let is_non_ref_pat = match &body[pat] { | 87 | let is_non_ref_pat = match &body[pat] { |
88 | Pat::Tuple(..) | 88 | Pat::Tuple { .. } |
89 | | Pat::Or(..) | 89 | | Pat::Or(..) |
90 | | Pat::TupleStruct { .. } | 90 | | Pat::TupleStruct { .. } |
91 | | Pat::Record { .. } | 91 | | Pat::Record { .. } |
@@ -116,7 +116,7 @@ impl<'a> InferenceContext<'a> { | |||
116 | let expected = expected; | 116 | let expected = expected; |
117 | 117 | ||
118 | let ty = match &body[pat] { | 118 | let ty = match &body[pat] { |
119 | Pat::Tuple(ref args) => { | 119 | Pat::Tuple { ref args, .. } => { |
120 | let expectations = match expected.as_tuple() { | 120 | let expectations = match expected.as_tuple() { |
121 | Some(parameters) => &*parameters.0, | 121 | Some(parameters) => &*parameters.0, |
122 | _ => &[], | 122 | _ => &[], |
@@ -155,7 +155,7 @@ impl<'a> InferenceContext<'a> { | |||
155 | let subty = self.infer_pat(*pat, expectation, default_bm); | 155 | let subty = self.infer_pat(*pat, expectation, default_bm); |
156 | Ty::apply_one(TypeCtor::Ref(*mutability), subty) | 156 | Ty::apply_one(TypeCtor::Ref(*mutability), subty) |
157 | } | 157 | } |
158 | Pat::TupleStruct { path: p, args: subpats } => { | 158 | Pat::TupleStruct { path: p, args: subpats, .. } => { |
159 | self.infer_tuple_struct_pat(p.as_ref(), subpats, expected, default_bm, pat) | 159 | self.infer_tuple_struct_pat(p.as_ref(), subpats, expected, default_bm, pat) |
160 | } | 160 | } |
161 | Pat::Record { path: p, args: fields, ellipsis: _ } => { | 161 | Pat::Record { path: p, args: fields, ellipsis: _ } => { |