Merge pull request #64526 from hborla/invalid-repeat

[ConstraintSystem] Diagnose pack expansion expressions in non-variadic contexts.

Author: hborla

include/swift/AST/DiagnosticsSema.def

@@ -5316,6 +5316,12 @@ ERROR(vararg_not_allowed,none,
 ERROR(expansion_not_allowed,none,
       "pack expansion %0 can only appear in a function parameter list, "
       "tuple element, or generic argument list", (Type))
+ERROR(expansion_expr_not_allowed,none,
+      "value pack expansion can only appear inside a function argument list "
+      "or tuple element", ())
+ERROR(invalid_expansion_argument,none,
+      "cannot pass value pack expansion to non-pack parameter of type %0",
+      (Type))
 ERROR(expansion_not_variadic,none,
       "pack expansion %0 must contain at least one pack reference", (Type))
 ERROR(pack_reference_outside_expansion,none,

include/swift/Sema/CSFix.h

@@ -431,6 +431,9 @@ enum class FixKind : uint8_t {
 
   /// Allow pack references outside of pack expansions.
   AllowInvalidPackReference,
+
+  /// Allow pack expansion expressions in a context that does not support them.
+  AllowInvalidPackExpansion,
 };
 
 class ConstraintFix {
@@ -2128,6 +2131,26 @@ class AllowInvalidPackReference final : public ConstraintFix {
   }
 };
 
+class AllowInvalidPackExpansion final : public ConstraintFix {
+  AllowInvalidPackExpansion(ConstraintSystem &cs,
+                            ConstraintLocator *locator)
+      : ConstraintFix(cs, FixKind::AllowInvalidPackExpansion, locator) {}
+
+public:
+  std::string getName() const override {
+    return "allow invalid pack expansion";
+  }
+
+  bool diagnose(const Solution &solution, bool asNote = false) const override;
+
+  static AllowInvalidPackExpansion *create(ConstraintSystem &cs,
+                                           ConstraintLocator *locator);
+
+  static bool classof(const ConstraintFix *fix) {
+    return fix->getKind() == FixKind::AllowInvalidPackExpansion;
+  }
+};
+
 class CollectionElementContextualMismatch final
     : public ContextualMismatch,
       private llvm::TrailingObjects<CollectionElementContextualMismatch,

lib/Sema/CSDiagnostics.cpp

@@ -6176,6 +6176,20 @@ bool InvalidPackReference::diagnoseAsError() {
   return true;
 }
 
+bool InvalidPackExpansion::diagnoseAsError() {
+  auto *locator = getLocator();
+  if (locator->isLastElement<LocatorPathElt::ApplyArgToParam>()) {
+    if (auto argInfo = getFunctionArgApplyInfo(locator)) {
+      emitDiagnostic(diag::invalid_expansion_argument,
+                     argInfo->getParamInterfaceType());
+      return true;
+    }
+  }
+
+  emitDiagnostic(diag::expansion_expr_not_allowed);
+  return true;
+}
+
 bool CollectionElementContextualFailure::diagnoseAsError() {
   auto anchor = getRawAnchor();
   auto *locator = getLocator();

lib/Sema/CSDiagnostics.h

@@ -1875,6 +1875,17 @@ class InvalidPackReference final : public FailureDiagnostic {
   bool diagnoseAsError() override;
 };
 
+/// Diagnose pack expansion expressions appearing in contexts that do not
+/// accept a comma-separated list of values.
+class InvalidPackExpansion final : public FailureDiagnostic {
+public:
+  InvalidPackExpansion(const Solution &solution,
+                       ConstraintLocator *locator)
+      : FailureDiagnostic(solution, locator) {}
+
+  bool diagnoseAsError() override;
+};
+
 /// Diagnose a contextual mismatch between expected collection element type
 /// and the one provided (e.g. source of the assignment or argument to a call)
 /// e.g.:

lib/Sema/CSFix.cpp

@@ -1429,6 +1429,18 @@ AllowInvalidPackReference::create(ConstraintSystem &cs, Type packType,
       AllowInvalidPackReference(cs, packType, locator);
 }
 
+bool AllowInvalidPackExpansion::diagnose(const Solution &solution,
+                                         bool asNote) const {
+  InvalidPackExpansion failure(solution, getLocator());
+  return failure.diagnose(asNote);
+}
+
+AllowInvalidPackExpansion *
+AllowInvalidPackExpansion::create(ConstraintSystem &cs,
+                                  ConstraintLocator *locator) {
+  return new (cs.getAllocator()) AllowInvalidPackExpansion(cs, locator);
+}
+
 bool CollectionElementContextualMismatch::diagnose(const Solution &solution,
                                                    bool asNote) const {
   CollectionElementContextualFailure failure(

lib/Sema/CSSimplify.cpp

@@ -4311,6 +4311,23 @@ ConstraintSystem::matchTypesBindTypeVar(
     return getTypeMatchFailure(locator);
   }
 
+  // Binding to a pack expansion type is always an error. This indicates
+  // that a pack expansion expression was used in a context that doesn't
+  // support it.
+  if (type->is<PackExpansionType>()) {
+    if (!shouldAttemptFixes())
+      return getTypeMatchFailure(locator);
+
+    auto *fix =
+        AllowInvalidPackExpansion::create(*this, getConstraintLocator(locator));
+    if (recordFix(fix))
+      return getTypeMatchFailure(locator);
+
+    // Don't allow the pack expansion type to propagate to other
+    // bindings.
+    type = PlaceholderType::get(typeVar->getASTContext(), typeVar);
+  }
+
   // We do not allow keypaths to go through AnyObject. Let's create a fix
   // so this can be diagnosed later.
   if (auto loc = typeVar->getImpl().getLocator()) {
@@ -7154,6 +7171,22 @@ ConstraintSystem::matchTypes(Type type1, Type type2, ConstraintKind kind,
     }
   }
 
+  // Matching types where one side is a pack expansion and the other is not
+  // means a pack expansion was used where it isn't supported.
+  if (type1->is<PackExpansionType>() != type2->is<PackExpansionType>()) {
+    if (!shouldAttemptFixes())
+      return getTypeMatchFailure(locator);
+
+    if (type1->isPlaceholder() || type2->isPlaceholder())
+      return getTypeMatchSuccess();
+
+    auto *loc = getConstraintLocator(locator);
+    if (recordFix(AllowInvalidPackExpansion::create(*this, loc)))
+      return getTypeMatchFailure(locator);
+
+    return getTypeMatchSuccess();
+  }
+
   if (kind >= ConstraintKind::Conversion) {
     // An lvalue of type T1 can be converted to a value of type T2 so long as
     // T1 is convertible to T2 (by loading the value).  Note that we cannot get
@@ -14171,6 +14204,7 @@ ConstraintSystem::SolutionKind ConstraintSystem::simplifyFixConstraint(
   case FixKind::MustBeCopyable:
   case FixKind::AllowInvalidPackElement:
   case FixKind::AllowInvalidPackReference:
+  case FixKind::AllowInvalidPackExpansion:
   case FixKind::MacroMissingPound:
   case FixKind::AllowGlobalActorMismatch:
   case FixKind::GenericArgumentsMismatch: {

test/Constraints/pack-expansion-expressions.swift

@@ -39,6 +39,7 @@ func coerceExpansion<each T>(_ value: repeat each T) {
 func localValuePack<each T>(_ t: repeat each T) -> (repeat each T, repeat each T) {
   let local = repeat each t
   let localAnnotated: repeat each T = repeat each t
+  // expected-error@-1{{value pack expansion can only appear inside a function argument list or tuple element}}
 
   return (repeat each local, repeat each localAnnotated)
 }
@@ -52,14 +53,14 @@ protocol P {
 }
 
 func outerArchetype<each T, U>(t: repeat each T, u: U) where each T: P {
-  let _: repeat (each T.A, U) = repeat ((each t).value, u)
+  let _: (repeat (each T.A, U)) = (repeat ((each t).value, u))
 }
 
 func sameElement<each T, U>(t: repeat each T, u: U) where each T: P, each T == U {
 // expected-error@-1{{same-element requirements are not yet supported}}
 
   // FIXME: Opened element archetypes in diagnostics
-  let _: repeat each T = repeat (each t).f(u)
+  let _: (repeat each T) = (repeat (each t).f(u))
   // expected-error@-1 {{cannot convert value of type 'U' to expected argument type 'τ_1_0'}}
 }
 
@@ -68,23 +69,23 @@ func forEachEach<each C, U>(c: repeat each C, function: (U) -> Void)
     // expected-error@-1{{same-element requirements are not yet supported}}
 
   // FIXME: Opened element archetypes in diagnostics
-  _ = repeat (each c).forEach(function)
+  _ = (repeat (each c).forEach(function))
   // expected-error@-1 {{cannot convert value of type '(U) -> Void' to expected argument type '(τ_1_0.Element) throws -> Void'}}
 }
 
 func typeReprPacks<each T>(_ t: repeat each T) where each T: ExpressibleByIntegerLiteral {
-  _ = repeat Array<each T>()
-  _ = repeat 1 as each T
+  _ = (repeat Array<each T>())
+  _ = (repeat 1 as each T)
 
   _ = Array<each T>() // expected-error {{pack reference 'T' requires expansion using keyword 'repeat'}}
   _ = 1 as each T // expected-error {{pack reference 'T' requires expansion using keyword 'repeat'}}
   repeat Invalid<String, each T>("") // expected-error {{cannot find 'Invalid' in scope}}
 }
 
 func sameShapeDiagnostics<each T, each U>(t: repeat each T, u: repeat each U) {
-  _ = repeat (each t, each u) // expected-error {{pack expansion requires that 'U' and 'T' have the same shape}}
-  _ = repeat Array<(each T, each U)>() // expected-error {{pack expansion requires that 'U' and 'T' have the same shape}}
-  _ = repeat (Array<each T>(), each u) // expected-error {{pack expansion requires that 'U' and 'T' have the same shape}}
+  _ = (repeat (each t, each u)) // expected-error {{pack expansion requires that 'U' and 'T' have the same shape}}
+  _ = (repeat Array<(each T, each U)>()) // expected-error {{pack expansion requires that 'U' and 'T' have the same shape}}
+  _ = (repeat (Array<each T>(), each u)) // expected-error {{pack expansion requires that 'U' and 'T' have the same shape}}
 }
 
 func returnPackExpansionType<each T>(_ t: repeat each T) -> repeat each T { // expected-error {{pack expansion 'repeat each T' can only appear in a function parameter list, tuple element, or generic argument list}}
@@ -240,3 +241,23 @@ func packOutsideExpansion<each T>(_ t: repeat each T) {
   _ = each tuple.element
   // expected-error@-1{{pack reference 'T' can only appear in pack expansion}}
 }
+
+func identity<T>(_ t: T) -> T { t }
+func concrete(_: Int) {}
+
+func invalidRepeat<each T>(t: repeat each T) {
+  _ = repeat each t
+  // expected-error@-1 {{value pack expansion can only appear inside a function argument list or tuple element}}
+
+  let _: Int = repeat each t
+  // expected-error@-1 {{value pack expansion can only appear inside a function argument list or tuple element}}
+
+  identity(identity(repeat each t))
+  // expected-error@-1 {{cannot pass value pack expansion to non-pack parameter of type 'T'}}
+
+  concrete(repeat each t)
+  // expected-error@-1 {{cannot pass value pack expansion to non-pack parameter of type 'Int'}}
+
+  _ = [repeat each t]
+  // expected-error@-1 {{value pack expansion can only appear inside a function argument list or tuple element}}
+}

test/type/pack_expansion.swift

@@ -93,5 +93,5 @@ func packRefOutsideExpansion<each T>(_: each T.Type) {}
 // coverage to ensure a 'repeat each' type is considered Copyable
 func golden<Z>(_ z: Z) {}
 func hour<each T>(_ t: repeat each T)  {
-  _ = repeat golden(each t)
+  _ = (repeat golden(each t))
 }