[ConstraintSystem] Detect and diagnose missing each for value pack re…

…ference Detect that a value pack is missing 'each' keyword during constraint generation and fix-it by injecting `PackElementExpr`.
swiftlang · May 5, 2023 · 1ad71ee · 1ad71ee
1 parent 938eb97
commit 1ad71ee
Show file tree

Hide file tree

Showing 3 changed files with 70 additions and 21 deletions.
diff --git a/lib/Sema/CSFix.cpp b/lib/Sema/CSFix.cpp
@@ -2787,7 +2787,9 @@ AllowValueExpansionWithoutPackReferences::create(ConstraintSystem &cs,
 
 bool IgnoreMissingEachKeyword::diagnose(const Solution &solution,
                                         bool asNote) const {
-  return false;
+  MissingEachForValuePackReference failure(solution, ValuePackType,
+                                           getLocator());
+  return failure.diagnose(asNote);
 }
 
 IgnoreMissingEachKeyword *

diff --git a/lib/Sema/CSGen.cpp b/lib/Sema/CSGen.cpp
@@ -1089,6 +1089,23 @@ namespace {
       return outputTy;
     }
 
+    Type openPackElement(Type packType, ConstraintLocator *locator) {
+      // 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(locator,
+                                     TVO_CanBindToHole | TVO_CanBindToNoEscape);
+      }
+
+      // The type of a PackElementExpr is the opened pack element archetype
+      // of the pack reference.
+      OpenPackElementType openPackElement(CS, locator,
+                                          PackElementEnvironments.back());
+      return openPackElement(packType, /*packRepr*/ nullptr);
+    }
+
   public:
     ConstraintGenerator(ConstraintSystem &CS, DeclContext *DC)
         : CS(CS), CurDC(DC ? DC : CS.DC), CurrPhase(CS.getPhase()) {
@@ -1407,6 +1424,20 @@ namespace {
             return invalidateReference();
           }
 
+          // value packs cannot be referenced without `each` immediately
+          // preceding them.
+          if (auto *expansion = knownType->getAs<PackExpansionType>()) {
+            if (!PackElementEnvironments.empty() &&
+                !isExpr<PackElementExpr>(CS.getParentExpr(E))) {
+              auto packType = expansion->getPatternType();
+              (void)CS.recordFix(
+                  IgnoreMissingEachKeyword::create(CS, packType, locator));
+              auto eltType = openPackElement(packType, locator);
+              CS.setType(E, eltType);
+              return eltType;
+            }
+          }
+
           if (!knownType->hasPlaceholder()) {
             // Set the favored type for this expression to the known type.
             CS.setFavoredType(E, knownType.getPointer());
@@ -3067,10 +3098,12 @@ namespace {
                               SmallVectorImpl<ASTNode> &packs) {
       struct PackCollector : public ASTWalker {
       private:
+        ConstraintSystem &CS;
         SmallVectorImpl<ASTNode> &Packs;
 
       public:
-        PackCollector(SmallVectorImpl<ASTNode> &packs) : Packs(packs) {}
+        PackCollector(ConstraintSystem &cs, SmallVectorImpl<ASTNode> &packs)
+            : CS(cs), Packs(packs) {}
 
         /// Walk everything that's available.
         MacroWalking getMacroWalkingBehavior() const override {
@@ -3087,6 +3120,21 @@ namespace {
             Packs.push_back(E);
           }
 
+          if (auto *declRef = dyn_cast<DeclRefExpr>(E)) {
+            auto type = CS.getTypeIfAvailable(declRef);
+            if (!type)
+              return Action::Continue(E);
+
+            if (type->is<ElementArchetypeType>() &&
+                CS.hasFixFor(CS.getConstraintLocator(declRef),
+                             FixKind::IgnoreMissingEachKeyword)) {
+              Packs.push_back(PackElementExpr::create(CS.getASTContext(),
+                                                      /*eachLoc=*/SourceLoc(),
+                                                      declRef,
+                                                      /*implicit=*/true));
+            }
+          }
+
           return Action::Continue(E);
         }
 
@@ -3102,7 +3150,7 @@ namespace {
 
           return Action::Continue();
         }
-      } packCollector(packs);
+      } packCollector(CS, packs);
 
       expansion->getPatternExpr()->walk(packCollector);
     }
@@ -3162,23 +3210,8 @@ namespace {
     }
 
     Type visitPackElementExpr(PackElementExpr *expr) {
-      auto packType = CS.getType(expr->getPackRefExpr());
-
-      // 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.
-      OpenPackElementType openPackElement(CS, CS.getConstraintLocator(expr),
-                                          PackElementEnvironments.back());
-      return openPackElement(packType, /*packRepr*/ nullptr);
+      return openPackElement(CS.getType(expr->getPackRefExpr()),
+                             CS.getConstraintLocator(expr));
     }
 
     Type visitMaterializePackExpr(MaterializePackExpr *expr) {

diff --git a/test/Constraints/pack-expansion-expressions.swift b/test/Constraints/pack-expansion-expressions.swift
@@ -455,7 +455,8 @@ do {
 
   func test_misplaced_each<each T: P>(_ value: repeat each T) -> (repeat each T.A) {
     return (repeat each value.makeA())
-    // expected-error@-1 {{pack reference 'each T' can only appear in pack expansion}}
+    // expected-error@-1 {{value pack 'each T' must be referenced with 'each'}} {{25-25=(each }} {{30-30=)}}
+    // expected-error@-2 {{pack expansion requires that '()' and 'each T' have the same shape}}
   }
 }
 
@@ -481,3 +482,16 @@ do {
     // expected-error@-1:25 {{value pack expansion must contain at least one pack reference}}
   }
 }
+
+// missing 'each' keyword before value pack references
+do {
+  func overloaded<each U>(_: String, _: repeat each U) -> Int { 42 }
+  func overloaded<each T>(_: Int, _ b: repeat each T) -> (repeat each T) {
+    fatalError()
+  }
+
+  func test<each T>(v: repeat each T) {
+    _ = (repeat overloaded(42, v)) // expected-error {{value pack 'each T' must be referenced with 'each'}} {{32-32=each }}
+    _ = (repeat overloaded(42, each v)) // Ok
+  }
+}