[RequirementMachine] Propagate explicit requirement IDs from redundant

rules in a separate pass after homotopy reduction.

RewriteSystem::propagateExplicitBits was too eager in propagating
IDs from explicit rules within rewrite loops, which resulted in bogus
redundancy warnings when the canonical form of an explicit rule was
given a different requirement ID. Instead, propagate requirement IDs
after homotopy reduction when redundant rules are computed and rewrite
loops are simplified.

Author: hborla

lib/AST/RequirementMachine/Debug.h

@@ -64,7 +64,11 @@ enum class DebugFlags : unsigned {
   RedundantRulesDetail = (1<<13),
 
   /// Print debug output from the concrete contraction pre-processing pass.
-  ConcreteContraction = (1<<14)
+  ConcreteContraction = (1<<14),
+
+  /// Print debug output from propagating explicit requirement
+  /// IDs from redundant rules.
+  PropagateRequirementIDs = (1<<15),
 };
 
 using DebugOptions = OptionSet<DebugFlags>;

lib/AST/RequirementMachine/HomotopyReduction.cpp

@@ -195,24 +195,100 @@ void RewriteSystem::propagateExplicitBits() {
       loop.findRulesAppearingOnceInEmptyContext(*this);
 
     bool sawExplicitRule = false;
-    Optional<unsigned> requirementID = None;
 
     for (unsigned ruleID : rulesInEmptyContext) {
       const auto &rule = getRule(ruleID);
-      if (rule.isExplicit()) {
+      if (rule.isExplicit())
         sawExplicitRule = true;
-        requirementID = rule.getRequirementID();
-        break;
-      }
     }
     if (sawExplicitRule) {
       for (unsigned ruleID : rulesInEmptyContext) {
         auto &rule = getRule(ruleID);
         if (!rule.isPermanent() && !rule.isExplicit())
-          rule.markExplicit(requirementID);
+          rule.markExplicit(None);
+      }
+    }
+  }
+}
+
+/// Propagate requirement IDs from redundant rules to their
+/// replacements that appear once in empty context.
+void RewriteSystem::propagateRedundantRequirementIDs() {
+  if (Debug.contains(DebugFlags::PropagateRequirementIDs)) {
+    llvm::dbgs() << "\nPropagating requirement IDs: {";
+  }
+
+  for (auto ruleAndReplacement : RedundantRules) {
+    auto ruleID = ruleAndReplacement.first;
+    auto rewritePath = ruleAndReplacement.second;
+    auto &rule = Rules[ruleID];
+
+    auto requirementID = rule.getRequirementID();
+    if (!requirementID.hasValue())
+      continue;
+
+    // FIXME: This code is almost identical to `RewriteLoop::recompute`.
+
+    // Rules appearing in empty context (possibly more than once).
+    llvm::SmallDenseSet<unsigned, 2> rulesInEmptyContext;
+    // The number of times each rule appears (with or without context).
+    llvm::SmallDenseMap<unsigned, unsigned, 2> ruleFrequency;
+
+    RewritePathEvaluator evaluator(MutableTerm(rule.getLHS()));
+    for (auto step : rewritePath) {
+      switch (step.Kind) {
+      case RewriteStep::Rule: {
+        if (!step.isInContext() && !evaluator.isInContext())
+          rulesInEmptyContext.insert(step.getRuleID());
+
+        ++ruleFrequency[step.getRuleID()];
+        break;
+      }
+
+      case RewriteStep::LeftConcreteProjection:
+      case RewriteStep::Decompose:
+      case RewriteStep::PrefixSubstitutions:
+      case RewriteStep::Shift:
+      case RewriteStep::Relation:
+      case RewriteStep::DecomposeConcrete:
+      case RewriteStep::RightConcreteProjection:
+        break;
+      }
+
+      evaluator.apply(step, *this);
+    }
+
+    // Collect all rules that we saw exactly once in empty context.
+    SmallVector<unsigned, 1> rulesOnceInEmptyContext;
+    for (auto rule : rulesInEmptyContext) {
+      auto found = ruleFrequency.find(rule);
+      assert(found != ruleFrequency.end());
+
+      if (found->second == 1)
+        rulesOnceInEmptyContext.push_back(rule);
+    }
+
+    for (auto ruleID : rulesOnceInEmptyContext) {
+      auto &replacement = Rules[ruleID];
+      if (!replacement.isPermanent() &&
+          !replacement.getRequirementID().hasValue()) {
+        if (Debug.contains(DebugFlags::PropagateRequirementIDs)) {
+          llvm::dbgs() << "\n- propagating ID = "
+            << requirementID
+            << "\n  from ";
+          rule.dump(llvm::dbgs());
+          llvm::dbgs() << "\n  to ";
+          replacement.dump(llvm::dbgs());
+        }
+
+        replacement.markExplicit(requirementID);
       }
     }
   }
+
+  if (Debug.contains(DebugFlags::PropagateRequirementIDs)) {
+    llvm::dbgs() << "\n}\n";
+  }
 }
 
 /// After propagating the 'explicit' bit on rules, process pairs of
