RequirementMachine: Protocol requirement signature minimization fixes

include/swift/AST/ASTContext.h

@@ -1231,6 +1231,11 @@ class ASTContext final {
   bool isRecursivelyConstructingRequirementMachine(
       CanGenericSignature sig);
 
+  /// This is a hack to break cycles. Don't introduce new callers of this
+  /// method.
+  bool isRecursivelyConstructingRequirementMachine(
+      const ProtocolDecl *proto);
+
   /// Retrieve a generic signature with a single unconstrained type parameter,
   /// like `<T>`.
   CanGenericSignature getSingleGenericParameterSignature() const;

lib/AST/ASTContext.cpp

@@ -2017,6 +2017,11 @@ bool ASTContext::isRecursivelyConstructingRequirementMachine(
   return getRewriteContext().isRecursivelyConstructingRequirementMachine(sig);
 }
 
+bool ASTContext::isRecursivelyConstructingRequirementMachine(
+      const ProtocolDecl *proto) {
+  return getRewriteContext().isRecursivelyConstructingRequirementMachine(proto);
+}
+
 Optional<llvm::TinyPtrVector<ValueDecl *>>
 OverriddenDeclsRequest::getCachedResult() const {
   auto decl = std::get<0>(getStorage());

lib/AST/GenericSignatureBuilder.cpp

@@ -3991,6 +3991,8 @@ ConstraintResult GenericSignatureBuilder::expandConformanceRequirement(
   auto protocolSubMap = SubstitutionMap::getProtocolSubstitutions(
       proto, selfType.getDependentType(*this), ProtocolConformanceRef(proto));
 
+  auto result = ConstraintResult::Resolved;
+
   // Use the requirement signature to avoid rewalking the entire protocol.  This
   // cannot compute the requirement signature directly, because that may be
   // infinitely recursive: this code is also used to construct it.
@@ -4007,19 +4009,20 @@ ConstraintResult GenericSignatureBuilder::expandConformanceRequirement(
       auto reqResult = substReq
                            ? addRequirement(*substReq, innerSource, nullptr)
                            : ConstraintResult::Conflicting;
-      if (isErrorResult(reqResult)) return reqResult;
+      if (isErrorResult(reqResult) && !isErrorResult(result))
+        result = reqResult;
     }
 
-    return ConstraintResult::Resolved;
+    return result;
   }
 
   if (!onlySameTypeConstraints) {
     // Add all of the inherited protocol requirements, recursively.
     auto inheritedReqResult =
       addInheritedRequirements(proto, selfType.getUnresolvedType(), source,
                                nullptr);
-    if (isErrorResult(inheritedReqResult))
-      return inheritedReqResult;
+    if (isErrorResult(inheritedReqResult) && !isErrorResult(inheritedReqResult))
+      result = inheritedReqResult;
   }
 
   // Add any requirements in the where clause on the protocol.
@@ -4048,7 +4051,7 @@ ConstraintResult GenericSignatureBuilder::expandConformanceRequirement(
                              getASTContext()),
                          innerSource);
 
-    return ConstraintResult::Resolved;
+    return result;
   }
 
   // Remaining logic is not relevant in ObjC protocol cases.
@@ -4156,11 +4159,13 @@ ConstraintResult GenericSignatureBuilder::expandConformanceRequirement(
     Type assocType =
       DependentMemberType::get(selfType.getDependentType(*this), assocTypeDecl);
     if (!onlySameTypeConstraints) {
+      (void) resolve(assocType, source);
+
       auto assocResult =
         addInheritedRequirements(assocTypeDecl, assocType, source,
                                  /*inferForModule=*/nullptr);
-      if (isErrorResult(assocResult))
-        return assocResult;
+      if (isErrorResult(assocResult) && !isErrorResult(result))
+        result = assocResult;
     }
 
     // Add requirements from this associated type's where clause.
@@ -4314,7 +4319,7 @@ ConstraintResult GenericSignatureBuilder::expandConformanceRequirement(
     }
   }
 
-  return ConstraintResult::Resolved;
+  return result;
 }
 
 ConstraintResult GenericSignatureBuilder::addConformanceRequirement(
@@ -8876,9 +8881,23 @@ RequirementSignatureRequest::evaluate(Evaluator &evaluator,
     auto rqmResult = buildViaRQM();
     auto gsbResult = buildViaGSB();
 
-    if (rqmResult.size() != gsbResult.size() ||
-        !std::equal(rqmResult.begin(), rqmResult.end(),
-                    gsbResult.begin())) {
+    // For now, only compare conformance requirements, since those are the
+    // important ones from the ABI perspective.
+    SmallVector<Requirement, 2> rqmConformances;
+    for (auto req : rqmResult) {
+      if (req.getKind() == RequirementKind::Conformance)
+        rqmConformances.push_back(req);
+    }
+    SmallVector<Requirement, 2> gsbConformances;
+    for (auto req : gsbResult) {
+      if (req.getKind() == RequirementKind::Conformance)
+        gsbConformances.push_back(req);
+    }
+
+    if (rqmConformances.size() != gsbConformances.size() ||
+        !std::equal(rqmConformances.begin(),
+                    rqmConformances.end(),
+                    gsbConformances.begin())) {
       llvm::errs() << "RequirementMachine protocol signature minimization is broken:\n";
       llvm::errs() << "Protocol: " << proto->getName() << "\n";