Skip to content

Commit

Permalink
handle tuple patterns with ellipsis
Browse files Browse the repository at this point in the history
  • Loading branch information
@JoshMcguigan
JoshMcguigan committed Apr 12, 2020
1 parent 268b798 commit 6e29227
Show file tree
Hide file tree
Showing 4 changed files with 141 additions and 58 deletions.
27 changes: 22 additions & 5 deletions crates/ra_hir_def/src/body/lower.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ use crate::{
};

use super::{ExprSource, PatSource};
use ast::AstChildren;

pub(super) fn lower(
db: &dyn DefDatabase,
Expand Down Expand Up @@ -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());
Expand All @@ -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 =
Expand Down Expand Up @@ -665,6 +666,9 @@ impl ExprCollector<'_> {
Pat::Missing
}
}
ast::Pat::DotDotPat(_) => unreachable!(
"`DotDotPat` requires special handling and should not be mapped to PatId."
),
// FIXME: implement
ast::Pat::BoxPat(_) | ast::Pat::RangePat(_) | ast::Pat::MacroPat(_) => Pat::Missing,
};
Expand All @@ -679,6 +683,19 @@ impl ExprCollector<'_> {
self.missing_pat()
}
}

fn collect_tuple_pat(&mut self, args: AstChildren<ast::Pat>) -> (Vec<PatId>, Option<usize>) {
// 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<ast::BinOp> for BinaryOp {
Expand Down
6 changes: 3 additions & 3 deletions crates/ra_hir_def/src/expr.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -374,15 +374,15 @@ pub struct RecordFieldPat {
pub enum Pat {
Missing,
Wild,
Tuple(Vec<PatId>),
Tuple { args: Vec<PatId>, ellipsis: Option<usize> },
Or(Vec<PatId>),
Record { path: Option<Path>, args: Vec<RecordFieldPat>, ellipsis: bool },
Range { start: ExprId, end: ExprId },
Slice { prefix: Vec<PatId>, slice: Option<PatId>, suffix: Vec<PatId> },
Path(Path),
Lit(ExprId),
Bind { mode: BindingAnnotation, name: Name, subpat: Option<PatId> },
TupleStruct { path: Option<Path>, args: Vec<PatId> },
TupleStruct { path: Option<Path>, args: Vec<PatId>, ellipsis: Option<usize> },
Ref { pat: PatId, mutability: Mutability },
}

Expand All @@ -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),
Expand Down
160 changes: 113 additions & 47 deletions crates/ra_hir_ty/src/_match.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<T: Into<PatIdOrWild> + Copy>(&self, pat_ids: &[T]) -> PatStack {
let mut patterns: PatStackInner = smallvec![];
for pat in pat_ids {
patterns.push((*pat).into());
Expand Down Expand Up @@ -320,12 +320,14 @@ impl PatStack {
constructor: &Constructor,
) -> MatchCheckResult<Option<PatStack>> {
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))
}
Expand All @@ -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_pattterns: Vec<PatIdOrWild> = vec![];

for pat_id in &pat_ids[0..ellipsis_position] {
new_pattterns.push((*pat_id).into());
}

for _ in 0..(constructor_arity - pat_ids.len()) {
new_pattterns.push(PatIdOrWild::Wild);
}

for pat_id in &pat_ids[ellipsis_position..pat_ids.len()] {
new_pattterns.push((*pat_id).into());
}

Some(self.replace_head_with(&new_pattterns))
} 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),
Expand Down Expand Up @@ -644,7 +674,11 @@ impl Constructor {
fn pat_constructor(cx: &MatchCheckCtx, pat: PatIdOrWild) -> MatchCheckResult<Option<Constructor>> {
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),
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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);
}

Expand All @@ -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);
}
}
6 changes: 3 additions & 3 deletions crates/ra_hir_ty/src/infer/pat.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 { .. }
Expand Down Expand Up @@ -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,
_ => &[],
Expand Down Expand Up @@ -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: _ } => {
Expand Down

0 comments on commit 6e29227

Please sign in to comment.