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_def/src/body/lower.rs | 27 ++++-- crates/ra_hir_def/src/expr.rs | 6 +- crates/ra_hir_ty/src/_match.rs | 160 +++++++++++++++++++++++++----------- crates/ra_hir_ty/src/infer/pat.rs | 6 +- 4 files changed, 141 insertions(+), 58 deletions(-) diff --git a/crates/ra_hir_def/src/body/lower.rs b/crates/ra_hir_def/src/body/lower.rs index 6caa87db4..79abe55ce 100644 --- a/crates/ra_hir_def/src/body/lower.rs +++ b/crates/ra_hir_def/src/body/lower.rs @@ -33,6 +33,7 @@ use crate::{ }; use super::{ExprSource, PatSource}; +use ast::AstChildren; pub(super) fn lower( db: &dyn DefDatabase, @@ -598,8 +599,8 @@ impl ExprCollector<'_> { } ast::Pat::TupleStructPat(p) => { let path = p.path().and_then(|path| self.expander.parse_path(path)); - let args = p.args().map(|p| self.collect_pat(p)).collect(); - Pat::TupleStruct { path, args } + let (args, ellipsis) = self.collect_tuple_pat(p.args()); + Pat::TupleStruct { path, args, ellipsis } } ast::Pat::RefPat(p) => { let pat = self.collect_pat_opt(p.pat()); @@ -616,10 +617,10 @@ impl ExprCollector<'_> { } ast::Pat::ParenPat(p) => return self.collect_pat_opt(p.pat()), ast::Pat::TuplePat(p) => { - let args = p.args().map(|p| self.collect_pat(p)).collect(); - Pat::Tuple(args) + let (args, ellipsis) = self.collect_tuple_pat(p.args()); + Pat::Tuple { args, ellipsis } } - ast::Pat::PlaceholderPat(_) | ast::Pat::DotDotPat(_) => Pat::Wild, + ast::Pat::PlaceholderPat(_) => Pat::Wild, ast::Pat::RecordPat(p) => { let path = p.path().and_then(|path| self.expander.parse_path(path)); let record_field_pat_list = @@ -665,6 +666,9 @@ impl ExprCollector<'_> { Pat::Missing } } + ast::Pat::DotDotPat(_) => unreachable!( + "`DotDotPat` requires special handling and should not be mapped to a Pat." + ), // FIXME: implement ast::Pat::BoxPat(_) | ast::Pat::RangePat(_) | ast::Pat::MacroPat(_) => Pat::Missing, }; @@ -679,6 +683,19 @@ impl ExprCollector<'_> { self.missing_pat() } } + + fn collect_tuple_pat(&mut self, args: AstChildren) -> (Vec, Option) { + // Find the location of the `..`, if there is one. Note that we do not + // consider the possiblity of there being multiple `..` here. + let ellipsis = args.clone().position(|p| matches!(p, ast::Pat::DotDotPat(_))); + // We want to skip the `..` pattern here, since we account for it above. + let args = args + .filter(|p| !matches!(p, ast::Pat::DotDotPat(_))) + .map(|p| self.collect_pat(p)) + .collect(); + + (args, ellipsis) + } } impl From for BinaryOp { diff --git a/crates/ra_hir_def/src/expr.rs b/crates/ra_hir_def/src/expr.rs index e11bdf3ec..a0cdad529 100644 --- a/crates/ra_hir_def/src/expr.rs +++ b/crates/ra_hir_def/src/expr.rs @@ -374,7 +374,7 @@ pub struct RecordFieldPat { pub enum Pat { Missing, Wild, - Tuple(Vec), + Tuple { args: Vec, ellipsis: Option }, Or(Vec), Record { path: Option, args: Vec, ellipsis: bool }, Range { start: ExprId, end: ExprId }, @@ -382,7 +382,7 @@ pub enum Pat { Path(Path), Lit(ExprId), Bind { mode: BindingAnnotation, name: Name, subpat: Option }, - TupleStruct { path: Option, args: Vec }, + TupleStruct { path: Option, args: Vec, ellipsis: Option }, Ref { pat: PatId, mutability: Mutability }, } @@ -393,7 +393,7 @@ impl Pat { Pat::Bind { subpat, .. } => { subpat.iter().copied().for_each(f); } - Pat::Or(args) | Pat::Tuple(args) | Pat::TupleStruct { args, .. } => { + Pat::Or(args) | Pat::Tuple { args, .. } | Pat::TupleStruct { args, .. } => { args.iter().copied().for_each(f); } Pat::Ref { pat, .. } => f(*pat), 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