RequirementMachine: Throw out rewrite loops not part of the minimization domain

When minimizing a generic signature, we only care about loops
where the basepoint is a generic parameter symbol.

When minimizing protocol requirement signatures in a connected
component, we only care about loops where the basepoint is a
protocol symbol or associated type symbol whose protocol is
part of the connected component.

All other loops can be discarded since they do not encode
redundancies that are relevant to us.

Author: slavapestov

lib/AST/RequirementMachine/HomotopyReduction.cpp

@@ -367,6 +367,8 @@ findRuleToDelete(llvm::function_ref<bool(unsigned)> isRedundantRuleFn,
       foundAny = true;
     }
 
+    // Delete loops that don't contain any rewrite rules in empty context,
+    // since such loops do not give us useful information.
     if (!foundAny)
       loop.markDeleted();
   }
@@ -566,15 +568,14 @@ bool RewriteSystem::hadError() const {
 }
 
 /// 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
-/// whose protocol is in \p proto.
+/// one of the protocols in the connected component represented by this
+/// rewrite system.
 ///
 /// These rules form the requirement signatures of these protocols.
 llvm::DenseMap<const ProtocolDecl *, std::vector<unsigned>>
-RewriteSystem::getMinimizedProtocolRules(
-    ArrayRef<const ProtocolDecl *> protos) const {
+RewriteSystem::getMinimizedProtocolRules() const {
   assert(Minimized);
+  assert(!Protos.empty());
 
   llvm::DenseMap<const ProtocolDecl *, std::vector<unsigned>> rules;
   for (unsigned ruleID : indices(Rules)) {
@@ -591,7 +592,7 @@ RewriteSystem::getMinimizedProtocolRules(
     assert(domain.size() == 1);
 
     const auto *proto = domain[0];
-    if (std::find(protos.begin(), protos.end(), proto) != protos.end())
+    if (std::find(Protos.begin(), Protos.end(), proto) != Protos.end())
       rules[proto].push_back(ruleID);
   }
 
@@ -605,6 +606,7 @@ RewriteSystem::getMinimizedProtocolRules(
 std::vector<unsigned>
 RewriteSystem::getMinimizedGenericSignatureRules() const {
   assert(Minimized);
+  assert(Protos.empty());
 
   std::vector<unsigned> rules;
   for (unsigned ruleID : indices(Rules)) {
@@ -685,6 +687,10 @@ void RewriteSystem::verifyMinimizedRules(
   for (unsigned ruleID : indices(Rules)) {
     const auto &rule = getRule(ruleID);
 
+    // Ignore the rewrite rule if it is not part of our minimization domain.
+    if (!isInMinimizationDomain(rule.getLHS().getRootProtocols()))
+      continue;
+
     // Note that sometimes permanent rules can be simplified, but they can never
     // be redundant.
     if (rule.isPermanent()) {

lib/AST/RequirementMachine/KnuthBendix.cpp

@@ -622,7 +622,7 @@ RewriteSystem::computeConfluentCompletion(unsigned maxIterations,
     }
 
     for (const auto &loop : resolvedLoops) {
-      recordRewriteLoop(loop);
+      recordRewriteLoop(loop.Basepoint, loop.Path);
     }
 
     resolvedCriticalPairs.clear();

lib/AST/RequirementMachine/PropertyUnification.cpp

@@ -484,8 +484,6 @@ void PropertyMap::addProperty(
   }
 
   case Symbol::Kind::Superclass: {
-    // FIXME: Also handle superclass vs concrete
-
     if (checkRuleOnce(ruleID)) {
       // A rule (T.[superclass: C] => T) induces a rule (T.[layout: L] => T),
       // where L is either AnyObject or _NativeObject.

lib/AST/RequirementMachine/RequirementMachine.cpp

@@ -81,6 +81,7 @@ void RequirementMachine::initWithGenericSignature(CanGenericSignature sig) {
 
   // Add the initial set of rewrite rules to the rewrite system.
   System.initialize(/*recordLoops=*/false,
+                    /*protos=*/ArrayRef<const ProtocolDecl *>(),
                     std::move(builder.PermanentRules),
                     std::move(builder.RequirementRules));
 
@@ -104,8 +105,6 @@ void RequirementMachine::initWithGenericSignature(CanGenericSignature sig) {
 /// 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");
 
   if (Dump) {
@@ -120,7 +119,7 @@ RequirementMachine::initWithProtocols(ArrayRef<const ProtocolDecl *> protos) {
   builder.addProtocols(protos);
 
   // Add the initial set of rewrite rules to the rewrite system.
-  System.initialize(/*recordLoops=*/true,
+  System.initialize(/*recordLoops=*/true, protos,
                     std::move(builder.PermanentRules),
                     std::move(builder.RequirementRules));
 
@@ -163,6 +162,7 @@ void RequirementMachine::initWithAbstractRequirements(
 
   // Add the initial set of rewrite rules to the rewrite system.
   System.initialize(/*recordLoops=*/true,
+                    /*protos=*/ArrayRef<const ProtocolDecl *>(),
                     std::move(builder.PermanentRules),
                     std::move(builder.RequirementRules));
 
@@ -205,6 +205,7 @@ RequirementMachine::initWithWrittenRequirements(
 
   // Add the initial set of rewrite rules to the rewrite system.
   System.initialize(/*recordLoops=*/true,
+                    /*protos=*/ArrayRef<const ProtocolDecl *>(),
                     std::move(builder.PermanentRules),
                     std::move(builder.RequirementRules));
 
@@ -291,9 +292,10 @@ void RequirementMachine::dump(llvm::raw_ostream &out) const {
     for (auto paramTy : Params)
       out << " " << Type(paramTy);
   } else {
-    assert(!Protos.empty());
+    auto protos = System.getProtocols();
+    assert(!protos.empty());
     out << "protocols [";
-    for (auto *proto : Protos) {
+    for (auto *proto : protos) {
       out << " " << proto->getName();
     }
     out << " ]";

lib/AST/RequirementMachine/RequirementMachine.h

@@ -54,7 +54,6 @@ class RequirementMachine final {
 
   CanGenericSignature Sig;
   SmallVector<Type, 2> Params;
-  ArrayRef<const ProtocolDecl *> Protos;
 
   RewriteContext &Context;
   RewriteSystem System;

lib/AST/RequirementMachine/RequirementMachineRequests.cpp

@@ -213,7 +213,9 @@ RequirementMachine::buildRequirementsFromRules(
 /// connected component.
 llvm::DenseMap<const ProtocolDecl *, std::vector<Requirement>>
 RequirementMachine::computeMinimalProtocolRequirements() {
-  assert(Protos.size() > 0 &&
+  auto protos = System.getProtocols();
+
+  assert(protos.size() > 0 &&
          "Not a protocol connected component rewrite system");
   assert(Params.empty() &&
          "Not a protocol connected component rewrite system");
@@ -225,13 +227,13 @@ RequirementMachine::computeMinimalProtocolRequirements() {
     dump(llvm::dbgs());
   }
 
-  auto rules = System.getMinimizedProtocolRules(Protos);
+  auto rules = System.getMinimizedProtocolRules();
 
-  // Note that we build 'result' by iterating over 'Protos' rather than
+  // Note that we build 'result' by iterating over 'protos' rather than
   // 'rules'; this is intentional, so that even if a protocol has no
   // rules, we still end up creating an entry for it in 'result'.
   llvm::DenseMap<const ProtocolDecl *, std::vector<Requirement>> result;
-  for (const auto *proto : Protos) {
+  for (const auto *proto : protos) {
     auto genericParams = proto->getGenericSignature().getGenericParams();
     result[proto] = buildRequirementsFromRules(rules[proto], genericParams);
   }
@@ -320,7 +322,7 @@ RequirementSignatureRequestRQM::evaluate(Evaluator &evaluator,
 /// Builds the top-level generic signature requirements for this rewrite system.
 std::vector<Requirement>
 RequirementMachine::computeMinimalGenericSignatureRequirements() {
-  assert(Protos.empty() &&
+  assert(System.getProtocols().empty() &&
          "Not a top-level generic signature rewrite system");
   assert(!Params.empty() &&
          "Not a from-source top-level generic signature rewrite system");

lib/AST/RequirementMachine/RewriteSystem.cpp

@@ -169,13 +169,14 @@ RewriteSystem::~RewriteSystem() {
 }
 
 void RewriteSystem::initialize(
-    bool recordLoops,
+    bool recordLoops, ArrayRef<const ProtocolDecl *> protos,
     std::vector<std::pair<MutableTerm, MutableTerm>> &&permanentRules,
     std::vector<std::pair<MutableTerm, MutableTerm>> &&requirementRules) {
   assert(!Initialized);
   Initialized = 1;
 
   RecordLoops = recordLoops;
+  Protos = protos;
 
   for (const auto &rule : permanentRules)
     addPermanentRule(rule.first, rule.second);
@@ -594,6 +595,40 @@ void RewriteSystem::simplifyRightHandSidesAndSubstitutions() {
   }
 }
 
+/// When minimizing a generic signature, we only care about loops where the
+/// basepoint is a generic parameter symbol.
+///
+/// When minimizing protocol requirement signatures, we only care about loops
+/// where the basepoint is a protocol symbol or associated type symbol whose
+/// protocol is part of the connected component.
+///
+/// All other loops can be discarded since they do not encode redundancies
+/// that are relevant to us.
+bool RewriteSystem::isInMinimizationDomain(
+    ArrayRef<const ProtocolDecl *> protos) const {
+  assert(protos.size() <= 1);
+
+  if (protos.empty() && Protos.empty())
+    return true;
+
+  if (std::find(Protos.begin(), Protos.end(), protos[0]) != Protos.end())
+    return true;
+
+  return false;
+}
+
+void RewriteSystem::recordRewriteLoop(MutableTerm basepoint,
+                                      RewritePath path) {
+  if (!RecordLoops)
+    return;
+
+  // Ignore the rewrite rule if it is not part of our minimization domain.
+  if (!isInMinimizationDomain(basepoint.getRootProtocols()))
+    return;
+
+  Loops.emplace_back(basepoint, path);
+}
+
 void RewriteSystem::verifyRewriteRules(ValidityPolicy policy) const {
 #ifndef NDEBUG
 

lib/AST/RequirementMachine/RewriteSystem.h

@@ -190,6 +190,11 @@ class RewriteSystem final {
   /// Rewrite context for memory allocation.
   RewriteContext &Context;
 
+  /// If this is a rewrite system for a connected component of protocols,
+  /// this array is non-empty. Otherwise, it is a rewrite system for a
+  /// top-level generic signature and this array is empty.
+  ArrayRef<const ProtocolDecl *> Protos;
+
   /// The rules added so far, including rules from our client, as well
   /// as rules introduced by the completion procedure.
   std::vector<Rule> Rules;
@@ -230,10 +235,14 @@ class RewriteSystem final {
 
   DebugOptions getDebugOptions() const { return Debug; }
 
-  void initialize(bool recordLoops,
+  void initialize(bool recordLoops, ArrayRef<const ProtocolDecl *> protos,
                   std::vector<std::pair<MutableTerm, MutableTerm>> &&permanentRules,
                   std::vector<std::pair<MutableTerm, MutableTerm>> &&requirementRules);
 
+  ArrayRef<const ProtocolDecl *> getProtocols() const {
+    return Protos;
+  }
+
   unsigned getRuleID(const Rule &rule) const {
     assert((unsigned)(&rule - &*Rules.begin()) < Rules.size());
     return (unsigned)(&rule - &*Rules.begin());
@@ -399,6 +408,7 @@ class RewriteSystem final {
   unsigned recordTypeWitness(TypeWitness witness);
   const TypeWitness &getTypeWitness(unsigned index) const;
 
+private:
   //////////////////////////////////////////////////////////////////////////////
   ///
   /// Homotopy reduction
@@ -421,20 +431,10 @@ class RewriteSystem final {
   /// algorithms.
   std::vector<RewriteLoop> Loops;
 
-  void recordRewriteLoop(RewriteLoop loop) {
-    if (!RecordLoops)
-      return;
-
-    Loops.push_back(loop);
-  }
+  bool isInMinimizationDomain(ArrayRef<const ProtocolDecl *> protos) const;
 
   void recordRewriteLoop(MutableTerm basepoint,
-                         RewritePath path) {
-    if (!RecordLoops)
-      return;
-
-    Loops.emplace_back(basepoint, path);
-  }
+                         RewritePath path);
 
   void propagateExplicitBits();
 
@@ -460,7 +460,7 @@ class RewriteSystem final {
   bool hadError() const;
 
   llvm::DenseMap<const ProtocolDecl *, std::vector<unsigned>>
-  getMinimizedProtocolRules(ArrayRef<const ProtocolDecl *> protos) const;
+  getMinimizedProtocolRules() const;
 
   std::vector<unsigned> getMinimizedGenericSignatureRules() const;