RequirementMachine: Remove merged associated types from completion

Author: slavapestov

include/swift/Basic/LangOptions.h

@@ -498,9 +498,6 @@ namespace swift {
     ASTVerifierOverrideKind ASTVerifierOverride =
         ASTVerifierOverrideKind::NoOverride;
 
-    /// Enables merged associated type support, which might go away.
-    bool RequirementMachineMergedAssociatedTypes = false;
-
     /// Enables dumping rewrite systems from the requirement machine.
     bool DumpRequirementMachine = false;
 

include/swift/Option/FrontendOptions.td

@@ -355,14 +355,6 @@ def requirement_machine_max_concrete_nesting : Joined<["-"], "requirement-machin
   Flags<[FrontendOption, HelpHidden, DoesNotAffectIncrementalBuild]>,
   HelpText<"Set the maximum concrete type nesting depth before giving up">;
 
-def disable_requirement_machine_merged_associated_types : Flag<["-"], "disable-requirement-machine-merged-associated-types">,
-  Flags<[FrontendOption, HelpHidden, DoesNotAffectIncrementalBuild]>,
-  HelpText<"Disable merged associated types">;
-
-def enable_requirement_machine_merged_associated_types : Flag<["-"], "enable-requirement-machine-merged-associated-types">,
-  Flags<[FrontendOption, HelpHidden, DoesNotAffectIncrementalBuild]>,
-  HelpText<"Enable merged associated types">;
-
 def debug_generic_signatures : Flag<["-"], "debug-generic-signatures">,
   HelpText<"Debug generic signatures">;
 

lib/AST/RequirementMachine/KnuthBendix.cpp

@@ -27,10 +27,6 @@
 // describing how this rule is derived from existing rules. See RewriteLoop.cpp
 // for a discussion of rewrite loops.
 //
-// This implementation also extends Knuth-Bendix to introduce new _generators_,
-// in addition to new relations as in the standard algorithm. See the comment at
-// the top of RewriteSystem::processMergedAssociatedTypes() for a description.
-//
 //===----------------------------------------------------------------------===//
 
 #include "swift/Basic/Range.h"
@@ -74,269 +70,6 @@ Symbol Symbol::prependPrefixToConcreteSubstitutions(
     }, ctx);
 }
 
