[Requirement Machine] When forming requirements from same-element rewrite rules, swap the sides depending on which one is longer once the [element] symbol is dropped.

This change fixes the "out-of-order" type parameters error in `GenericSignature::verify`

Author: simanerush

lib/AST/GenericSignature.cpp

@@ -936,12 +936,8 @@ void GenericSignature::verify(ArrayRef<Requirement> reqts) const {
           llvm::errs() << "\n";
           dumpAndAbort();
         }
-        // FIXME: Same-element rewrite rules are of the form [element].T => U,
-        // which turns into a same-type requirement repeat U == each T. The
-        // pack element must always be on the left side of the rule, so type
-        // parameters can appear out of order in the final same-type requirement.
-        if (firstType->isParameterPack() == secondType->isParameterPack() &&
-            compareDependentTypes(firstType, secondType) >= 0) {
+
+        if (compareDependentTypes(firstType, secondType) >= 0) {
           llvm::errs() << "Out-of-order type parameters: ";
           reqt.dump(llvm::errs());
           llvm::errs() << "\n";

lib/AST/RequirementMachine/RequirementBuilder.cpp

@@ -236,18 +236,42 @@ void RequirementBuilder::addRequirementRules(ArrayRef<unsigned> rules) {
       llvm_unreachable("Invalid symbol kind");
     }
 
+    MutableTerm constraintTerm = MutableTerm(rule.getLHS());
+    MutableTerm subjectTerm = MutableTerm(rule.getRHS());
+
+    RewriteContext &ctx = this->System.getRewriteContext();
+
+    // Drop the [element] symbol from lhs to determine if we need to swap the
+    // sides.
+    if (constraintTerm[0].getKind() == Symbol::Kind::PackElement) {
+      constraintTerm =
+          MutableTerm(constraintTerm.begin() + 1, constraintTerm.end());
+
+      // Make sure that the shorter term is ordered first.
+      if (constraintTerm.compare(subjectTerm, ctx) == -1) {
+        MutableTerm tempTerm = subjectTerm;
+        subjectTerm = constraintTerm;
+        constraintTerm = tempTerm;
+      }
+    }
+
     ASSERT(rule.getLHS().back().getKind() != Symbol::Kind::Protocol);
 
-    MutableTerm constraintTerm(rule.getLHS());
     if (constraintTerm.back().getKind() == Symbol::Kind::Shape) {
       ASSERT(rule.getRHS().back().getKind() == Symbol::Kind::Shape);
       // Strip off the shape symbol from the constraint term.
       constraintTerm = MutableTerm(constraintTerm.begin(),
                                    constraintTerm.end() - 1);
     }
 
+    if (constraintTerm.front().getKind() == Symbol::Kind::PackElement) {
+      // Strip off the element symbol from the constraint term.
+      constraintTerm = MutableTerm(constraintTerm.begin() + 1,
+                                   constraintTerm.end());
+    }
+
     auto constraintType = Map.getTypeForTerm(constraintTerm, GenericParams);
-    Components[rule.getRHS()].Members.push_back(constraintType);
+    Components[Term::get(subjectTerm, ctx)].Members.push_back(constraintType);
   };
 
   if (Debug) {
@@ -314,6 +338,10 @@ void RequirementBuilder::processConnectedComponents() {
       subjectTerm = MutableTerm(subjectTerm.begin(), subjectTerm.end() - 1);
     } else {
       kind = RequirementKind::SameType;
+      if (subjectTerm.front().getKind() == Symbol::Kind::PackElement) {
+        // Strip off the element symbol from the subject term.
+        subjectTerm = MutableTerm(subjectTerm.begin() + 1, subjectTerm.end());
+      }
     }
 
     auto subjectType = Map.getTypeForTerm(subjectTerm, GenericParams);

lib/AST/RequirementMachine/Term.h

@@ -171,6 +171,10 @@ class MutableTerm final {
 
   std::reverse_iterator<Symbol *> rbegin() { return Symbols.rbegin(); }
   std::reverse_iterator<Symbol *> rend() { return Symbols.rend(); }
+  
+  Symbol front() const {
+    return Symbols.front();
+  }
 
   Symbol back() const {
     return Symbols.back();