Merge pull request #77203 from hamishknight/wrapping-paper-sema

hamishknight · web-flow · commit f4bea5f7c0a7 · 2024-10-26T21:25:17.000+01:00
[Sema] Avoid relying on ParenType in a couple of places
diff --git a/include/swift/AST/Decl.h b/include/swift/AST/Decl.h
@@ -8547,6 +8547,10 @@ class EnumElementDecl : public DeclContext, public ValueDecl {
   /// associated values, or null if there are no associated values.
   Type getPayloadInterfaceType() const;
 
+  /// Retrieve the parameters of the implicit case constructor for the enum
+  /// element.
+  ArrayRef<AnyFunctionType::Param> getCaseConstructorParams() const;
+
   void setParameterList(ParameterList *params);
   ParameterList *getParameterList() const { return Params; }
 
diff --git a/lib/AST/Decl.cpp b/lib/AST/Decl.cpp
@@ -10733,22 +10733,32 @@ SourceRange EnumElementDecl::getSourceRange() const {
   return {getStartLoc(), getNameLoc()};
 }
 
-Type EnumElementDecl::getPayloadInterfaceType() const {
+ArrayRef<AnyFunctionType::Param>
+EnumElementDecl::getCaseConstructorParams() const {
   if (!hasAssociatedValues())
-    return nullptr;
+    return {};
 
   auto interfaceType = getInterfaceType();
-  if (interfaceType->is<ErrorType>()) {
-    return interfaceType;
-  }
+  if (interfaceType->is<ErrorType>())
+    return {};
 
   auto funcTy = interfaceType->castTo<AnyFunctionType>();
-  funcTy = funcTy->getResult()->castTo<FunctionType>();
+  return funcTy->getResult()->castTo<FunctionType>()->getParams();
+}
+
+Type EnumElementDecl::getPayloadInterfaceType() const {
+  if (!hasAssociatedValues())
+    return Type();
+
+  auto interfaceType = getInterfaceType();
+  if (interfaceType->is<ErrorType>())
+    return interfaceType;
 
   // The payload type of an enum is an imploded tuple of the internal arguments
   // of the case constructor. As such, compose a tuple type with the parameter
   // flags dropped.
-  return AnyFunctionType::composeTuple(getASTContext(), funcTy->getParams(),
+  return AnyFunctionType::composeTuple(getASTContext(),
+                                       getCaseConstructorParams(),
                                        ParameterFlagHandling::IgnoreNonEmpty);
 }
 
diff --git a/lib/Sema/TypeCheckPattern.cpp b/lib/Sema/TypeCheckPattern.cpp
@@ -942,64 +942,72 @@ void repairTupleOrAssociatedValuePatternIfApplicable(
     Type enumPayloadType,
     const EnumElementDecl *enumCase) {
   auto &DE = Ctx.Diags;
-  bool addDeclNote = false;
-  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) {
-          auto diag = DE.diagnose(tuplePattern->getLoc(),
-              diag::converting_tuple_into_several_associated_values,
-              enumCase->getNameStr(), tupleType->getNumElements());
-          auto subPattern =
-            dyn_cast<ParenPattern>(enumElementInnerPat)->getSubPattern();
-
-          // We might also have code like
-          //
-          // enum Upair { case upair(Int, Int) }
-          // func f(u: Upair) { switch u { case .upair(let (x, y)): () } }
-          //
-          // This needs a more complex rearrangement to fix the code. So only
-          // apply the fix-it if we have a tuple immediately inside.
-          if (subPattern->getKind() == PatternKind::Tuple) {
-            auto leadingParen = SourceRange(enumElementInnerPat->getStartLoc());
-            auto trailingParen = SourceRange(enumElementInnerPat->getEndLoc());
-            diag.fixItRemove(leadingParen)
-                .fixItRemove(trailingParen);
-          }
-
-          addDeclNote = true;
-          enumElementInnerPat = semantic;
-        }
-      } else {
-        DE.diagnose(enumElementInnerPat->getLoc(),
-          diag::found_one_pattern_for_several_associated_values,
-          enumCase->getNameStr(),
-          tupleType->getNumElements());
-        addDeclNote = true;
-      }
-    }
-  } else if (auto *tupleType = enumPayloadType->getAs<TupleType>()) {
-    if (tupleType->getNumElements() >= 2) {
-      if (auto *tuplePattern = dyn_cast<TuplePattern>(enumElementInnerPat)) {
-        DE.diagnose(enumElementInnerPat->getLoc(),
-                    diag::converting_several_associated_values_into_tuple,
-                    enumCase->getNameStr(),
-                    tupleType->getNumElements())
-          .fixItInsert(enumElementInnerPat->getStartLoc(), "(")
-          .fixItInsertAfter(enumElementInnerPat->getEndLoc(), ")");
-        addDeclNote = true;
-        enumElementInnerPat =
-          new (Ctx) ParenPattern(enumElementInnerPat->getStartLoc(),
-                                 enumElementInnerPat,
-                                 enumElementInnerPat->getEndLoc());
+  auto addDeclNote = [&]() {
+    DE.diagnose(enumCase->getStartLoc(), diag::decl_declared_here, enumCase);
+  };
+  auto payloadParams = enumCase->getCaseConstructorParams();
+
+  // First check to see whether we need to untuple a pattern.
+  if (payloadParams.size() >= 2) {
+    if (enumElementInnerPat->getKind() != PatternKind::Paren)
+      return;
+
+    auto *semantic = enumElementInnerPat->getSemanticsProvidingPattern();
+    if (auto *tuplePattern = dyn_cast<TuplePattern>(semantic)) {
+      if (tuplePattern->getNumElements() < 2)
+        return;
+
+      auto diag =
+          DE.diagnose(tuplePattern->getLoc(),
+                      diag::converting_tuple_into_several_associated_values,
+                      enumCase->getNameStr(), payloadParams.size());
+      auto subPattern =
+          dyn_cast<ParenPattern>(enumElementInnerPat)->getSubPattern();
+
+      // We might also have code like
+      //
+      // enum Upair { case upair(Int, Int) }
+      // func f(u: Upair) { switch u { case .upair(let (x, y)): () } }
+      //
+      // This needs a more complex rearrangement to fix the code. So only
+      // apply the fix-it if we have a tuple immediately inside.
+      if (subPattern->getKind() == PatternKind::Tuple) {
+        auto leadingParen = SourceRange(enumElementInnerPat->getStartLoc());
+        auto trailingParen = SourceRange(enumElementInnerPat->getEndLoc());
+        diag.fixItRemove(leadingParen).fixItRemove(trailingParen);
       }
+      diag.flush();
+      addDeclNote();
+      enumElementInnerPat = semantic;
+    } else {
+      DE.diagnose(enumElementInnerPat->getLoc(),
+                  diag::found_one_pattern_for_several_associated_values,
+                  enumCase->getNameStr(), payloadParams.size());
+      addDeclNote();
     }
+    return;
   }
 
-  if (addDeclNote)
-    DE.diagnose(enumCase->getStartLoc(), diag::decl_declared_here, enumCase);
+  // Then check to see whether we need to tuple a pattern.
+  if (payloadParams.size() == 1 && !payloadParams[0].hasLabel()) {
+    auto *tupleType = enumPayloadType->getAs<TupleType>();
+    if (!tupleType || tupleType->getNumElements() < 2)
+      return;
+
+    auto *tuplePattern = dyn_cast<TuplePattern>(enumElementInnerPat);
+    if (!tuplePattern)
+      return;
+
+    DE.diagnose(enumElementInnerPat->getLoc(),
+                diag::converting_several_associated_values_into_tuple,
+                enumCase->getNameStr(), tupleType->getNumElements())
+        .fixItInsert(enumElementInnerPat->getStartLoc(), "(")
+        .fixItInsertAfter(enumElementInnerPat->getEndLoc(), ")");
+    addDeclNote();
+    enumElementInnerPat = new (Ctx)
+        ParenPattern(enumElementInnerPat->getStartLoc(), enumElementInnerPat,
+                     enumElementInnerPat->getEndLoc());
+  }
 }
 
 NullablePtr<Pattern> TypeChecker::trySimplifyExprPattern(ExprPattern *EP,
diff --git a/lib/Sema/TypeCheckSwitchStmt.cpp b/lib/Sema/TypeCheckSwitchStmt.cpp
@@ -785,28 +785,11 @@ 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,
+      /// Splat a tuple type into a set of element type spaces.
+      static void getTupleTypeSpaces(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()));
-        }
+        for (auto &elt : tty->getElements())
+          spaces.push_back(Space::forType(elt.getType(), elt.getName()));
       };
 
       // Decompose a type into its component spaces, ignoring any enum