-/// If we have two symbols [P:T] and [Q:T], produce a merged symbol:
-///
-/// - If P inherits from Q, this is just [P:T].
-/// - If Q inherits from P, this is just [Q:T].
-/// - If P and Q are unrelated, this is [P&Q:T].
-Symbol RewriteContext::mergeAssociatedTypes(Symbol lhs, Symbol rhs) {
-  auto key = std::make_pair(lhs, rhs);
-
-  auto found = MergedAssocTypes.find(key);
-  if (found != MergedAssocTypes.end())
-    return found->second;
-
-  // Check preconditions that were established by RewriteSystem::addRule().
-  assert(lhs.getKind() == Symbol::Kind::AssociatedType);
-  assert(rhs.getKind() == Symbol::Kind::AssociatedType);
-  assert(lhs.getName() == rhs.getName());
-  assert(*lhs.compare(rhs, *this) > 0);
-
-  auto protos = lhs.getProtocols();
-  auto otherProtos = rhs.getProtocols();
-
-  // This must follow from lhs > rhs.
-  assert(getProtocolSupport(protos) <= getProtocolSupport(otherProtos));
-
-  // Compute sorted and merged list of protocols, with duplicates.
-  llvm::TinyPtrVector<const ProtocolDecl *> newProtos;
-  std::merge(protos.begin(), protos.end(),
-             otherProtos.begin(), otherProtos.end(),
-             std::back_inserter(newProtos),
-             [&](const ProtocolDecl *lhs,
-                 const ProtocolDecl *rhs) -> int {
-               return compareProtocols(lhs, rhs) < 0;
-             });
-
-  // Prune duplicates and protocols that are inherited by other
-  // protocols.
-  llvm::TinyPtrVector<const ProtocolDecl *> minimalProtos;
-  for (const auto *newProto : newProtos) {
-    auto inheritsFrom = [&](const ProtocolDecl *thisProto) {
-      if (thisProto == newProto)
-        return true;
-
-      const auto &inherited = getInheritedProtocols(thisProto);
-      return std::find(inherited.begin(),
-                       inherited.end(),
-                       newProto) != inherited.end();
-    };
-
-    if (std::find_if(minimalProtos.begin(), minimalProtos.end(),
-                     inheritsFrom)
-        == minimalProtos.end()) {
-      minimalProtos.push_back(newProto);
-    }
-  }
-
-  // The two input sets are minimal already, so the merged set
-  // should have at least as many elements as the smallest
-  // input set.
-  assert(minimalProtos.size() >= std::min(protos.size(), otherProtos.size()));
-
-  // The merged set cannot contain more elements than the union
-  // of the two sets.
-  assert(minimalProtos.size() <= protos.size() + otherProtos.size());
-
-  auto result = Symbol::forAssociatedType(minimalProtos, lhs.getName(), *this);
-  auto inserted = MergedAssocTypes.insert(std::make_pair(key, result));
-  assert(inserted.second);
-  (void) inserted;
-
-  return result;
-}
-
-/// Consider the following example:
-///
-///   protocol P1 { associatedtype T : P1 }
-///   protocol P2 { associatedtype T : P2 }
-///   struct G<T : P1 & P2> {}
-///
-/// We start with these rewrite rules:
-///
-///   [P1].T => [P1:T]
-///   [P2].T => [P2:T]
-///   [P1:T].[P1] => [P1:T]
-///   [P2:T].[P1] => [P2:T]
-///   τ_0_0.[P1] => τ_0_0
-///   τ_0_0.[P2] => τ_0_0
-///   τ_0_0.T => τ_0_0.[P1:T]
-///   τ_0_0.[P2:T] => τ_0_0.[P1:T]
-///
-/// The completion procedure ends up adding an infinite series of rules of the
-/// form
-///
-///   τ_0_0.[P1:T].[P2]                 => τ_0_0.[P1:T]
-///   τ_0_0.[P1:T].[P2:T]               => τ_0_0.[P1:T].[P1:T]
-///
-///   τ_0_0.[P1:T].[P1:T].[P2]          => τ_0_0.[P1:T].[P1:T]
-///   τ_0_0.[P1:T].[P1:T].[P2:T]        => τ_0_0.[P1:T].[P1:T].[P1:T]
-///
-///   τ_0_0.[P1:T].[P1:T].[P1:T].[P2]   => τ_0_0.[P1:T].[P1:T].[P1.T]
-///   τ_0_0.[P1:T].[P1:T].[P1:T].[P2:T] => τ_0_0.[P1:T].[P1:T].[P1:T].[P1.T]
-///
-/// The difficulty here stems from the fact that an arbitrary sequence of
-/// [P1:T] following a τ_0_0 is known to conform to P2, but P1:T itself
-/// does not conform to P2.
-///
-/// We use a heuristic to compute a completion in this case by using
-/// merged associated type terms.
-///
-/// The key is the following rewrite rule:
-///
-///   τ_0_0.[P2:T] => τ_0_0.[P1:T]
-///
-/// When we add this rule, we introduce a new merged symbol [P1&P2:T] and
-/// a new rule:
-///
-///   τ_0_0.[P1:T] => τ_0_0.[P1&P2:T]
-///
-/// We also look for any existing rules of the form [P1:T].[Q] => [P1:T]
-/// or [P2:T].[Q] => [P2:T], and introduce a new rule:
-///
-///   [P1&P2:T].[Q] => [P1&P2:T]
-///
-/// In the above example, we have such a rule for Q == P1 and Q == P2, so
-/// in total we end up adding the following four rules:
-///
-///   τ_0_0.[P1:T] => τ_0_0.[P1&P2:T]
-///   [P1&P2:T].[P1] => [P1&P2:T]
-///   [P1&P2:T].[P2] => [P1&P2:T]
-///
-/// Intuitively, since the conformance requirements on the merged term
-/// are not prefixed by the root τ_0_0, they apply at any level; we've
-/// "tied off" the recursion, and the rewrite system is now convergent.
-void RewriteSystem::processMergedAssociatedTypes() {
-  if (MergedAssociatedTypes.empty())
-    return;
-
-  unsigned i = 0;
-
-  // Chase the end of the vector, since addRule() might add new elements below.
-  while (i < MergedAssociatedTypes.size()) {
-    // Copy the entry out, since addRule() might add new elements below.
-    auto entry = MergedAssociatedTypes[i++];
-
-    if (Debug.contains(DebugFlags::Merge)) {
-      llvm::dbgs() << "## Processing associated type merge with ";
-      llvm::dbgs() << entry.rhs << ", ";
-      llvm::dbgs() << entry.lhsSymbol << ", ";
-      llvm::dbgs() << entry.mergedSymbol << "\n";
-    }
-
-    // If we have X.[P2:T] => Y.[P1:T], add a new rule:
-    // X.[P1:T] => X.[P1&P2:T]
-    MutableTerm lhs(entry.rhs);
-
-    // Build the term X.[P1&P2:T].
-    MutableTerm rhs(entry.rhs);
-    rhs.back() = entry.mergedSymbol;
-
-    // Add the rule X.[P1:T] => X.[P1&P2:T].
-    addRule(lhs, rhs);
-
-    // Collect new rules here so that we're not adding rules while traversing
-    // the trie.
-    SmallVector<std::pair<MutableTerm, MutableTerm>, 2> inducedRules;
-
-    // Look for conformance requirements on [P1:T] and [P2:T].
-    auto visitRule = [&](unsigned ruleID) {
-      const auto &otherRule = getRule(ruleID);
-      const auto &otherLHS = otherRule.getLHS();
-      if (otherLHS.size() == 2 &&
-          otherLHS[1].getKind() == Symbol::Kind::Protocol) {
-        if (otherLHS[0] == entry.lhsSymbol ||
-            otherLHS[0] == entry.rhs.back()) {
-          // We have a rule of the form
-          //
-          //   [P1:T].[Q] => [P1:T]
-          //
-          // or
-          //
-          //   [P2:T].[Q] => [P2:T]
-          if (Debug.contains(DebugFlags::Merge)) {
-            llvm::dbgs() << "### Lifting conformance rule " << otherRule << "\n";
-          }
-
-          // We know that [P1:T] or [P2:T] conforms to Q, therefore the
-          // merged type [P1&P2:T] must conform to Q as well. Add a new rule
-          // of the form:
-          //
-          //   [P1&P2:T].[Q] => [P1&P2:T]
-          //
-          MutableTerm newLHS;
-          newLHS.add(entry.mergedSymbol);
-          newLHS.add(otherLHS[1]);
-
-          MutableTerm newRHS;
-          newRHS.add(entry.mergedSymbol);
-
-          inducedRules.emplace_back(newLHS, newRHS);
-        }
-      }
-    };
-
-    // Visit rhs first to preserve the ordering of protocol requirements in the
-    // the property map. This is just for aesthetic purposes in the debug dump,
-    // it doesn't change behavior.
-    Trie.findAll(entry.rhs.back(), visitRule);
-    Trie.findAll(entry.lhsSymbol, visitRule);
-
-    // Now add the new rules.
-    for (const auto &pair : inducedRules)
-      addRule(pair.first, pair.second);
-  }
-
-  MergedAssociatedTypes.clear();
-}
-
-/// Check if we have a rule of the form
-///
-///   X.[P1:T] => X.[P2:T]
-///
-/// If so, record this rule for later. We'll try to merge the associated
-/// types in RewriteSystem::processMergedAssociatedTypes().
-void RewriteSystem::checkMergedAssociatedType(Term lhs, Term rhs) {
-  // FIXME: Figure out 3-cell representation for merged associated types
-  if (RecordLoops ||
-      !Context.getASTContext().LangOpts.RequirementMachineMergedAssociatedTypes)
-    return;
-
-  if (lhs.size() == rhs.size() &&
-      std::equal(lhs.begin(), lhs.end() - 1, rhs.begin()) &&
-      lhs.back().getKind() == Symbol::Kind::AssociatedType &&
-      rhs.back().getKind() == Symbol::Kind::AssociatedType &&
-      lhs.back().getName() == rhs.back().getName()) {
-    if (Debug.contains(DebugFlags::Merge)) {
-      llvm::dbgs() << "## Associated type merge candidate ";
-      llvm::dbgs() << lhs << " => " << rhs << "\n\n";
-    }
-
-    auto mergedSymbol = Context.mergeAssociatedTypes(lhs.back(), rhs.back());
-    if (Debug.contains(DebugFlags::Merge)) {
-      llvm::dbgs() << "### Merged symbol " << mergedSymbol << "\n";
-    }
-
-    // We must have mergedSymbol <= rhs < lhs, therefore mergedSymbol != lhs.
-    assert(lhs.back() != mergedSymbol &&
-           "Left hand side should not already end with merged symbol?");
-    assert(*mergedSymbol.compare(rhs.back(), Context) <= 0);
-    assert(*rhs.back().compare(lhs.back(), Context) < 0);
-
-    // If the merge didn't actually produce a new symbol, there is nothing else
-    // to do.
-    if (rhs.back() == mergedSymbol) {
-      if (Debug.contains(DebugFlags::Merge)) {
-        llvm::dbgs() << "### Skipping\n";
-      }
-
-      return;
-    }
-
-    MergedAssociatedTypes.push_back({rhs, lhs.back(), mergedSymbol});
-  }
-}
-
 /// Compute a critical pair from the left hand sides of two rewrite rules,
 /// where \p rhs begins at \p from, which must be an iterator pointing
 /// into \p lhs.
