RequirementMachine: Fix generating conformances algorithm for rules of the form [P].[P] => [P]

Author: slavapestov

lib/AST/RequirementMachine/GeneratingConformances.cpp

@@ -65,15 +65,18 @@ void HomotopyGenerator::findProtocolConformanceRules(
     step.apply(term, system);
   }
 
-  assert(notInContext.empty() || !inContext.empty() &&
-         "A conformance rule not based on another conformance rule?");
+  if (notInContext.size() == 1 && inContext.empty()) {
+    llvm::errs() << "A conformance rule not based on another conformance rule?\n";
+    dump(llvm::errs(), system);
+    llvm::errs() << "\n";
+    abort();
+  }
 }
 
 /// Write the term as a product of left hand sides of protocol conformance
 /// rules.
 ///
-/// The term should already be simplified, except for a protocol symbol
-/// at the end.
+/// The term should be irreducible, except for a protocol symbol at the end.
 void
 RewriteSystem::decomposeTermIntoConformanceRuleLeftHandSides(
     MutableTerm term, SmallVectorImpl<unsigned> &result) const {
@@ -97,27 +100,54 @@ RewriteSystem::decomposeTermIntoConformanceRuleLeftHandSides(
   assert(step.EndOffset == 0);
   assert(!step.Inverse);
 
-  const auto &rule = getRule(step.RuleID);
-  assert(rule.isProtocolConformanceRule());
-
   // If |U| > 0, recurse with the term U.[domain(V)]. Since T is
   // canonical, we know that U is canonical as well.
   if (step.StartOffset > 0) {
     // Build the term U.
     MutableTerm prefix(term.begin(), term.begin() + step.StartOffset);
 
-    // Compute domain(V).
-    const auto &lhs = rule.getLHS();
-    auto protocols = lhs[0].getProtocols();
-    assert(protocols.size() == 1);
+    decomposeTermIntoConformanceRuleLeftHandSides(prefix, step.RuleID, result);
+  } else {
+    result.push_back(step.RuleID);
+  }
+}
 
-    // Build the term U.[domain(V)].
-    prefix.add(Symbol::forProtocol(protocols[0], Context));
+/// Given a term U and a rule (V.[P] => V), write U.[domain(V)] as a
+/// product of left hand sdies of conformance rules. The term U should
+/// be irreducible.
+void
+RewriteSystem::decomposeTermIntoConformanceRuleLeftHandSides(
+    MutableTerm term, unsigned ruleID,
+    SmallVectorImpl<unsigned> &result) const {
+  const auto &rule = getRule(ruleID);
+  assert(rule.isProtocolConformanceRule());
 
-    decomposeTermIntoConformanceRuleLeftHandSides(prefix, result);
+  // Compute domain(V).
+  const auto &lhs = rule.getLHS();
+  auto protocols = lhs[0].getProtocols();
+  assert(protocols.size() == 1);
+  auto protocol = Symbol::forProtocol(protocols[0], Context);
+
+  // A same-type requirement of the form 'Self.Foo == Self' can induce a
+  // conformance rule [P].[P] => [P], and we can end up with a generating
+  // conformance decomposition of the form
+  //
+  //   (V.[Q] => V) := [P].(V'.[Q] => V'),
+  //
+  // where domain(V) == [P]. Don't recurse on [P].[P] here since it won't
+  // yield anything useful, instead just return with (V'.[Q] => V').
+  if (term.size() == 1 && term[0] == protocol) {
+    result.push_back(ruleID);
+    return;
   }
 
-  result.push_back(step.RuleID);
+  // Build the term U.[domain(V)].
+  term.add(protocol);
+
+  decomposeTermIntoConformanceRuleLeftHandSides(term, result);
+
+  // Add the rule V => V.[P].
+  result.push_back(ruleID);
 }
 
 /// Use homotopy information to discover all ways of writing the left hand side
@@ -222,29 +252,16 @@ void RewriteSystem::computeCandidateConformancePaths(
     SmallVector<SmallVector<unsigned, 2>, 2> candidatePaths;
     for (auto pair : inContext) {
       // We have a term U, and a rule V.[P] => V.
-      const auto &rule = getRule(pair.second);
-      assert(rule.isProtocolConformanceRule());
-
       SmallVector<unsigned, 2> conformancePath;
 
       // Simplify U to get U'.
       MutableTerm term = pair.first;
       (void) simplify(term);
 
-      // Compute domain(V).
-      const auto &lhs = rule.getLHS();
-      auto protocols = lhs[0].getProtocols();
-      assert(protocols.size() == 1);
-
-      // Build the term U'.[domain(V)].
-      term.add(Symbol::forProtocol(protocols[0], Context));
-
       // Write U'.[domain(V)] as a product of left hand sides of protocol
       // conformance rules.
-      decomposeTermIntoConformanceRuleLeftHandSides(term, conformancePath);
-
-      // Add the rule V => V.[P].
-      conformancePath.push_back(pair.second);
+      decomposeTermIntoConformanceRuleLeftHandSides(term, pair.second,
+                                                    conformancePath);
 
       candidatePaths.push_back(conformancePath);
     }

lib/AST/RequirementMachine/HomotopyReduction.cpp

@@ -658,6 +658,7 @@ void RewriteSystem::deleteRule(unsigned ruleID,
 /// Use the 3-cells to delete rewrite rules, updating and simplifying existing
 /// 3-cells as each rule is deleted.
 void RewriteSystem::minimizeRewriteSystem() {
+  // First, eliminate all redundant rules that are not conformance rules.
   while (true) {
     RewritePath replacementPath;
     if (auto optRuleID = findRuleToDelete(replacementPath, nullptr))
@@ -666,9 +667,15 @@ void RewriteSystem::minimizeRewriteSystem() {
       break;
   }
 
+  // Now find a minimal set of generating conformances.
+  //
+  // FIXME: For now this just produces a set of redundant conformances, but
+  // it should actually compute the full generating conformance basis, since
+  // we want to use the same information for finding conformance access paths.
   llvm::DenseSet<unsigned> redundantConformances;
   computeGeneratingConformances(redundantConformances);
 
+  // Now, eliminate all redundant conformance rules.
   while (true) {
     RewritePath replacementPath;
     if (auto optRuleID = findRuleToDelete(replacementPath,

lib/AST/RequirementMachine/RewriteSystem.cpp

@@ -41,6 +41,13 @@ Optional<Symbol> Rule::isPropertyRule() const {
   if (!std::equal(RHS.begin(), RHS.end(), LHS.begin()))
     return None;
 
+  // A same-type requirement of the form 'Self.Foo == Self' can induce a
+  // conformance rule [P].[P] => [P]. Don't consider this a property-like
+  // rule, since it messes up the generating conformances algorithm and
+  // doesn't mean anything useful anyway.
+  if (RHS.size() == 1 && RHS[0] == LHS[1])
+    return None;
+
   return property;
 }
 

lib/AST/RequirementMachine/RewriteSystem.h

@@ -471,6 +471,9 @@ class RewriteSystem final {
   void decomposeTermIntoConformanceRuleLeftHandSides(
       MutableTerm term,
       SmallVectorImpl<unsigned> &result) const;
+  void decomposeTermIntoConformanceRuleLeftHandSides(
+      MutableTerm term, unsigned ruleID,
+      SmallVectorImpl<unsigned> &result) const;
 
   void computeCandidateConformancePaths(
       llvm::MapVector<unsigned,