Merge pull request #64498 from hborla/diagnose-pack-outside-expansion-expr

[ConstraintSystem] Enforce `TVO_CanBindToPack`, and diagnose pack references outside of pack expansion expressions.

Author: hborla

include/swift/AST/DiagnosticsSema.def

@@ -5319,8 +5319,9 @@ ERROR(expansion_not_allowed,none,
 ERROR(expansion_not_variadic,none,
       "pack expansion %0 must contain at least one pack reference", (Type))
 ERROR(pack_reference_outside_expansion,none,
-      "pack reference %0 can only appear in pack expansion or generic requirement",
-      (Type))
+      "pack reference %0 can only appear in pack expansion "
+      "%select{or generic requirement|}1",
+      (Type, /*inExpression*/ bool))
 ERROR(each_non_pack,none,
       "'each' cannot be applied to non-pack type %0",
       (Type))

include/swift/Sema/CSFix.h

@@ -428,6 +428,9 @@ enum class FixKind : uint8_t {
 
   /// Allow 'each' applied to a non-pack type.
   AllowInvalidPackElement,
+
+  /// Allow pack references outside of pack expansions.
+  AllowInvalidPackReference,
 };
 
 class ConstraintFix {
@@ -2099,6 +2102,30 @@ class AllowInvalidPackElement final : public ConstraintFix {
   }
 };
 
+class AllowInvalidPackReference final : public ConstraintFix {
+  Type packType;
+
+  AllowInvalidPackReference(ConstraintSystem &cs, Type packType,
+                            ConstraintLocator *locator)
+      : ConstraintFix(cs, FixKind::AllowInvalidPackReference, locator),
+        packType(packType) {}
+
+public:
+  std::string getName() const override {
+    return "allow pack outside pack expansion";
+  }
+
+  bool diagnose(const Solution &solution, bool asNote = false) const override;
+
+  static AllowInvalidPackReference *create(ConstraintSystem &cs,
+                                           Type packType,
+                                           ConstraintLocator *locator);
+
+  static bool classof(const ConstraintFix *fix) {
+    return fix->getKind() == FixKind::AllowInvalidPackReference;
+  }
+};
+
 class CollectionElementContextualMismatch final
     : public ContextualMismatch,
       private llvm::TrailingObjects<CollectionElementContextualMismatch,

lib/Sema/CSDiagnostics.cpp

@@ -6170,6 +6170,12 @@ bool InvalidPackElement::diagnoseAsError() {
   return true;
 }
 