@@ -629,16 +362,7 @@ RewriteSystem::computeConfluentCompletion(unsigned maxRuleCount,
 
     simplifyRightHandSides();
     simplifyLeftHandSideSubstitutions();
-
-    // If the added rules merged any associated types, process the merges now
-    // before we continue with the completion procedure. This is important
-    // to perform incrementally since merging is required to repair confluence
-    // violations.
-    processMergedAssociatedTypes();
   } while (again);
 
-  assert(MergedAssociatedTypes.empty() &&
-         "Should have processed all merge candidates");
-
   return std::make_pair(CompletionResult::Success, 0);
 }

lib/AST/RequirementMachine/RewriteContext.h

@@ -182,8 +182,6 @@ class RewriteContext final {
 
   AssociatedTypeDecl *getAssociatedTypeForSymbol(Symbol symbol);
 
-  Symbol mergeAssociatedTypes(Symbol lhs, Symbol rhs);
-
   //////////////////////////////////////////////////////////////////////////////
   ///
   /// Construction of requirement machines for connected components in the

lib/AST/RequirementMachine/RewriteSystem.cpp

@@ -449,9 +449,7 @@ bool RewriteSystem::addRule(MutableTerm lhs, MutableTerm rhs,
 
   unsigned newRuleID = Rules.size();
 
-  auto uniquedLHS = Term::get(lhs, Context);
-  auto uniquedRHS = Term::get(rhs, Context);
-  Rules.emplace_back(uniquedLHS, uniquedRHS);
+  Rules.emplace_back(Term::get(lhs, Context), Term::get(rhs, Context));
 
   if (path) {
     // We have a rewrite path from the simplified lhs to the simplified rhs;
@@ -482,8 +480,6 @@ bool RewriteSystem::addRule(MutableTerm lhs, MutableTerm rhs,
     abort();
   }
 
-  checkMergedAssociatedType(uniquedLHS, uniquedRHS);
-
   // Tell the caller that we added a new rule.
   return true;
 }

lib/AST/RequirementMachine/RewriteSystem.h

@@ -359,29 +359,6 @@ class RewriteSystem final {
       std::vector<CriticalPair> &pairs,
       std::vector<RewriteLoop> &loops) const;
 
-  /// Constructed from a rule of the form X.[P2:T] => X.[P1:T] by
-  /// checkMergedAssociatedType().
-  struct MergedAssociatedType {
-    /// The *right* hand side of the original rule, X.[P1:T].
-    Term rhs;
-
-    /// The associated type symbol appearing at the end of the *left*
-    /// hand side of the original rule, [P2:T].
-    Symbol lhsSymbol;
-
-    /// The merged associated type symbol, [P1&P2:T].
-    Symbol mergedSymbol;
-  };
-
-  /// A list of pending terms for the associated type merging completion
-  /// heuristic. Entries are added by checkMergedAssociatedType(), and
-  /// consumed in processMergedAssociatedTypes().
-  std::vector<MergedAssociatedType> MergedAssociatedTypes;
-
-  void processMergedAssociatedTypes();
-
-  void checkMergedAssociatedType(Term lhs, Term rhs);
-
   //////////////////////////////////////////////////////////////////////////////
   ///
   /// Relations are "pseudo-rules" introduced by the property map

lib/Frontend/CompilerInvocation.cpp

@@ -914,12 +914,6 @@ static bool ParseLangArgs(LangOptions &Opts, ArgList &Args,
                      A->getAsString(Args), A->getValue());
   }
 
-  if (auto A = Args.getLastArg(OPT_enable_requirement_machine_merged_associated_types,
-                               OPT_disable_requirement_machine_merged_associated_types)) {
-    Opts.RequirementMachineMergedAssociatedTypes
-      = A->getOption().matches(OPT_enable_requirement_machine_merged_associated_types);
-  }
-
   Opts.DumpRequirementMachine = Args.hasArg(
       OPT_dump_requirement_machine);
   Opts.AnalyzeRequirementMachine = Args.hasArg(