RequirementMachine: Diagnose non-confluent rewrite systems instead of asserting

We assert when building a rewrite system for an existing generic
signature, or in AbstractGenericSignatureRequest, where there is no
source location.

In RequirementSignatureRequest and InferredGenericSignatureRequest
we now produce a diagnostic.

Author: slavapestov

include/swift/AST/DiagnosticsSema.def

@@ -2533,6 +2533,11 @@ WARNING(associated_type_override_typealias,none,
         "associated type %0 is redundant with type %0 declared in inherited "
         "%1 %2", (Identifier, DescriptiveDeclKind, Type))
 
+ERROR(requirement_machine_completion_failed,none,
+      "cannot build rewrite system for %select{generic signature|protocol}0; "
+      "%select{step|depth}1 limit exceeded",
+      (unsigned, unsigned))
+
 ERROR(associated_type_objc,none,
       "associated type %0 cannot be declared inside '@objc' protocol %1",
       (Identifier, Identifier))

lib/AST/RequirementMachine/RequirementMachine.cpp

@@ -35,12 +35,32 @@ RequirementMachine::RequirementMachine(RewriteContext &ctx)
 
 RequirementMachine::~RequirementMachine() {}
 
+static void checkCompletionResult(const RequirementMachine &machine,
+                                  CompletionResult result) {
+  switch (result) {
+  case CompletionResult::Success:
+    break;
+
+  case CompletionResult::MaxIterations:
+    llvm::errs() << "Rewrite system exceeds maximum completion step count\n";
+    machine.dump(llvm::errs());
+    abort();
+
+  case CompletionResult::MaxDepth:
+    llvm::errs() << "Rewrite system exceeds maximum completion depth\n";
+    machine.dump(llvm::errs());
+    abort();
+  }
+}
+
 /// Build a requirement machine for the requirements of a generic signature.
 ///
 /// This must only be called exactly once, before any other operations are
 /// performed on this requirement machine.
 ///
 /// Used by ASTContext::getOrCreateRequirementMachine().
