RequirementMachine: Don't assert if a rewrite system has unresolved rules

This is a source-level error, not an invariant violation. Instead, plumb
a new hadError() flag, which in the future will assert if no diagnostic
was produced.

Author: slavapestov

lib/AST/RequirementMachine/GeneratingConformances.cpp

@@ -646,6 +646,9 @@ void RewriteSystem::computeGeneratingConformances(
     if (rule.isRedundant())
       continue;
 
+    if (rule.containsUnresolvedSymbols())
+      continue;
+
     if (!rule.isProtocolConformanceRule())
       continue;
 

lib/AST/RequirementMachine/HomotopyReduction.cpp

@@ -790,6 +790,23 @@ void RewriteSystem::minimizeRewriteSystem() {
   verifyMinimizedRules();
 }
 
+/// In a conformance-valid rewrite system, any rule with unresolved symbols on
+/// the left or right hand side should have been simplified by another rule.
+bool RewriteSystem::hasNonRedundantUnresolvedRules() const {
+  assert(Complete);
+  assert(Minimized);
+
+  for (const auto &rule : Rules) {
+    if (!rule.isRedundant() &&
+        !rule.isPermanent() &&
+        rule.containsUnresolvedSymbols()) {
+      return true;
+    }
+  }
+
+  return false;
+}
+
 /// Collect all non-permanent, non-redundant rules whose domain is equal to
 /// one of the protocols in \p proto. In other words, the first symbol of the
 /// left hand side term is either a protocol symbol or associated type symbol
@@ -805,11 +822,11 @@ RewriteSystem::getMinimizedProtocolRules(
   for (unsigned ruleID : indices(Rules)) {
     const auto &rule = getRule(ruleID);
 
-    if (rule.isPermanent())
-      continue;
-
-    if (rule.isRedundant())
+    if (rule.isPermanent() ||
+        rule.isRedundant() ||
+        rule.containsUnresolvedSymbols()) {
       continue;
+    }
 
     auto domain = rule.getLHS()[0].getProtocols();
     assert(domain.size() == 1);
@@ -834,11 +851,11 @@ RewriteSystem::getMinimizedGenericSignatureRules() const {
   for (unsigned ruleID : indices(Rules)) {
     const auto &rule = getRule(ruleID);
 
-    if (rule.isPermanent())
-      continue;
-
-    if (rule.isRedundant())
+    if (rule.isPermanent() ||
+        rule.isRedundant() ||
+        rule.containsUnresolvedSymbols()) {
       continue;
+    }
 
     if (rule.getLHS()[0].getKind() != Symbol::Kind::GenericParam)
       continue;
@@ -917,29 +934,17 @@ void RewriteSystem::verifyMinimizedRules() const {
       continue;
     }
 
-    if (rule.isRedundant())
-      continue;
-
     // Simplified rules should be redundant, unless they're protocol conformance
     // rules, which unfortunately might no be redundant, because we try to keep
     // them in the original protocol definition for compatibility with the
     // GenericSignatureBuilder's minimization algorithm.
-    if (rule.isSimplified() && !rule.isProtocolConformanceRule()) {
+    if (rule.isSimplified() &&
+        !rule.isRedundant() &&
+        !rule.isProtocolConformanceRule()) {
       llvm::errs() << "Simplified rule is not redundant: " << rule << "\n\n";
       dump(llvm::errs());
       abort();
     }
-
-    // Rules with unresolved name symbols (other than permanent rules for
-    // associated type introduction) should be redundant.
-    //
-    // FIXME: What about invalid code?
-    if (rule.getLHS().containsUnresolvedSymbols() ||
-        rule.getRHS().containsUnresolvedSymbols()) {
-      llvm::errs() << "Unresolved rule is not redundant: " << rule << "\n\n";
-      dump(llvm::errs());
-      abort();
-    }
   }
 #endif
 }

lib/AST/RequirementMachine/RequirementMachine.cpp

@@ -256,6 +256,12 @@ bool RequirementMachine::isComplete() const {
   return Complete;
 }
 
+bool RequirementMachine::hadError() const {
+  // FIXME: Implement other checks here
+  // FIXME: Assert if hadError() is true but we didn't emit any diagnostics?
+  return System.hasNonRedundantUnresolvedRules();
+}
+
 void RequirementMachine::dump(llvm::raw_ostream &out) const {
   out << "Requirement machine for ";
   if (Sig)

lib/AST/RequirementMachine/RequirementMachine.h

@@ -137,6 +137,8 @@ class RequirementMachine final {
 
   std::vector<Requirement> computeMinimalGenericSignatureRequirements();
 
+  bool hadError() const;
+
   void verify(const MutableTerm &term) const;
   void dump(llvm::raw_ostream &out) const;
 

lib/AST/RequirementMachine/RequirementMachineRequests.cpp

@@ -359,10 +359,9 @@ AbstractGenericSignatureRequestRQM::evaluate(
   auto minimalRequirements =
     machine->computeMinimalGenericSignatureRequirements();
 
-  // FIXME: Implement this
-  bool hadError = false;
-
   auto result = GenericSignature::get(genericParams, minimalRequirements);
+  bool hadError = machine->hadError();
+
   return GenericSignatureWithError(result, hadError);
 }
 
@@ -463,10 +462,8 @@ InferredGenericSignatureRequestRQM::evaluate(
   auto minimalRequirements =
     machine->computeMinimalGenericSignatureRequirements();
 
-  // FIXME: Implement this
-  bool hadError = false;
-
   auto result = GenericSignature::get(genericParams, minimalRequirements);
+  bool hadError = machine->hadError();
 
   // FIXME: Handle allowConcreteGenericParams
 

lib/AST/RequirementMachine/RewriteSystem.h

@@ -103,6 +103,11 @@ class Rule final {
     return Redundant;
   }
 
+  bool containsUnresolvedSymbols() const {
+    return (LHS.containsUnresolvedSymbols() ||
+            RHS.containsUnresolvedSymbols());
+  }
+
   void markSimplified() {
     assert(!Simplified);
     Simplified = true;
@@ -314,8 +319,6 @@ class RewriteSystem final {
 
   void checkMergedAssociatedType(Term lhs, Term rhs);
 
-public:
-
   //////////////////////////////////////////////////////////////////////////////
   ///
   /// Homotopy reduction
@@ -337,13 +340,17 @@ class RewriteSystem final {
   void performHomotopyReduction(
       const llvm::DenseSet<unsigned> *redundantConformances);
 
+public:
   void minimizeRewriteSystem();
 
+  bool hasNonRedundantUnresolvedRules() const;
+
   llvm::DenseMap<const ProtocolDecl *, std::vector<unsigned>>
   getMinimizedProtocolRules(ArrayRef<const ProtocolDecl *> protos) const;
 
   std::vector<unsigned> getMinimizedGenericSignatureRules() const;
 
+private:
   void verifyRewriteLoops() const;
 
   void verifyRedundantConformances(
@@ -398,6 +405,7 @@ class RewriteSystem final {
   void computeGeneratingConformances(
       llvm::DenseSet<unsigned> &redundantConformances);
 
+public:
   void dump(llvm::raw_ostream &out) const;
 };