Correctly distinguish between tuple fields and associated values in patterns

include/swift/AST/DiagnosticsSema.def

@@ -3389,6 +3389,18 @@ ERROR(pattern_type_mismatch_context,none,
 
 ERROR(tuple_pattern_in_non_tuple_context,none,
       "tuple pattern cannot match values of the non-tuple type %0", (Type))
+WARNING(matching_pattern_with_many_assoc_values, none,
+        "cannot match several associated values at once, "
+        "implicitly tupling the associated values and trying to match that "
+        "instead", ())
+WARNING(matching_tuple_pattern_with_many_assoc_values,none,
+        "a tuple pattern cannot match several associated values at once, "
+        "implicitly tupling the associated values and trying to match "
+        "that instead", ())
+WARNING(matching_many_patterns_with_tupled_assoc_value,none,
+        "the enum case has a single tuple as an associated value, but "
+        "there are several patterns here, implicitly tupling the patterns "
+        "and trying to match that instead", ())
 ERROR(closure_argument_list_tuple,none,
       "contextual closure type %0 expects %1 argument%s1, "
       "but %2 %select{were|was}3 used in closure body", (Type, unsigned, unsigned, bool))

include/swift/AST/PatternNodes.def

@@ -32,17 +32,19 @@
 #define LAST_PATTERN(Id)
 #endif
 
-PATTERN(Paren, Pattern)
-PATTERN(Tuple, Pattern)
-PATTERN(Named, Pattern)
-PATTERN(Any, Pattern)
-PATTERN(Typed, Pattern)
-PATTERN(Var, Pattern)
-REFUTABLE_PATTERN(Is, Pattern)
-REFUTABLE_PATTERN(EnumElement, Pattern)
-REFUTABLE_PATTERN(OptionalSome, Pattern)
-REFUTABLE_PATTERN(Bool, Pattern)
-REFUTABLE_PATTERN(Expr, Pattern)
+// Metavars: x (variable), pat (pattern), e (expression)
+PATTERN(Paren, Pattern) // (pat)
+PATTERN(Tuple, Pattern) // (pat1, ..., patN), N >= 1
+PATTERN(Named, Pattern) // let pat, var pat
+PATTERN(Any, Pattern)   // _
+PATTERN(Typed, Pattern) // pat : type
+PATTERN(Var, Pattern)   // x
+REFUTABLE_PATTERN(Is, Pattern)           // x is myclass
+REFUTABLE_PATTERN(EnumElement, Pattern)  // .mycase(pat1, ..., patN)
+                                         // MyType.mycase(pat1, ..., patN)
+REFUTABLE_PATTERN(OptionalSome, Pattern) // pat?  nil
+REFUTABLE_PATTERN(Bool, Pattern)         // true, false
+REFUTABLE_PATTERN(Expr, Pattern)         // e
 LAST_PATTERN(Expr)
 
 #undef REFUTABLE_PATTERN

lib/Sema/TypeCheckPattern.cpp

@@ -1020,6 +1020,46 @@ bool TypeChecker::typeCheckPattern(Pattern *P, DeclContext *dc,
   llvm_unreachable("bad pattern kind!");
 }
 
+namespace {
+
+/// We need to allow particular matches for backwards compatibility, so we
+/// "repair" the pattern if needed, so that the exhaustiveness checker receives
+/// well-formed input. Also emit diagnostics warning the user to fix their code.
+///
+/// See SR-11160 and SR-11212 for more discussion.
+//
+// type ~ (T1, ..., Tn) (n >= 2)
+//   1a. pat ~ ((P1, ..., Pm)) (m >= 2)
+//   1b. pat
+// type ~ ((T1, ..., Tn)) (n >= 2)
+//   2. pat ~ (P1, ..., Pm) (m >= 2)
+void implicitlyUntuplePatternIfApplicable(TypeChecker *TC,
+                                          Pattern *&enumElementInnerPat,
+                                          Type enumPayloadType) {
+  if (auto *tupleType = dyn_cast<TupleType>(enumPayloadType.getPointer())) {
+    if (tupleType->getNumElements() >= 2
+        && enumElementInnerPat->getKind() == PatternKind::Paren) {
+      auto *semantic = enumElementInnerPat->getSemanticsProvidingPattern();
+      if (auto *tuplePattern = dyn_cast<TuplePattern>(semantic)) {
+        if (tuplePattern->getNumElements() >= 2) {
+          TC->diagnose(tuplePattern->getLoc(),
+                       diag::matching_tuple_pattern_with_many_assoc_values);
+          enumElementInnerPat = semantic;
+        }
+      } else {
+        TC->diagnose(enumElementInnerPat->getLoc(),
+                     diag::matching_pattern_with_many_assoc_values);
+      }
+    }
+  } else if (auto *tupleType = enumPayloadType->getAs<TupleType>()) {
+    if (tupleType->getNumElements() >= 2
+        && enumElementInnerPat->getKind() == PatternKind::Tuple)
+      TC->diagnose(enumElementInnerPat->getLoc(),
+                   diag::matching_many_patterns_with_tupled_assoc_value);
+  }
+}
+}
+
 /// Perform top-down type coercion on the given pattern.
 bool TypeChecker::coercePatternToType(Pattern *&P, TypeResolution resolution,
                                       Type type,
@@ -1513,6 +1553,9 @@ bool TypeChecker::coercePatternToType(Pattern *&P, TypeResolution resolution,
       auto newSubOptions = subOptions;
       newSubOptions.setContext(TypeResolverContext::EnumPatternPayload);
       newSubOptions |= TypeResolutionFlags::FromNonInferredPattern;
+
+      ::implicitlyUntuplePatternIfApplicable(this, sub, elementType);
+
       if (coercePatternToType(sub, resolution, elementType, newSubOptions))
         return true;
       EEP->setSubPattern(sub);

lib/Sema/TypeCheckSwitchStmt.cpp

@@ -47,10 +47,13 @@ namespace {
 
 namespace {
 
-  /// The SpaceEngine encapsulates an algorithm for computing the exhaustiveness
-  /// of a switch statement using an algebra of spaces described by Fengyun Liu
-  /// and an algorithm for computing warnings for pattern matching by
-  /// Luc Maranget.
+  /// The SpaceEngine encapsulates
+  ///
+  /// 1. An algorithm for computing the exhaustiveness of a switch statement
+  ///    using an algebra of spaces based on Fengyun Liu's
+  ///    "A Generic Algorithm for Checking Exhaustivity of Pattern Matching".
+  /// 2. An algorithm for computing warnings for pattern matching based on
+  ///    Luc Maranget's "Warnings for pattern matching".
   ///
   /// The main algorithm centers around the computation of the difference and
   /// the containment of the "Spaces" given in each case, which reduces the
@@ -298,7 +301,6 @@ namespace {
         }
 
         switch (PairSwitch(getKind(), other.getKind())) {
-        PAIRCASE (SpaceKind::Disjunct, SpaceKind::Empty):
         PAIRCASE (SpaceKind::Disjunct, SpaceKind::Type):
         PAIRCASE (SpaceKind::Disjunct, SpaceKind::Constructor):
         PAIRCASE (SpaceKind::Disjunct, SpaceKind::Disjunct):
@@ -735,6 +737,30 @@ namespace {
         }
       }
 
+      /// Use this if you're doing getAs<TupleType> on a Type (and it succeeds)
+      /// to compute the spaces for it. Handy for disambiguating fields
+      /// that are tuples from associated values.
+      ///
+      ///    .e((a: X, b: X)) -> ((a: X, b: X))
+      /// vs .f(a: X, b: X)   -> (a: X, b: X)
+      static void getTupleTypeSpaces(Type &outerType,
+                                     TupleType *tty,
+                                     SmallVectorImpl<Space> &spaces) {
+        ArrayRef<TupleTypeElt> ttyElts = tty->getElements();
+        if (isa<ParenType>(outerType.getPointer())) {
+          // We had an actual tuple!
+          SmallVector<Space, 4> innerSpaces;
+          for (auto &elt: ttyElts)
+            innerSpaces.push_back(Space::forType(elt.getType(), elt.getName()));
+          spaces.push_back(
+            Space::forConstructor(tty, Identifier(), innerSpaces));
+        } else {
+          // We're looking at the fields of a constructor here.
+          for (auto &elt: ttyElts)
+            spaces.push_back(Space::forType(elt.getType(), elt.getName()));
+        }
+      };
+
       // Decompose a type into its component spaces.
       static void decompose(TypeChecker &TC, const DeclContext *DC, Type tp,
                             SmallVectorImpl<Space> &arr) {
@@ -748,8 +774,6 @@ namespace {
           auto children = E->getAllElements();
           std::transform(children.begin(), children.end(),
                          std::back_inserter(arr), [&](EnumElementDecl *eed) {
-            SmallVector<Space, 4> constElemSpaces;
-
             // We need the interface type of this enum case but it may
             // not have been computed.
             if (!eed->hasInterfaceType()) {
@@ -768,17 +792,15 @@ namespace {
               return Space();
             }
 
+            // .e(a: X, b: X)   -> (a: X, b: X)
+            // .f((a: X, b: X)) -> ((a: X, b: X)
             auto eedTy = tp->getCanonicalType()
                            ->getTypeOfMember(E->getModuleContext(), eed,
                                              eed->getArgumentInterfaceType());
+            SmallVector<Space, 4> constElemSpaces;
             if (eedTy) {
               if (auto *TTy = eedTy->getAs<TupleType>()) {
-                // Decompose the payload tuple into its component type spaces.
-                llvm::transform(TTy->getElements(),
-                                std::back_inserter(constElemSpaces),
-                                [&](TupleTypeElt elt) {
-                  return Space::forType(elt.getType(), elt.getName());
-                });
+                Space::getTupleTypeSpaces(eedTy, TTy, constElemSpaces);
               } else if (auto *TTy = dyn_cast<ParenType>(eedTy.getPointer())) {
                 constElemSpaces.push_back(
                     Space::forType(TTy->getUnderlyingType(), Identifier()));
@@ -1441,14 +1463,11 @@ namespace {
           // FIXME: SE-0155 makes this case unreachable.
           if (SP->getKind() == PatternKind::Named
               || SP->getKind() == PatternKind::Any) {
-            if (auto *TTy = SP->getType()->getAs<TupleType>()) {
-              for (auto ty : TTy->getElements()) {
-                conArgSpace.push_back(Space::forType(ty.getType(),
-                                                     ty.getName()));
-              }
-            } else {
+            Type outerType = SP->getType();
+            if (auto *TTy = outerType->getAs<TupleType>())
+              Space::getTupleTypeSpaces(outerType, TTy, conArgSpace);
+            else
               conArgSpace.push_back(projectPattern(TC, SP));
-            }
           } else if (SP->getKind() == PatternKind::Tuple) {
             Space argTupleSpace = projectPattern(TC, SP);
             // Tuples are modeled as if they are enums with a single, nameless
@@ -1458,9 +1477,7 @@ namespace {
             if (argTupleSpace.isEmpty())
               return Space();
             assert(argTupleSpace.getKind() == SpaceKind::Constructor);
-            conArgSpace.insert(conArgSpace.end(),
-                               argTupleSpace.getSpaces().begin(),
-                               argTupleSpace.getSpaces().end());
+            conArgSpace.push_back(argTupleSpace);
           } else {
             conArgSpace.push_back(projectPattern(TC, SP));
           }

test/Compatibility/implicit_tupling_untupling_codegen.swift

@@ -0,0 +1,149 @@
+// RUN: %target-run-simple-swift | %FileCheck %s
+
+// Even though we test that type-checking and exhaustiveness checking work fine
+// in the presence of implicit tupling/untupling in exhaustive_switch.swift,
+// make sure that the "patched" patterns do not lead to incorrect codegen.
+
+enum Untupled {
+  case upair(Int, Int)
+}
+
+let u_ex = Untupled.upair(1, 2)
+
+func sr11212_content_untupled_pattern_tupled1(u: Untupled) -> (Int, Int) {
+  switch u { case .upair((let x, let y)): return (x, y) }
+}
+print(sr11212_content_untupled_pattern_tupled1(u: u_ex))
+// CHECK: (1, 2)
+
+func sr11212_content_untupled_pattern_tupled2(u: Untupled) -> (Int, Int) {
+  switch u { case .upair(let (x, y)): return (x, y) }
+}
+print(sr11212_content_untupled_pattern_tupled2(u: u_ex))
+// CHECK: (1, 2)
+
+func sr11212_content_untupled_pattern_tupled3(u: Untupled) -> (Int, Int) {
+  switch u { case let .upair((x, y)): return (x, y) }
+}
+print(sr11212_content_untupled_pattern_tupled3(u: u_ex))
+// CHECK: (1, 2)
+
+func sr11212_content_untupled_pattern_untupled1(u: Untupled) -> (Int, Int) {
+  switch u { case .upair(let x, let y): return (x, y) }
+}
+print(sr11212_content_untupled_pattern_untupled1(u: u_ex))
+// CHECK: (1, 2)
+
+func sr11212_content_untupled_pattern_untupled2(u: Untupled) -> (Int, Int) {
+    switch u { case let .upair(x, y): return (x, y) }
+}
+print(sr11212_content_untupled_pattern_untupled2(u: u_ex))
+// CHECK: (1, 2)
+
+func sr11212_content_untupled_pattern_ambiguous1(u: Untupled) -> (Int, Int) {
+  switch u { case .upair(let u_): return u_ }
+}
+print(sr11212_content_untupled_pattern_ambiguous1(u: u_ex))
+// CHECK: (1, 2)
+
+func sr11212_content_untupled_pattern_ambiguous2(u: Untupled) -> (Int, Int) {
+  switch u { case let .upair(u_): return u_ }
+}
+print(sr11212_content_untupled_pattern_ambiguous2(u: u_ex))
+// CHECK: (1, 2)
+
+enum Tupled {
+  case tpair((Int, Int))
+}
+
+let t_ex = Tupled.tpair((1, 2))
+
+func sr11212_content_tupled_pattern_tupled1(t: Tupled) -> (Int, Int) {
+  switch t { case .tpair((let x, let y)): return (x, y) }
+}
+print(sr11212_content_tupled_pattern_tupled1(t: t_ex))
+// CHECK: (1, 2)
+
+func sr11212_content_tupled_pattern_tupled2(t: Tupled) -> (Int, Int) {
+  switch t { case .tpair(let (x, y)): return (x, y) }
+}
+print(sr11212_content_tupled_pattern_tupled2(t: t_ex))
+// CHECK: (1, 2)
+
+func sr11212_content_tupled_pattern_tupled3(t: Tupled) -> (Int, Int) {
+  switch t { case let .tpair((x, y)): return (x, y) }
+}
+print(sr11212_content_tupled_pattern_tupled3(t: t_ex))
+// CHECK: (1, 2)
+
+func sr11212_content_tupled_pattern_untupled1(t: Tupled) -> (Int, Int) {
+  switch t { case .tpair(let x, let y): return (x, y) }
+}
+print(sr11212_content_tupled_pattern_untupled1(t: t_ex))
+// CHECK: (1, 2)
+
+func sr11212_content_tupled_pattern_untupled2(t: Tupled) -> (Int, Int) {
+  switch t { case let .tpair(x, y): return (x, y) }
+}
+print(sr11212_content_tupled_pattern_untupled2(t: t_ex))
+// CHECK: (1, 2)
+
+func sr11212_content_tupled_pattern_ambiguous1(t: Tupled) -> (Int, Int) {
+  switch t { case .tpair(let t_): return t_ }
+}
+print(sr11212_content_tupled_pattern_ambiguous1(t: t_ex))
+// CHECK: (1, 2)
+
+func sr11212_content_tupled_pattern_ambiguous2(t: Tupled) -> (Int, Int) {
+  switch t { case let .tpair(t_): return t_ }
+}
+print(sr11212_content_tupled_pattern_ambiguous2(t: t_ex))
+// CHECK: (1, 2)
+
+enum Box<T> {
+  case box(T)
+}
+
+let b_ex : Box<(Int, Int)> = Box.box((1, 2))
+
+func sr11212_content_generic_pattern_tupled1(b: Box<(Int, Int)>) -> (Int, Int) {
+  switch b { case .box((let x, let y)): return (x, y) }
+}
+print(sr11212_content_generic_pattern_tupled1(b: b_ex))
+// CHECK: (1, 2)
+
+func sr11212_content_generic_pattern_tupled2(b: Box<(Int, Int)>) -> (Int, Int) {
+  switch b { case .box(let (x, y)): return (x, y) }
+}
+print(sr11212_content_generic_pattern_tupled2(b: b_ex))
+// CHECK: (1, 2)
+
+func sr11212_content_generic_pattern_tupled3(b: Box<(Int, Int)>) -> (Int, Int) {
+ switch b { case let .box((x, y)): return (x, y) }
+}
+print(sr11212_content_generic_pattern_tupled3(b: b_ex))
+// CHECK: (1, 2)
+
+func sr11212_content_generic_pattern_untupled1(b: Box<(Int, Int)>) -> (Int, Int) {
+  switch b { case .box(let x, let y): return (x, y) }
+}
+print(sr11212_content_generic_pattern_untupled1(b: b_ex))
+// CHECK: (1, 2)
+
+func sr11212_content_generic_pattern_untupled2(b: Box<(Int, Int)>) -> (Int, Int) {
+  switch b { case let .box(x, y): return (x, y) }
+}
+print(sr11212_content_generic_pattern_untupled2(b: b_ex))
+// CHECK: (1, 2)
+
+func sr11212_content_generic_pattern_ambiguous1(b: Box<(Int, Int)>) -> (Int, Int) {
+  switch b { case .box(let b_): return b_ }
+}
+print(sr11212_content_generic_pattern_ambiguous1(b: b_ex))
+// CHECK: (1, 2)
+
+func sr11212_content_generic_pattern_ambiguous2(b: Box<(Int, Int)>) -> (Int, Int) {
+  switch b { case let .box(b_): return b_ }
+}
+print(sr11212_content_generic_pattern_ambiguous2(b: b_ex))
+// CHECK: (1, 2)

test/Parse/matching_patterns.swift

@@ -100,7 +100,7 @@ enum Voluntary<T> : Equatable {
       ()
 
     case .Twain(), // expected-error{{tuple pattern has the wrong length for tuple type '(T, T)'}}
-         .Twain(_),
+         .Twain(_), // expected-warning {{cannot match several associated values at once, implicitly tupling the associated values and trying to match that instead}}
          .Twain(_, _),
          .Twain(_, _, _): // expected-error{{tuple pattern has the wrong length for tuple type '(T, T)'}}
       ()
@@ -143,7 +143,7 @@ case Voluntary<Int>.Mere,
      .Mere(_):
   ()
 case .Twain,
-     .Twain(_),
+     .Twain(_), // expected-warning {{cannot match several associated values at once, implicitly tupling the associated values and trying to match that instead}}
      .Twain(_, _),
      .Twain(_, _, _): // expected-error{{tuple pattern has the wrong length for tuple type '(Int, Int)'}}
   ()