+///
+/// Asserts if completion fails within the configured number of steps.
 void RequirementMachine::initWithGenericSignature(CanGenericSignature sig) {
   Sig = sig;
   Params.append(sig.getGenericParams().begin(),
@@ -64,7 +84,8 @@ void RequirementMachine::initWithGenericSignature(CanGenericSignature sig) {
                     std::move(builder.PermanentRules),
                     std::move(builder.RequirementRules));
 
-  computeCompletion(RewriteSystem::DisallowInvalidRequirements);
+  auto result = computeCompletion(RewriteSystem::DisallowInvalidRequirements);
+  checkCompletionResult(*this, result);
 
   if (Dump) {
     llvm::dbgs() << "}\n";
@@ -79,7 +100,10 @@ void RequirementMachine::initWithGenericSignature(CanGenericSignature sig) {
 /// performed on this requirement machine.
 ///
 /// Used by RequirementSignatureRequest.
-void RequirementMachine::initWithProtocols(ArrayRef<const ProtocolDecl *> protos) {
+///
+/// Returns failure if completion fails within the configured number of steps.
+CompletionResult
+RequirementMachine::initWithProtocols(ArrayRef<const ProtocolDecl *> protos) {
   Protos = protos;
 
   FrontendStatsTracer tracer(Stats, "build-rewrite-system");
@@ -100,12 +124,13 @@ void RequirementMachine::initWithProtocols(ArrayRef<const ProtocolDecl *> protos
                     std::move(builder.PermanentRules),
                     std::move(builder.RequirementRules));
 
-  // FIXME: Only if the protocols were written in source, though.
-  computeCompletion(RewriteSystem::AllowInvalidRequirements);
+  auto result = computeCompletion(RewriteSystem::AllowInvalidRequirements);
 
   if (Dump) {
     llvm::dbgs() << "}\n";
   }
+
+  return result;
 }
 
 /// Build a requirement machine from a set of generic parameters and
@@ -115,6 +140,8 @@ void RequirementMachine::initWithProtocols(ArrayRef<const ProtocolDecl *> protos
 /// performed on this requirement machine.
 ///
 /// Used by AbstractGenericSignatureRequest.
+///
+/// Asserts if completion fails within the configured number of steps.
 void RequirementMachine::initWithAbstractRequirements(
     ArrayRef<GenericTypeParamType *> genericParams,
     ArrayRef<Requirement> requirements) {
@@ -139,7 +166,8 @@ void RequirementMachine::initWithAbstractRequirements(
                     std::move(builder.PermanentRules),
                     std::move(builder.RequirementRules));
 
-  computeCompletion(RewriteSystem::AllowInvalidRequirements);
+  auto result = computeCompletion(RewriteSystem::AllowInvalidRequirements);
+  checkCompletionResult(*this, result);
 
   if (Dump) {
     llvm::dbgs() << "}\n";
@@ -153,7 +181,10 @@ void RequirementMachine::initWithAbstractRequirements(
 /// performed on this requirement machine.
 ///
 /// Used by InferredGenericSignatureRequest.
-void RequirementMachine::initWithWrittenRequirements(
+///
+/// Returns failure if completion fails within the configured number of steps.
+CompletionResult
+RequirementMachine::initWithWrittenRequirements(
     ArrayRef<GenericTypeParamType *> genericParams,
     ArrayRef<StructuralRequirement> requirements) {
   Params.append(genericParams.begin(), genericParams.end());
@@ -177,17 +208,20 @@ void RequirementMachine::initWithWrittenRequirements(
                     std::move(builder.PermanentRules),
                     std::move(builder.RequirementRules));
 
-  computeCompletion(RewriteSystem::AllowInvalidRequirements);
+  auto result = computeCompletion(RewriteSystem::AllowInvalidRequirements);
 
   if (Dump) {
     llvm::dbgs() << "}\n";
   }
+
+  return result;
 }
 
 /// Attempt to obtain a confluent rewrite system by iterating the Knuth-Bendix
 /// completion procedure together with property map construction until fixed
 /// point.
-void RequirementMachine::computeCompletion(RewriteSystem::ValidityPolicy policy) {
+CompletionResult
+RequirementMachine::computeCompletion(RewriteSystem::ValidityPolicy policy) {
   while (true) {
     // First, run the Knuth-Bendix algorithm to resolve overlapping rules.
     auto result = System.computeConfluentCompletion(
@@ -200,26 +234,8 @@ void RequirementMachine::computeCompletion(RewriteSystem::ValidityPolicy policy)
     }
 
     // Check for failure.
-    auto checkCompletionResult = [&]() {
-      switch (result.first) {
-      case CompletionResult::Success:
-        break;
-
-      case CompletionResult::MaxIterations:
-        llvm::errs() << "Generic signature " << Sig
-                     << " exceeds maximum completion step count\n";
-        System.dump(llvm::errs());
-        abort();
-
-      case CompletionResult::MaxDepth:
-        llvm::errs() << "Generic signature " << Sig
-                     << " exceeds maximum completion depth\n";
-        System.dump(llvm::errs());
-        abort();
-      }
-    };
-
-    checkCompletionResult();
+    if (result.first != CompletionResult::Success)
+      return result.first;
 
     // Check invariants.
     System.verifyRewriteRules(policy);
@@ -236,7 +252,9 @@ void RequirementMachine::computeCompletion(RewriteSystem::ValidityPolicy policy)
         .NumRequirementMachineUnifiedConcreteTerms += result.second;
     }
 
-    checkCompletionResult();
+    // Check for failure.
+    if (result.first != CompletionResult::Success)
+      return result.first;
 
     // If buildPropertyMap() added new rules, we run another round of
     // Knuth-Bendix, and build the property map again.
@@ -250,6 +268,8 @@ void RequirementMachine::computeCompletion(RewriteSystem::ValidityPolicy policy)
 
   assert(!Complete);
   Complete = true;
+
+  return CompletionResult::Success;
 }
 
 bool RequirementMachine::isComplete() const {

lib/AST/RequirementMachine/RequirementMachine.h

@@ -86,17 +86,17 @@ class RequirementMachine final {
   RequirementMachine &operator=(RequirementMachine &&) = delete;
 
   void initWithGenericSignature(CanGenericSignature sig);
-  void initWithProtocols(ArrayRef<const ProtocolDecl *> protos);
+  CompletionResult initWithProtocols(ArrayRef<const ProtocolDecl *> protos);
   void initWithAbstractRequirements(
       ArrayRef<GenericTypeParamType *> genericParams,
       ArrayRef<Requirement> requirements);
-  void initWithWrittenRequirements(
+  CompletionResult initWithWrittenRequirements(
       ArrayRef<GenericTypeParamType *> genericParams,
       ArrayRef<StructuralRequirement> requirements);
 
   bool isComplete() const;
 
-  void computeCompletion(RewriteSystem::ValidityPolicy policy);
+  CompletionResult computeCompletion(RewriteSystem::ValidityPolicy policy);
 
   MutableTerm getLongestValidPrefix(const MutableTerm &term) const;
 

lib/AST/RequirementMachine/RequirementMachineRequests.cpp

@@ -21,6 +21,7 @@
 #include "RequirementMachine.h"
 #include "swift/AST/ASTContext.h"
 #include "swift/AST/Decl.h"
+#include "swift/AST/DiagnosticsSema.h"
 #include "swift/AST/GenericSignature.h"
 #include "swift/AST/LazyResolver.h"
 #include "swift/AST/Requirement.h"
@@ -247,7 +248,25 @@ RequirementSignatureRequestRQM::evaluate(Evaluator &evaluator,
   std::unique_ptr<RequirementMachine> machine(new RequirementMachine(
       ctx.getRewriteContext()));
 
-  machine->initWithProtocols(component);
+  auto status = machine->initWithProtocols(component);
+  if (status != CompletionResult::Success) {
+    // All we can do at this point is diagnose and give each protocol an empty
+    // requirement signature.
+    for (const auto *otherProto : component) {
+      ctx.Diags.diagnose(otherProto->getLoc(),
+                         diag::requirement_machine_completion_failed,
+                         /*protocol=*/1,
+                         status == CompletionResult::MaxIterations ? 0 : 1);
+
+      if (otherProto != proto) {
+        ctx.evaluator.cacheOutput(
+          RequirementSignatureRequestRQM{const_cast<ProtocolDecl *>(otherProto)},
+          ArrayRef<Requirement>());
+      }
+    }
+
+    return ArrayRef<Requirement>();
+  }
 
   auto minimalRequirements = machine->computeMinimalProtocolRequirements();
 
@@ -400,7 +419,10 @@ InferredGenericSignatureRequestRQM::evaluate(
     return false;
   };
 
+  SourceLoc loc;
   if (genericParamList) {
+    loc = genericParamList->getLAngleLoc();
+
     // Extensions never have a parent signature.
     assert(genericParamList->getOuterParameters() == nullptr || !parentSig);
 
@@ -440,6 +462,9 @@ InferredGenericSignatureRequestRQM::evaluate(
   }
 
   if (whereClause) {
+    if (loc.isInvalid())
+      loc = whereClause.getLoc();
+
     std::move(whereClause).visitRequirements(
         TypeResolutionStage::Structural,
         visitRequirement);
@@ -464,7 +489,16 @@ InferredGenericSignatureRequestRQM::evaluate(
   std::unique_ptr<RequirementMachine> machine(new RequirementMachine(
       ctx.getRewriteContext()));
 
-  machine->initWithWrittenRequirements(genericParams, requirements);
+  auto status = machine->initWithWrittenRequirements(genericParams, requirements);
+  if (status != CompletionResult::Success) {
+    ctx.Diags.diagnose(loc,
+                       diag::requirement_machine_completion_failed,
+                       /*protocol=*/0,
+                       status == CompletionResult::MaxIterations ? 0 : 1);
+
+    auto result = GenericSignature::get(genericParams, {});
+    return GenericSignatureWithError(result, /*hadError=*/true);
+  }
 
   auto minimalRequirements =
     machine->computeMinimalGenericSignatureRequirements();

test/Generics/non_confluent.swift

@@ -0,0 +1,30 @@
+// RUN: %target-typecheck-verify-swift -requirement-machine-protocol-signatures=on -requirement-machine-inferred-signatures=on
+
+protocol Undecidable // expected-error {{cannot build rewrite system for protocol; depth limit exceeded}}
+    where A.C == C.A, // expected-error *{{is not a member type}}
+          A.D == D.A, // expected-error *{{is not a member type}}
+          B.C == C.B, // expected-error *{{is not a member type}}
+          B.D == D.B, // expected-error *{{is not a member type}}
+          C.E == E.C.A, // expected-error *{{is not a member type}}
+          D.E == E.D.B, // expected-error *{{is not a member type}}
+          C.C.A == C.C.A.E { // expected-error *{{is not a member type}}
+  associatedtype A : Undecidable
+  associatedtype B : Undecidable
+  associatedtype C : Undecidable
+  associatedtype D : Undecidable
+  associatedtype E : Undecidable
+}
+
+protocol P1 {
+  associatedtype T : P1
+}
+
+protocol P2 {
+  associatedtype T : P2
+}
+
+func foo<T : P1 & P2>(_: T) {}
+// expected-error@-1 {{cannot build rewrite system for generic signature; depth limit exceeded}}
+
+extension P1 where Self : P2 {}
+// expected-error@-1 {{cannot build rewrite system for generic signature; depth limit exceeded}}