+bool InvalidPackReference::diagnoseAsError() {
+  emitDiagnostic(diag::pack_reference_outside_expansion,
+                 packType, /*inExpression*/true);
+  return true;
+}
+
 bool CollectionElementContextualFailure::diagnoseAsError() {
   auto anchor = getRawAnchor();
   auto *locator = getLocator();

lib/Sema/CSDiagnostics.h

@@ -1862,6 +1862,19 @@ class InvalidPackElement final : public FailureDiagnostic {
   bool diagnoseAsError() override;
 };
 
+/// Diagnose pack references outside of pack expansion expressions.
+class InvalidPackReference final : public FailureDiagnostic {
+  Type packType;
+
+public:
+  InvalidPackReference(const Solution &solution, Type packType,
+                       ConstraintLocator *locator)
+      : FailureDiagnostic(solution, locator),
+        packType(packType) {}
+
+  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

@@ -1401,6 +1401,19 @@ AllowInvalidPackElement::create(ConstraintSystem &cs,
       AllowInvalidPackElement(cs, packElementType, locator);
 }
 
+bool AllowInvalidPackReference::diagnose(const Solution &solution,
+                                         bool asNote) const {
+  InvalidPackReference failure(solution, packType, getLocator());
+  return failure.diagnose(asNote);
+}
+
+AllowInvalidPackReference *
+AllowInvalidPackReference::create(ConstraintSystem &cs, Type packType,
+                                  ConstraintLocator *locator) {
+  return new (cs.getAllocator())
+      AllowInvalidPackReference(cs, packType, locator);
+}
+
 bool CollectionElementContextualMismatch::diagnose(const Solution &solution,
                                                    bool asNote) const {
   CollectionElementContextualFailure failure(

lib/Sema/CSGen.cpp

@@ -923,9 +923,14 @@ namespace {
             CS.getConstraintLocator(expr, ConstraintLocator::Member),
             TVO_CanBindToHole);
       }
+      unsigned options = (TVO_CanBindToLValue |
+                          TVO_CanBindToNoEscape);
+      if (!PackElementEnvironments.empty())
+        options |= TVO_CanBindToPack;
+
       auto tv = CS.createTypeVariable(
                   CS.getConstraintLocator(expr, ConstraintLocator::Member),
-                  TVO_CanBindToLValue | TVO_CanBindToNoEscape);
+                  options);
       SmallVector<OverloadChoice, 4> outerChoices;
       for (auto decl : outerAlternatives) {
         outerChoices.push_back(OverloadChoice(Type(), decl, functionRefKind));
@@ -1419,11 +1424,14 @@ namespace {
         return invalidateReference();
       }
 
+      unsigned options = (TVO_CanBindToLValue |
+                          TVO_CanBindToNoEscape);
+      if (!PackElementEnvironments.empty())
+        options |= TVO_CanBindToPack;
+
       // Create an overload choice referencing this declaration and immediately
       // resolve it. This records the overload for use later.
-      auto tv = CS.createTypeVariable(locator,
-                                      TVO_CanBindToLValue |
-                                      TVO_CanBindToNoEscape);
+      auto tv = CS.createTypeVariable(locator, options);
 
       OverloadChoice choice =
           OverloadChoice(Type(), E->getDecl(), E->getFunctionRefKind());
@@ -3097,8 +3105,15 @@ namespace {
     Type visitPackElementExpr(PackElementExpr *expr) {
       auto packType = CS.getType(expr->getPackRefExpr());
 
-      if (PackElementEnvironments.empty())
-        return Type();
+      // If 'each t' is written outside of a pack expansion expression, allow the
+      // type to bind to a hole. The invalid pack reference will be diagnosed when
+      // attempting to bind the type variable for the underlying pack reference to
+      // a pack type without TVO_CanBindToPack.
+      if (PackElementEnvironments.empty()) {
+        return CS.createTypeVariable(CS.getConstraintLocator(expr),
+                                     TVO_CanBindToHole |
+                                     TVO_CanBindToNoEscape);
+      }
 
       // The type of a PackElementExpr is the opened pack element archetype
       // of the pack reference.

lib/Sema/CSSimplify.cpp

@@ -4294,6 +4294,23 @@ ConstraintSystem::matchTypesBindTypeVar(
     }
   }
 
+  // If we're attempting to bind a PackType or PackArchetypeType to a type
+  // variable that doesn't support it, we have a pack reference outside of a
+  // pack expansion expression.
+  if (!typeVar->getImpl().canBindToPack() &&
+      (type->is<PackArchetypeType>() || type->is<PackType>())) {
+    if (shouldAttemptFixes()) {
+      auto *fix = AllowInvalidPackReference::create(*this, type,
+                                                    getConstraintLocator(locator));
+      if (!recordFix(fix)) {
+        recordPotentialHole(typeVar);
+        return getTypeMatchSuccess();
+      }
+    }
+
+    return getTypeMatchFailure(locator);
+  }
+
   // 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()) {
@@ -14153,6 +14170,7 @@ ConstraintSystem::SolutionKind ConstraintSystem::simplifyFixConstraint(
   case FixKind::RenameConflictingPatternVariables:
   case FixKind::MustBeCopyable:
   case FixKind::AllowInvalidPackElement:
+  case FixKind::AllowInvalidPackReference:
   case FixKind::MacroMissingPound:
   case FixKind::AllowGlobalActorMismatch:
   case FixKind::GenericArgumentsMismatch: {

lib/Sema/TypeCheckType.cpp

@@ -4618,7 +4618,7 @@ NeverNullType TypeResolver::resolvePackElement(PackElementTypeRepr *repr,
   if (!options.contains(TypeResolutionFlags::AllowPackReferences)) {
     ctx.Diags.diagnose(repr->getLoc(),
                        diag::pack_reference_outside_expansion,
-                       packReference);
+                       packReference, /*inExpression*/false);
     return ErrorType::get(ctx);
   }
 

test/Constraints/pack-expansion-expressions.swift

@@ -218,3 +218,25 @@ func takesFunctionPack<each T, R>(functions: repeat ((each T) -> R)) {}
 func forwardFunctionPack<each T>(functions: repeat (each T) -> Bool) {
   takesFunctionPack(functions: repeat each functions)
 }
+
+func packOutsideExpansion<each T>(_ t: repeat each T) {
+  _ = t
+  // expected-error@-1{{pack reference 'T' can only appear in pack expansion}}
+
+  forward(t)
+  // expected-error@-1{{pack reference 'T' can only appear in pack expansion}}
+
+  _ = each t
+  // expected-error@-1{{pack reference 'T' can only appear in pack expansion}}
+
+  forward(each t)
+  // expected-error@-1{{pack reference 'T' can only appear in pack expansion}}
+
+  let tuple = (repeat each t)
+
+  _ = tuple.element
+  // expected-error@-1{{pack reference 'T' can only appear in pack expansion}}
+
+  _ = each tuple.element
+  // expected-error@-1{{pack reference 'T' can only appear in pack expansion}}
+}