RequirementMachine: Refactor concrete substitution pass

Author: slavapestov

lib/AST/RequirementMachine/PropertyMap.cpp

@@ -221,6 +221,36 @@ void PropertyBag::copyPropertiesFrom(const PropertyBag *next,
   }
 }
 
+Symbol PropertyBag::concretelySimplifySubstitution(const MutableTerm &mutTerm,
+                                                   RewriteContext &ctx,
+                                                   RewritePath *path) const {
+  // The property map entry might apply to a suffix of the substitution
+  // term, so prepend the appropriate prefix to its own substitutions.
+  auto prefix = getPrefixAfterStrippingKey(mutTerm);
+  auto concreteSymbol =
+    ConcreteType->prependPrefixToConcreteSubstitutions(
+        prefix, ctx);
+
+  // If U.V is the substitution term and V is the property map key,
+  // apply the rewrite step U.(V => V.[concrete: C]) followed by
+  // prepending the prefix U to each substitution in the concrete type
+  // symbol if |U| > 0.
+  if (path) {
+    path->add(RewriteStep::forRewriteRule(/*startOffset=*/prefix.size(),
+                                          /*endOffset=*/0,
+                                          /*ruleID=*/*ConcreteTypeRule,
+                                          /*inverse=*/true));
+
+    if (!prefix.empty()) {
+      path->add(RewriteStep::forPrefixSubstitutions(/*length=*/prefix.size(),
+                                                    /*endOffset=*/0,
+                                                    /*inverse=*/false));
+    }
+  }
+
+  return concreteSymbol;
+}
+
 void PropertyBag::verify(const RewriteSystem &system) const {
 #ifndef NDEBUG
   assert(ConformsTo.size() == ConformsToRules.size());
@@ -408,7 +438,7 @@ void PropertyMap::buildPropertyMap() {
 
   // Finally, a post-processing pass to reduce substitutions down to
   // concrete types.
-  concretelySimplifyLeftHandSideSubstitutions();
+  System.simplifyLeftHandSideSubstitutions(this);
 
   // Check invariants of the constructed property map.
   verify();

lib/AST/RequirementMachine/PropertyMap.h

@@ -116,6 +116,8 @@ class PropertyBag {
     return found->second;
   }
 
+  MutableTerm getPrefixAfterStrippingKey(const MutableTerm &lookupTerm) const;
+
 public:
   Term getKey() const { return Key; }
   void dump(llvm::raw_ostream &out) const;
@@ -157,10 +159,12 @@ class PropertyBag {
   llvm::TinyPtrVector<const ProtocolDecl *>
   getConformsToExcludingSuperclassConformances() const;
 
-  MutableTerm getPrefixAfterStrippingKey(const MutableTerm &lookupTerm) const;
-
   AssociatedTypeDecl *getAssociatedType(Identifier name);
 
+  Symbol concretelySimplifySubstitution(const MutableTerm &mutTerm,
+                                        RewriteContext &ctx,
+                                        RewritePath *path) const;
+
   void verify(const RewriteSystem &system) const;
 };
 
@@ -320,12 +324,6 @@ class PropertyMap {
     RequirementKind requirementKind,
     Symbol concreteConformanceSymbol) const;
 
-  Optional<unsigned> concretelySimplifySubstitutions(Term baseTerm,
-                                                     Symbol symbol,
-                                                     RewritePath *path) const;
-
-  void concretelySimplifyLeftHandSideSubstitutions() const;
-
   void verify() const;
 };
 

lib/AST/RequirementMachine/RewriteSystem.h

@@ -318,6 +318,10 @@ class RewriteSystem final {
 
   bool simplifySubstitutions(Symbol &symbol, RewritePath *path=nullptr) const;
 
+  Optional<unsigned>
+  simplifySubstitutions(Term baseTerm, Symbol symbol, const PropertyMap *map,
+                        RewritePath *path);
+
   //////////////////////////////////////////////////////////////////////////////
   ///
   /// Completion
@@ -337,6 +341,8 @@ class RewriteSystem final {
 
   void simplifyLeftHandSideSubstitutions();
 
+  void simplifyLeftHandSideSubstitutions(const PropertyMap *map);
+
   enum ValidityPolicy {
     AllowInvalidRequirements,
     DisallowInvalidRequirements

lib/AST/RequirementMachine/SimplifySubstitutions.cpp

@@ -128,17 +128,21 @@ void RewriteSystem::simplifyLeftHandSideSubstitutions() {
   }
 }
 
-/// Similar to RewriteSystem::simplifySubstitutions(), but also replaces type
-/// parameters with concrete types and builds a type difference describing
-/// the transformation.
+/// Simplify terms appearing in the substitutions of the last symbol of \p term,
+/// which must be a superclass or concrete type symbol.
+///
+/// Additionally, if \p map is non-null, any terms which become concrete types
+/// will cause the corresponding generic parameter in the concrete type symbol
+/// to be replaced.
 ///
 /// Returns None if the concrete type symbol cannot be simplified further.
 ///
 /// Otherwise returns an index which can be passed to
 /// RewriteSystem::getTypeDifference().
 Optional<unsigned>
-PropertyMap::concretelySimplifySubstitutions(Term baseTerm, Symbol symbol,
-                                             RewritePath *path) const {
+RewriteSystem::simplifySubstitutions(Term baseTerm, Symbol symbol,
+                                     const PropertyMap *map,
+                                     RewritePath *path) {
   assert(symbol.hasSubstitutions());
 
   // Fast path if the type is fully concrete.
@@ -181,39 +185,19 @@ PropertyMap::concretelySimplifySubstitutions(Term baseTerm, Symbol symbol,
     // has to iterate until fixed point anyway.
     //
     // This should be rare in practice.
-    if (System.simplify(mutTerm, path)) {
+    if (simplify(mutTerm, path)) {
       // Record a mapping from this substitution to the simplified term.
       sameTypes.emplace_back(index, Term::get(mutTerm, Context));
-    } else {
-      auto *props = lookUpProperties(mutTerm);
 
-      if (props && props->ConcreteType) {
-        // The property map entry might apply to a suffix of the substitution
-        // term, so prepend the appropriate prefix to its own substitutions.
-        auto prefix = props->getPrefixAfterStrippingKey(mutTerm);
-        auto concreteSymbol =
-          props->ConcreteType->prependPrefixToConcreteSubstitutions(
-              prefix, Context);
+    } else if (map) {
+      auto *props = map->lookUpProperties(mutTerm);
+
+      if (props && props->isConcreteType()) {
+        auto concreteSymbol = props->concretelySimplifySubstitution(
+            mutTerm, Context, path);
 
         // Record a mapping from this substitution to the concrete type.
         concreteTypes.emplace_back(index, concreteSymbol);
-
-        // If U.V is the substitution term and V is the property map key,
-        // apply the rewrite step U.(V => V.[concrete: C]) followed by
-        // prepending the prefix U to each substitution in the concrete type
-        // symbol if |U| > 0.
-        if (path) {
-          path->add(RewriteStep::forRewriteRule(/*startOffset=*/prefix.size(),
-                                                /*endOffset=*/0,
-                                                /*ruleID=*/*props->ConcreteTypeRule,
-                                                /*inverse=*/true));
-
-          if (!prefix.empty()) {
-            path->add(RewriteStep::forPrefixSubstitutions(/*length=*/prefix.size(),
-                                                          /*endOffset=*/0,
-                                                          /*inverse=*/false));
-          }
-        }
       }
     }
   }
@@ -240,7 +224,7 @@ PropertyMap::concretelySimplifySubstitutions(Term baseTerm, Symbol symbol,
                                         Context);
   assert(difference.LHS != difference.RHS);
 
-  unsigned differenceID = System.recordTypeDifference(difference);
+  unsigned differenceID = recordTypeDifference(difference);
 
   // All simplified substitutions are now on the primary stack. Collect them to
   // produce the new term.
@@ -252,9 +236,15 @@ PropertyMap::concretelySimplifySubstitutions(Term baseTerm, Symbol symbol,
   return differenceID;
 }
 
-void PropertyMap::concretelySimplifyLeftHandSideSubstitutions() const {
-  for (unsigned ruleID = 0, e = System.getRules().size(); ruleID < e; ++ruleID) {
-    auto &rule = System.getRule(ruleID);
+/// Simplify substitution terms in superclass, concrete type and concrete
+/// conformance symbols.
+///
+/// During completion, \p map will be null. After completion, the property map
+/// is built, and a final simplification pass is performed with \p map set to
+/// the new property map.
+void RewriteSystem::simplifyLeftHandSideSubstitutions(const PropertyMap *map) {
+  for (unsigned ruleID = 0, e = Rules.size(); ruleID < e; ++ruleID) {
+    auto &rule = getRule(ruleID);
     if (rule.isLHSSimplified() ||
         rule.isRHSSimplified() ||
         rule.isSubstitutionSimplified())
@@ -268,14 +258,13 @@ void PropertyMap::concretelySimplifyLeftHandSideSubstitutions() const {
 
     RewritePath path;
 
-    auto differenceID = concretelySimplifySubstitutions(
-        rule.getRHS(), symbol, &path);
+    auto differenceID = simplifySubstitutions(rule.getRHS(), symbol, map, &path);
     if (!differenceID)
       continue;
 
     rule.markSubstitutionSimplified();
 
-    auto difference = System.getTypeDifference(*differenceID);
+    auto difference = getTypeDifference(*differenceID);
     assert(difference.LHS == symbol);
 
     // If the original rule is (T.[concrete: C] => T) and [concrete: C'] is
@@ -295,6 +284,6 @@ void PropertyMap::concretelySimplifyLeftHandSideSubstitutions() const {
     MutableTerm lhs(rhs);
     lhs.add(difference.RHS);
 
-    System.addRule(lhs, rhs, &path);
+    addRule(lhs, rhs, &path);
   }
 }