RequirementMachine: Move buildPropertyMap() from RewriteSystem to PropertyMap

slavapestov · slavapestov · commit 5ac83e2819d2 · 2021-11-01T22:51:26.000-04:00
Now that the property map stores a reference to the RewriteSystem,
defining the buildPropertyMap() method on PropertyMap feels cleaner,
and allows more of the property map's internals to be private.
diff --git a/lib/AST/RequirementMachine/PropertyMap.cpp b/lib/AST/RequirementMachine/PropertyMap.cpp
@@ -296,15 +296,6 @@ void PropertyMap::addProperty(
   props->addProperty(property, Context,
                      inducedRules, Debug.contains(DebugFlags::ConcreteUnification));
 }
-void PropertyMap::dump(llvm::raw_ostream &out) const {
-  out << "Property map: {\n";
-  for (const auto &props : Entries) {
-    out << "  ";
-    props->dump(out);
-    out << "\n";
-  }
-  out << "}\n";
-}
 
 /// Build the property map from all rules of the form T.[p] => T, where
 /// [p] is a property symbol.
@@ -318,19 +309,18 @@ void PropertyMap::dump(llvm::raw_ostream &out) const {
 /// left hand side has a length exceeding \p maxDepth.
 ///
 /// Otherwise, the status is CompletionResult::Success.
-std::pair<RewriteSystem::CompletionResult, unsigned>
-RewriteSystem::buildPropertyMap(PropertyMap &map,
-                                unsigned maxIterations,
-                                unsigned maxDepth) {
-  map.clear();
+std::pair<CompletionResult, unsigned>
+PropertyMap::buildPropertyMap(unsigned maxIterations,
+                              unsigned maxDepth) {
+  clear();
 
   // PropertyMap::addRule() requires that shorter rules are added
   // before longer rules, so that it can perform lookups on suffixes and call
   // PropertyBag::copyPropertiesFrom(). However, we don't have to perform a
   // full sort by term order here; a bucket sort by term length suffices.
   SmallVector<std::vector<std::pair<Term, Symbol>>, 4> properties;
 
-  for (const auto &rule : Rules) {
+  for (const auto &rule : System.getRules()) {
     if (rule.isSimplified())
       continue;
 
@@ -355,34 +345,44 @@ RewriteSystem::buildPropertyMap(PropertyMap &map,
 
   for (const auto &bucket : properties) {
     for (auto pair : bucket) {
-      map.addProperty(pair.first, pair.second, inducedRules);
+      addProperty(pair.first, pair.second, inducedRules);
     }
   }
 
   // We collect terms with fully concrete types so that we can re-use them
   // to tie off recursion in the next step.
-  map.computeConcreteTypeInDomainMap();
+  computeConcreteTypeInDomainMap();
 
   // Now, we merge concrete type rules with conformance rules, by adding
   // relations between associated type members of type parameters with
   // the concrete type witnesses in the concrete type's conformance.
-  map.concretizeNestedTypesFromConcreteParents(inducedRules);
+  concretizeNestedTypesFromConcreteParents(inducedRules);
 
   // Some of the induced rules might be trivial; only count the induced rules
   // where the left hand side is not already equivalent to the right hand side.
   unsigned addedNewRules = 0;
   for (auto pair : inducedRules) {
-    if (addRule(pair.first, pair.second)) {
+    if (System.addRule(pair.first, pair.second)) {
       ++addedNewRules;
 
-      const auto &newRule = Rules.back();
-      if (newRule.getLHS().size() > maxDepth)
+      const auto &newRule = System.getRules().back();
+      if (newRule.getDepth() > maxDepth)
         return std::make_pair(CompletionResult::MaxDepth, addedNewRules);
     }
   }
 
-  if (Rules.size() > maxIterations)
+  if (System.getRules().size() > maxIterations)
     return std::make_pair(CompletionResult::MaxIterations, addedNewRules);
 
   return std::make_pair(CompletionResult::Success, addedNewRules);
+}
+
+void PropertyMap::dump(llvm::raw_ostream &out) const {
+  out << "Property map: {\n";
+  for (const auto &props : Entries) {
+    out << "  ";
+    props->dump(out);
+    out << "\n";
+  }
+  out << "}\n";
 }
diff --git a/lib/AST/RequirementMachine/PropertyMap.h b/lib/AST/RequirementMachine/PropertyMap.h
@@ -163,8 +163,13 @@ class PropertyMap {
 
   PropertyBag *lookUpProperties(const MutableTerm &key) const;
 
+  std::pair<CompletionResult, unsigned>
+  buildPropertyMap(unsigned maxIterations,
+                   unsigned maxDepth);
+
   void dump(llvm::raw_ostream &out) const;
 
+private:
   void clear();
   void addProperty(Term key, Symbol property,
                    SmallVectorImpl<std::pair<MutableTerm, MutableTerm>> &inducedRules);
@@ -173,7 +178,6 @@ class PropertyMap {
   void concretizeNestedTypesFromConcreteParents(
                    SmallVectorImpl<std::pair<MutableTerm, MutableTerm>> &inducedRules) const;
 
-private:
   void concretizeNestedTypesFromConcreteParent(
                    Term key, RequirementKind requirementKind,
                    CanType concreteType, ArrayRef<Term> substitutions,
diff --git a/lib/AST/RequirementMachine/RequirementMachine.cpp b/lib/AST/RequirementMachine/RequirementMachine.cpp
@@ -528,16 +528,16 @@ void RequirementMachine::computeCompletion(RewriteSystem::ValidityPolicy policy)
     // Check for failure.
     auto checkCompletionResult = [&]() {
       switch (result.first) {
-      case RewriteSystem::CompletionResult::Success:
+      case CompletionResult::Success:
         break;
 
-      case RewriteSystem::CompletionResult::MaxIterations:
+      case CompletionResult::MaxIterations:
         llvm::errs() << "Generic signature " << Sig
                      << " exceeds maximum completion step count\n";
         System.dump(llvm::errs());
         abort();
 
-      case RewriteSystem::CompletionResult::MaxDepth:
+      case CompletionResult::MaxDepth:
         llvm::errs() << "Generic signature " << Sig
                      << " exceeds maximum completion depth\n";
         System.dump(llvm::errs());
@@ -553,8 +553,7 @@ void RequirementMachine::computeCompletion(RewriteSystem::ValidityPolicy policy)
     // Build the property map, which also performs concrete term
     // unification; if this added any new rules, run the completion
     // procedure again.
-    result = System.buildPropertyMap(
-        Map,
+    result = Map.buildPropertyMap(
         RequirementMachineStepLimit,
         RequirementMachineDepthLimit);
 
diff --git a/lib/AST/RequirementMachine/RewriteSystem.h b/lib/AST/RequirementMachine/RewriteSystem.h
@@ -132,6 +132,20 @@ class Rule final {
   }
 };
 
+/// Result type for RewriteSystem::computeConfluentCompletion() and
+/// PropertyMap::buildPropertyMap().
+enum class CompletionResult {
+  /// Confluent completion was computed successfully.
+  Success,
+
+  /// Maximum number of iterations reached.
+  MaxIterations,
+
+  /// Completion produced a rewrite rule whose left hand side has a length
+  /// exceeding the limit.
+  MaxDepth
+};
+
 /// A term rewrite system for working with types in a generic signature.
 ///
 /// Out-of-line methods are documented in RewriteSystem.cpp.
@@ -249,18 +263,6 @@ class RewriteSystem final {
   ///
   //////////////////////////////////////////////////////////////////////////////
 
-  enum class CompletionResult {
-    /// Confluent completion was computed successfully.
-    Success,
-
-    /// Maximum number of iterations reached.
-    MaxIterations,
-
-    /// Completion produced a rewrite rule whose left hand side has a length
-    /// exceeding the limit.
-    MaxDepth
-  };
-
   std::pair<CompletionResult, unsigned>
   computeConfluentCompletion(unsigned maxIterations,
                              unsigned maxDepth);
@@ -385,17 +387,6 @@ class RewriteSystem final {
   void computeGeneratingConformances(
       llvm::DenseSet<unsigned> &redundantConformances);
 
-  //////////////////////////////////////////////////////////////////////////////
-  ///
-  /// Property map
-  ///
-  //////////////////////////////////////////////////////////////////////////////
-
-  std::pair<CompletionResult, unsigned>
-  buildPropertyMap(PropertyMap &map,
-                   unsigned maxIterations,
-                   unsigned maxDepth);
-
   void dump(llvm::raw_ostream &out) const;
 };
 
diff --git a/lib/AST/RequirementMachine/RewriteSystemCompletion.cpp b/lib/AST/RequirementMachine/RewriteSystemCompletion.cpp
@@ -490,7 +490,7 @@ RewriteSystem::computeCriticalPair(ArrayRef<Symbol>::const_iterator from,
 /// left hand side has a length exceeding \p maxDepth.
 ///
 /// Otherwise, the status is CompletionResult::Success.
-std::pair<RewriteSystem::CompletionResult, unsigned>
+std::pair<CompletionResult, unsigned>
 RewriteSystem::computeConfluentCompletion(unsigned maxIterations,
                                           unsigned maxDepth) {
   assert(Initialized);
