From ee822d19b7662a9055bc6693c4c40d8dcf752ea1 Mon Sep 17 00:00:00 2001 From: Josh Mcguigan Date: Sun, 12 Apr 2020 08:40:09 -0700 Subject: handle tuple patterns with ellipsis --- crates/ra_hir_ty/src/_match.rs | 160 +++++++++++++++++++++++++++----------- crates/ra_hir_ty/src/infer/pat.rs | 6 +- 2 files changed, 116 insertions(+), 50 deletions(-) (limited to 'crates/ra_hir_ty/src') 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 { Self::from_slice(&self.0[1..]) } - fn replace_head_with(&self, pat_ids: &[PatId]) -> PatStack { + fn replace_head_with + Copy>(&self, pat_ids: &[T]) -> PatStack { let mut patterns: PatStackInner = smallvec![]; for pat in pat_ids { patterns.push((*pat).into()); @@ -320,12 +320,14 @@ impl PatStack { constructor: &Constructor, ) -> MatchCheckResult> { let result = match (self.head().as_pat(cx), constructor) { - (Pat::Tuple(ref pat_ids), Constructor::Tuple { arity }) => { - debug_assert_eq!( - pat_ids.len(), - *arity, - "we type check before calling this code, so we should never hit this case", - ); + (Pat::Tuple { args: ref pat_ids, ellipsis }, Constructor::Tuple { arity: _ }) => { + if ellipsis.is_some() { + // If there are ellipsis here, we should add the correct number of + // Pat::Wild patterns to `pat_ids`. We should be able to use the + // constructors arity for this, but at the time of writing we aren't + // correctly calculating this arity when ellipsis are present. + return Err(MatchCheckErr::NotImplemented); + } Some(self.replace_head_with(pat_ids)) } @@ -351,19 +353,47 @@ impl PatStack { Some(self.to_tail()) } } - (Pat::TupleStruct { args: ref pat_ids, .. }, Constructor::Enum(enum_constructor)) => { + ( + Pat::TupleStruct { args: ref pat_ids, ellipsis, .. }, + Constructor::Enum(enum_constructor), + ) => { let pat_id = self.head().as_id().expect("we know this isn't a wild"); if !enum_variant_matches(cx, pat_id, *enum_constructor) { None } else { - // If the enum variant matches, then we need to confirm - // that the number of patterns aligns with the expected - // number of patterns for that enum variant. - if pat_ids.len() != constructor.arity(cx)? { - return Err(MatchCheckErr::MalformedMatchArm); + let constructor_arity = constructor.arity(cx)?; + if let Some(ellipsis_position) = ellipsis { + // If there are ellipsis in the pattern, the ellipsis must take the place + // of at least one sub-pattern, so `pat_ids` should be smaller than the + // constructor arity. + if pat_ids.len() < constructor_arity { + let mut new_patterns: Vec = vec![]; + + for pat_id in &pat_ids[0..ellipsis_position] { + new_patterns.push((*pat_id).into()); + } + + for _ in 0..(constructor_arity - pat_ids.len()) { + new_patterns.push(PatIdOrWild::Wild); + } + + for pat_id in &pat_ids[ellipsis_position..pat_ids.len()] { + new_patterns.push((*pat_id).into()); + } + + Some(self.replace_head_with(&new_patterns)) + } else { + return Err(MatchCheckErr::MalformedMatchArm); + } + } else { + // If there is no ellipsis in the tuple pattern, the number + // of patterns must equal the constructor arity. + if pat_ids.len() == constructor_arity { + Some(self.replace_head_with(pat_ids)) + } else { + return Err(MatchCheckErr::MalformedMatchArm); + } } - - Some(self.replace_head_with(pat_ids)) } } (Pat::Or(_), _) => return Err(MatchCheckErr::NotImplemented), @@ -644,7 +674,11 @@ impl Constructor { fn pat_constructor(cx: &MatchCheckCtx, pat: PatIdOrWild) -> MatchCheckResult> { let res = match pat.as_pat(cx) { Pat::Wild => None, - Pat::Tuple(pats) => Some(Constructor::Tuple { arity: pats.len() }), + // FIXME somehow create the Tuple constructor with the proper arity. If there are + // ellipsis, the arity is not equal to the number of patterns. + Pat::Tuple { args: pats, ellipsis } if ellipsis.is_none() => { + Some(Constructor::Tuple { arity: pats.len() }) + } Pat::Lit(lit_expr) => match cx.body.exprs[lit_expr] { Expr::Literal(Literal::Bool(val)) => Some(Constructor::Bool(val)), _ => return Err(MatchCheckErr::NotImplemented), @@ -1506,6 +1540,67 @@ mod tests { check_no_diagnostic(content); } + #[test] + fn enum_tuple_partial_ellipsis_2_no_diagnostic() { + let content = r" + enum Either { + A(bool, bool, bool, bool), + B, + } + fn test_fn() { + match Either::B { + Either::A(true, .., true) => {}, + Either::A(true, .., false) => {}, + Either::A(.., true) => {}, + Either::A(.., false) => {}, + Either::B => {}, + } + } + "; + + check_no_diagnostic(content); + } + + #[test] + fn enum_tuple_partial_ellipsis_missing_arm() { + let content = r" + enum Either { + A(bool, bool, bool, bool), + B, + } + fn test_fn() { + match Either::B { + Either::A(true, .., true) => {}, + Either::A(true, .., false) => {}, + Either::A(false, .., false) => {}, + Either::B => {}, + } + } + "; + + check_diagnostic(content); + } + + #[test] + fn enum_tuple_partial_ellipsis_2_missing_arm() { + let content = r" + enum Either { + A(bool, bool, bool, bool), + B, + } + fn test_fn() { + match Either::B { + Either::A(true, .., true) => {}, + Either::A(true, .., false) => {}, + Either::A(.., true) => {}, + Either::B => {}, + } + } + "; + + check_diagnostic(content); + } + #[test] fn enum_tuple_ellipsis_no_diagnostic() { let content = r" @@ -1645,11 +1740,7 @@ mod false_negatives { "; // This is a false negative. - // The `..` pattern is currently lowered to a single `Pat::Wild` - // no matter how many fields the `..` pattern is covering. This - // causes the match arm in this test not to type check against - // the match expression, which causes this diagnostic not to - // fire. + // We don't currently handle tuple patterns with ellipsis. check_no_diagnostic(content); } @@ -1664,32 +1755,7 @@ mod false_negatives { "; // This is a false negative. - // See comments on `tuple_of_bools_with_ellipsis_at_end_missing_arm`. - check_no_diagnostic(content); - } - - #[test] - fn enum_tuple_partial_ellipsis_missing_arm() { - let content = r" - enum Either { - A(bool, bool, bool, bool), - B, - } - fn test_fn() { - match Either::B { - Either::A(true, .., true) => {}, - Either::A(true, .., false) => {}, - Either::A(false, .., false) => {}, - Either::B => {}, - } - } - "; - - // This is a false negative. - // The `..` pattern is currently lowered to a single `Pat::Wild` - // no matter how many fields the `..` pattern is covering. This - // causes us to return a `MatchCheckErr::MalformedMatchArm` in - // `Pat::specialize_constructor`. + // We don't currently handle tuple patterns with ellipsis. check_no_diagnostic(content); } } 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> { let body = Arc::clone(&self.body); // avoid borrow checker problem let is_non_ref_pat = match &body[pat] { - Pat::Tuple(..) + Pat::Tuple { .. } | Pat::Or(..) | Pat::TupleStruct { .. } | Pat::Record { .. } @@ -116,7 +116,7 @@ impl<'a> InferenceContext<'a> { let expected = expected; let ty = match &body[pat] { - Pat::Tuple(ref args) => { + Pat::Tuple { ref args, .. } => { let expectations = match expected.as_tuple() { Some(parameters) => &*parameters.0, _ => &[], @@ -155,7 +155,7 @@ impl<'a> InferenceContext<'a> { let subty = self.infer_pat(*pat, expectation, default_bm); Ty::apply_one(TypeCtor::Ref(*mutability), subty) } - Pat::TupleStruct { path: p, args: subpats } => { + Pat::TupleStruct { path: p, args: subpats, .. } => { self.infer_tuple_struct_pat(p.as_ref(), subpats, expected, default_bm, pat) } Pat::Record { path: p, args: fields, ellipsis: _ } => { -- cgit v1.2.3