@@ -658,7 +734,7 @@ void RewriteSystem::performHomotopyReduction(
 
     // If no redundant rules remain which can be eliminated by this pass, stop.
     if (!optPair)
-      return;
+      break;
 
     unsigned loopID = optPair->first;
     unsigned ruleID = optPair->second;
@@ -683,6 +759,8 @@ void RewriteSystem::performHomotopyReduction(
 
     deleteRule(ruleID, replacementPath);
   }
+
+  propagateRedundantRequirementIDs();
 }
 
 void RewriteSystem::normalizeRedundantRules() {

lib/AST/RequirementMachine/RewriteContext.cpp

@@ -42,6 +42,7 @@ static DebugOptions parseDebugFlags(StringRef debugFlags) {
       .Case("redundant-rules", DebugFlags::RedundantRules)
       .Case("redundant-rules-detail", DebugFlags::RedundantRulesDetail)
       .Case("concrete-contraction", DebugFlags::ConcreteContraction)
+      .Case("propagate-requirement-ids", DebugFlags::PropagateRequirementIDs)
       .Default(None);
     if (!flag) {
       llvm::errs() << "Unknown debug flag in -debug-requirement-machine "

lib/AST/RequirementMachine/RewriteSystem.cpp

@@ -770,7 +770,7 @@ void RewriteSystem::dump(llvm::raw_ostream &out) const {
     out << "- " << rule;
     if (auto ID = rule.getRequirementID()) {
       auto requirement = WrittenRequirements[*ID];
-      out << ", ";
+      out << ", ID: " << *ID << ", ";
       requirement.req.dump(out);
       out << " at ";
       requirement.loc.print(out, Context.getASTContext().SourceMgr);

lib/AST/RequirementMachine/RewriteSystem.h

@@ -177,7 +177,9 @@ class Rule final {
   }
 
   void markExplicit(Optional<unsigned> requirementID) {
-    assert(!Explicit && !Permanent &&
+    assert((!Explicit || requirementID.hasValue()) &&
+           "Rule is already explicit");
+    assert(!Permanent &&
            "Permanent and explicit are mutually exclusive");
     this->requirementID = requirementID;
     Explicit = true;
@@ -505,6 +507,8 @@ class RewriteSystem final {
 
   void propagateExplicitBits();
 
+  void propagateRedundantRequirementIDs();
+
   void processConflicts();
 
   Optional<std::pair<unsigned, unsigned>>

test/Generics/requirement_machine_diagnostics.swift

@@ -152,8 +152,28 @@ protocol P7 : P6 {
   associatedtype AssocP7: P6
 }
 
-extension P7 where AssocP6.Element : P6,
-        AssocP7.AssocP6.Element : P6, // expected-warning{{redundant conformance constraint 'Self.AssocP7.AssocP6.Element' : 'P6'}}
+extension P7 where AssocP6.Element : P6, // expected-warning{{redundant conformance constraint 'Self.AssocP6.Element' : 'P6'}}
+        AssocP7.AssocP6.Element : P6,
         AssocP6.Element == AssocP7.AssocP6.Element {
   func nestedSameType1() { }
 }
+
+protocol P8 {
+  associatedtype A
+  associatedtype B
+}
+
+protocol P9 : P8 {
+  associatedtype A
+  associatedtype B
+}
+
+protocol P10 {
+  associatedtype A
+  associatedtype C
+}
+
+class X3 { }
+
+func sameTypeConcrete2<T : P9 & P10>(_: T) where T.B : X3, T.C == T.B, T.C == X3 { }
+// expected-warning@-1{{redundant superclass constraint 'T.B' : 'X3'}}