@@ -843,19 +826,29 @@ namespace {
                   return Space();
                 }
 
-                // .e(a: X, b: X)   -> (a: X, b: X)
-                // .f((a: X, b: X)) -> ((a: X, b: X)
                 SmallVector<Space, 4> constElemSpaces;
-                if (auto payloadTy = eed->getPayloadInterfaceType()) {
-                  auto eedTy = tp->getCanonicalType()->getTypeOfMember(
-                      eed, payloadTy);
-                  if (auto *TTy = eedTy->getAs<TupleType>()) {
-                    Space::getTupleTypeSpaces(eedTy, TTy, constElemSpaces);
-                  } else if (auto *TTy =
-                                 dyn_cast<ParenType>(eedTy.getPointer())) {
-                    constElemSpaces.push_back(
-                        Space::forType(TTy->getUnderlyingType(), Identifier()));
+                auto params = eed->getCaseConstructorParams();
+                auto isParenLike = params.size() == 1 && !params[0].hasLabel();
+
+                // .e(a: X, b: X)   -> (a: X, b: X)
+                // .f((a: X, b: X)) -> ((a: X, b: X))
+                for (auto &param : params) {
+                  auto payloadTy = tp->getCanonicalType()->getTypeOfMember(
+                      eed, param.getParameterType());
+                  // A single tuple payload gets splatted into a constructor
+                  // space of its constituent elements. This allows the
+                  // deprecated ability to match using a .x(a, b, c) pattern
+                  // instead of a .x((a, b, c)) pattern.
+                  auto *tupleTy = payloadTy->getAs<TupleType>();
+                  if (tupleTy && isParenLike) {
+                    SmallVector<Space, 4> innerSpaces;
+                    Space::getTupleTypeSpaces(tupleTy, innerSpaces);
+                    constElemSpaces.push_back(Space::forConstructor(
+                        tupleTy, Identifier(), innerSpaces));
+                    continue;
                   }
+                  constElemSpaces.push_back(
+                      Space::forType(payloadTy, param.getLabel()));
                 }
                 return Space::forConstructor(tp, eed->getName(),
                                              constElemSpaces);
@@ -870,11 +863,8 @@ namespace {
         } else if (auto *TTy = tp->castTo<TupleType>()) {
           // Decompose each of the elements into its component type space.
           SmallVector<Space, 4> constElemSpaces;
-          llvm::transform(TTy->getElements(),
-                          std::back_inserter(constElemSpaces),
-                          [&](TupleTypeElt elt) {
-            return Space::forType(elt.getType(), elt.getName());
-          });
+          Space::getTupleTypeSpaces(TTy, constElemSpaces);
+
           // Create an empty constructor head for the tuple space.
           arr.push_back(Space::forConstructor(tp, Identifier(),
                                               constElemSpaces));
@@ -1519,12 +1509,12 @@ namespace {
           // matched by a single var pattern.  Project it like the tuple it
           // really is.
           //
-          // FIXME: SE-0155 makes this case unreachable.
+          // FIXME: SE-0155 will eventually this case unreachable. For now it's
+          // permitted as a deprecated behavior.
           if (SP->getKind() == PatternKind::Named
               || SP->getKind() == PatternKind::Any) {
-            Type outerType = SP->getType();
-            if (auto *TTy = outerType->getAs<TupleType>())
-              Space::getTupleTypeSpaces(outerType, TTy, conArgSpace);
+            if (auto *TTy = SP->getType()->getAs<TupleType>())
+              Space::getTupleTypeSpaces(TTy, conArgSpace);
             else
               conArgSpace.push_back(projectPattern(SP));
           } else if (SP->getKind() == PatternKind::Tuple) {
