RequirementMachine: Rules store uniqued Terms

slavapestov · slavapestov · commit 92ac06a25b32 · 2021-08-05T21:42:50.000-04:00
diff --git a/lib/AST/RequirementMachine/PropertyMap.cpp b/lib/AST/RequirementMachine/PropertyMap.cpp
@@ -1063,17 +1063,22 @@ RewriteSystem::buildPropertyMap(PropertyMap &map,
     if (rule.isDeleted())
       continue;
 
-    const auto &lhs = rule.getLHS();
+    auto lhs = rule.getLHS();
+    auto rhs = rule.getRHS();
 
     // Collect all rules of the form T.[p] => T where T is canonical.
     auto property = lhs.back();
     if (!property.isProperty())
       continue;
 
-    MutableTerm key(lhs.begin(), lhs.end() - 1);
-    if (key != rule.getRHS())
+    if (lhs.size() - 1 != rhs.size())
       continue;
 
+    if (!std::equal(rhs.begin(), rhs.end(), lhs.begin()))
+      continue;
+
+    MutableTerm key(rhs);
+
 #ifndef NDEBUG
     assert(!simplify(key) &&
            "Right hand side of a property rule should already be reduced");
diff --git a/lib/AST/RequirementMachine/RewriteSystem.cpp b/lib/AST/RequirementMachine/RewriteSystem.cpp
@@ -89,7 +89,7 @@ bool RewriteSystem::addRule(MutableTerm lhs, MutableTerm rhs) {
   }
 
   unsigned i = Rules.size();
-  Rules.emplace_back(lhs, rhs);
+  Rules.emplace_back(Term::get(lhs, Context), Term::get(rhs, Context));
 
   // Check if we have a rule of the form
   //
@@ -171,9 +171,11 @@ void RewriteSystem::simplifyRightHandSides() {
     if (rule.isDeleted())
       continue;
 
-    auto rhs = rule.getRHS();
-    simplify(rhs);
-    rule = Rule(rule.getLHS(), rhs);
+    MutableTerm rhs(rule.getRHS());
+    if (!simplify(rhs))
+      continue;
+
+    rule = Rule(rule.getLHS(), Term::get(rhs, Context));
   }
 
 #ifndef NDEBUG
diff --git a/lib/AST/RequirementMachine/RewriteSystem.h b/lib/AST/RequirementMachine/RewriteSystem.h
@@ -36,16 +36,16 @@ class RewriteContext;
 ///
 /// Out-of-line methods are documented in RewriteSystem.cpp.
 class Rule final {
-  MutableTerm LHS;
-  MutableTerm RHS;
+  Term LHS;
+  Term RHS;
   bool deleted;
 
 public:
-  Rule(const MutableTerm &lhs, const MutableTerm &rhs)
+  Rule(Term lhs, Term rhs)
       : LHS(lhs), RHS(rhs), deleted(false) {}
 
-  const MutableTerm &getLHS() const { return LHS; }
-  const MutableTerm &getRHS() const { return RHS; }
+  const Term &getLHS() const { return LHS; }
+  const Term &getRHS() const { return RHS; }
 
   bool apply(MutableTerm &term) const {
     return term.rewriteSubTerm(LHS, RHS);
@@ -81,12 +81,6 @@ class Rule final {
     return LHS.size();
   }
 
-  /// Partial order on rules orders rules by their left hand side.
-  int compare(const Rule &other,
-              const ProtocolGraph &protos) const {
-    return LHS.compare(other.LHS, protos);
-  }
-
   void dump(llvm::raw_ostream &out) const;
 
   friend llvm::raw_ostream &operator<<(llvm::raw_ostream &out,
diff --git a/lib/AST/RequirementMachine/RewriteSystemCompletion.cpp b/lib/AST/RequirementMachine/RewriteSystemCompletion.cpp
@@ -84,12 +84,9 @@ Symbol Symbol::prependPrefixToConcreteSubstitutions(
 /// Note that this relation is not commutative; we need to check
 /// for overlap between both (X and Y) and (Y and X).
 OverlapKind
-MutableTerm::checkForOverlap(const MutableTerm &other,
-                             MutableTerm &t,
-                             MutableTerm &v) const {
-  assert(!empty());
-  assert(!other.empty());
-
+Term::checkForOverlap(Term other,
+                      MutableTerm &t,
+                      MutableTerm &v) const {
   if (*this == other) {
     // If this term is equal to the other term, we have an overlap.
     t = MutableTerm();
@@ -410,7 +407,7 @@ RewriteSystem::computeCriticalPair(const Rule &lhs, const Rule &rhs) const {
     // Compute the term TYV.
     t.append(rhs.getRHS());
     t.append(v);
-    return std::make_pair(lhs.getRHS(), t);
+    return std::make_pair(MutableTerm(lhs.getRHS()), t);
   }
 
   case OverlapKind::Second: {
diff --git a/lib/AST/RequirementMachine/Symbol.cpp b/lib/AST/RequirementMachine/Symbol.cpp
@@ -418,6 +418,34 @@ Symbol Symbol::forConcreteType(CanType type, ArrayRef<Term> substitutions,
   return symbol;
 }
 
+/// Given that this symbol is the first symbol of a term, return the
+/// "domain" of the term.
+///
+/// - If the first symbol is a protocol symbol [P], the domain is P.
+/// - If the first symbol is an associated type symbol [P1&...&Pn],
+///   the domain is {P1, ..., Pn}.
+/// - If the first symbol is a generic parameter symbol, the domain is
+///   the empty set {}.
+/// - Anything else will assert.
+ArrayRef<const ProtocolDecl *> Symbol::getRootProtocols() const {
+  switch (getKind()) {
+  case Symbol::Kind::Protocol:
+  case Symbol::Kind::AssociatedType:
+    return getProtocols();
+
+  case Symbol::Kind::GenericParam:
+    return ArrayRef<const ProtocolDecl *>();
+
+  case Symbol::Kind::Name:
+  case Symbol::Kind::Layout:
+  case Symbol::Kind::Superclass:
+  case Symbol::Kind::ConcreteType:
+    break;
+  }
+
+  llvm_unreachable("Bad root symbol");
+}
+
 /// Linear order on symbols.
 ///
 /// First, we order different kinds as follows, from smallest to largest:
diff --git a/lib/AST/RequirementMachine/Symbol.h b/lib/AST/RequirementMachine/Symbol.h
@@ -191,6 +191,8 @@ class Symbol final {
                                 ArrayRef<Term> substitutions,
                                 RewriteContext &ctx);
 
+  ArrayRef<const ProtocolDecl *> getRootProtocols() const;
+
   int compare(Symbol other, const ProtocolGraph &protos) const;
 
   Symbol transformConcreteSubstitutions(
diff --git a/lib/AST/RequirementMachine/Term.cpp b/lib/AST/RequirementMachine/Term.cpp
@@ -106,42 +106,22 @@ Term Term::get(const MutableTerm &mutableTerm, RewriteContext &ctx) {
   return term;
 }
 
+/// Find the start of \p other in this term, returning end() if
+/// \p other does not occur as a subterm of this term.
+ArrayRef<Symbol>::iterator Term::findSubTerm(Term other) const {
+  if (other.size() > size())
+    return end();
+
+  return std::search(begin(), end(), other.begin(), other.end());
+}
+
 void Term::Storage::Profile(llvm::FoldingSetNodeID &id) const {
   id.AddInteger(Size);
 
   for (auto symbol : getElements())
     id.AddPointer(symbol.getOpaquePointer());
 }
 
-/// Returns the "domain" of this term by looking at the first symbol.
-///
-/// - If the first symbol is a protocol symbol [P], the domain is P.
-/// - If the first symbol is an associated type symbol [P1&...&Pn],
-///   the domain is {P1, ..., Pn}.
-/// - If the first symbol is a generic parameter symbol, the domain is
-///   the empty set {}.
-/// - Anything else will assert.
-ArrayRef<const ProtocolDecl *> MutableTerm::getRootProtocols() const {
-  auto symbol = *begin();
-
-  switch (symbol.getKind()) {
-  case Symbol::Kind::Protocol:
-  case Symbol::Kind::AssociatedType:
-    return symbol.getProtocols();
-
-  case Symbol::Kind::GenericParam:
-    return ArrayRef<const ProtocolDecl *>();
-
-  case Symbol::Kind::Name:
-  case Symbol::Kind::Layout:
-  case Symbol::Kind::Superclass:
-  case Symbol::Kind::ConcreteType:
-    break;
-  }
-
-  llvm_unreachable("Bad root symbol");
-}
-
 /// Shortlex order on terms.
 ///
 /// First we compare length, then perform a lexicographic comparison
@@ -170,7 +150,7 @@ int MutableTerm::compare(const MutableTerm &other,
 /// Find the start of \p other in this term, returning end() if
 /// \p other does not occur as a subterm of this term.
 decltype(MutableTerm::Symbols)::const_iterator
-MutableTerm::findSubTerm(const MutableTerm &other) const {
+MutableTerm::findSubTerm(Term other) const {
   if (other.size() > size())
     return end();
 
@@ -179,7 +159,7 @@ MutableTerm::findSubTerm(const MutableTerm &other) const {
 
 /// Non-const variant of the above.
 decltype(MutableTerm::Symbols)::iterator
-MutableTerm::findSubTerm(const MutableTerm &other) {
+MutableTerm::findSubTerm(Term other) {
   if (other.size() > size())
     return end();
 
@@ -191,8 +171,7 @@ MutableTerm::findSubTerm(const MutableTerm &other) {
 /// order on terms. Returns true if the term contained \p lhs;
 /// otherwise returns false, in which case the term remains
 /// unchanged.
-bool MutableTerm::rewriteSubTerm(const MutableTerm &lhs,
-                                 const MutableTerm &rhs) {
+bool MutableTerm::rewriteSubTerm(Term lhs, Term rhs) {
   // Find the start of lhs in this term.
   auto found = findSubTerm(lhs);
 
diff --git a/lib/AST/RequirementMachine/Term.h b/lib/AST/RequirementMachine/Term.h
@@ -77,6 +77,21 @@ class Term final {
 
   static Term get(const MutableTerm &term, RewriteContext &ctx);
 
+  OverlapKind checkForOverlap(Term other,
+                              MutableTerm &t,
+                              MutableTerm &v) const;
+
+  ArrayRef<Symbol>::iterator findSubTerm(Term other) const;
+
+  /// Returns true if this term contains, or is equal to, \p other.
+  bool containsSubTerm(Term other) const {
+    return findSubTerm(other) != end();
+  }
+
+  ArrayRef<const ProtocolDecl *> getRootProtocols() const {
+    return begin()->getRootProtocols();
+  }
+
   void dump(llvm::raw_ostream &out) const;
 
   friend bool operator==(Term lhs, Term rhs) {
@@ -144,7 +159,9 @@ class MutableTerm final {
 
   size_t size() const { return Symbols.size(); }
 
-  ArrayRef<const ProtocolDecl *> getRootProtocols() const;
+  ArrayRef<const ProtocolDecl *> getRootProtocols() const {
+    return begin()->getRootProtocols();
+  }
 
   decltype(Symbols)::const_iterator begin() const { return Symbols.begin(); }
   decltype(Symbols)::const_iterator end() const { return Symbols.end(); }
@@ -174,22 +191,11 @@ class MutableTerm final {
     return Symbols[index];
   }
 
-  decltype(Symbols)::const_iterator findSubTerm(
-      const MutableTerm &other) const;
+  decltype(Symbols)::const_iterator findSubTerm(Term other) const;
 
-  decltype(Symbols)::iterator findSubTerm(
-      const MutableTerm &other);
+  decltype(Symbols)::iterator findSubTerm(Term other);
 
-  /// Returns true if this term contains, or is equal to, \p other.
-  bool containsSubTerm(const MutableTerm &other) const {
-    return findSubTerm(other) != end();
-  }
-
-  bool rewriteSubTerm(const MutableTerm &lhs, const MutableTerm &rhs);
-
-  OverlapKind checkForOverlap(const MutableTerm &other,
-                              MutableTerm &t,
-                              MutableTerm &v) const;
+  bool rewriteSubTerm(Term lhs, Term rhs);
 
   void dump(llvm::raw_ostream &out) const;
 
