Merge pull request #41435 from slavapestov/rqm-fixes-for-inferred-signatures-enabled

RequirementMachine: Fix assorted issues when running validation tests with -requirement-machine-inferred-signatures=verify

Author: slavapestov

lib/AST/RequirementMachine/ConcreteTypeWitness.cpp

@@ -47,15 +47,16 @@ void PropertyMap::concretizeNestedTypesFromConcreteParents() {
         llvm::dbgs() << "- via concrete type requirement\n";
       }
 
-      concretizeNestedTypesFromConcreteParent(
-          props->getKey(),
-          RequirementKind::SameType,
-          *props->ConcreteTypeRule,
-          props->ConcreteType->getConcreteType(),
-          props->ConcreteType->getSubstitutions(),
-          props->ConformsToRules,
-          props->ConformsTo,
-          props->ConcreteConformances);
+      for (auto pair : props->ConcreteTypeRules) {
+        concretizeNestedTypesFromConcreteParent(
+            props->getKey(),
+            RequirementKind::SameType,
+            pair.second,
+            pair.first.getConcreteType(),
+            pair.first.getSubstitutions(),
+            props->ConformsToRules,
+            props->ConformsTo);
+      }
     }
 
     if (props->hasSuperclassBound()) {
@@ -64,15 +65,16 @@ void PropertyMap::concretizeNestedTypesFromConcreteParents() {
       }
 
       const auto &superclassReq = props->getSuperclassRequirement();
-      concretizeNestedTypesFromConcreteParent(
-          props->getKey(),
-          RequirementKind::Superclass,
-          *superclassReq.SuperclassRule,
-          superclassReq.SuperclassType->getConcreteType(),
-          superclassReq.SuperclassType->getSubstitutions(),
-          props->ConformsToRules,
-          props->ConformsTo,
-          props->SuperclassConformances);
+      for (auto pair : superclassReq.SuperclassRules) {
+        concretizeNestedTypesFromConcreteParent(
+            props->getKey(),
+            RequirementKind::Superclass,
+            pair.second,
+            pair.first.getConcreteType(),
+            pair.first.getSubstitutions(),
+            props->ConformsToRules,
+            props->ConformsTo);
+      }
     }
   }
 }
@@ -115,8 +117,7 @@ void PropertyMap::concretizeNestedTypesFromConcreteParent(
     CanType concreteType,
     ArrayRef<Term> substitutions,
     ArrayRef<unsigned> conformsToRules,
-    ArrayRef<const ProtocolDecl *> conformsTo,
-    llvm::TinyPtrVector<ProtocolConformance *> &conformances) {
+    ArrayRef<const ProtocolDecl *> conformsTo) {
   assert(requirementKind == RequirementKind::SameType ||
          requirementKind == RequirementKind::Superclass);
   assert(conformsTo.size() == conformsToRules.size());
@@ -132,10 +133,8 @@ void PropertyMap::concretizeNestedTypesFromConcreteParent(
     // entry for this key's suffix.
     auto pair = std::make_pair(concreteRuleID, conformanceRuleID);
     auto found = ConcreteConformances.find(pair);
-    if (found != ConcreteConformances.end()) {
-      conformances.push_back(found->second);
+    if (found != ConcreteConformances.end())
       continue;
-    }
 
     // FIXME: Either remove the ModuleDecl entirely from conformance lookup,
     // or pass the correct one down in here.
@@ -183,10 +182,6 @@ void PropertyMap::concretizeNestedTypesFromConcreteParent(
     assert(inserted.second);
     (void) inserted;
 
-    // Record the conformance for use by
-    // PropertyBag::getConformsToExcludingSuperclassConformances().
-    conformances.push_back(concrete);
-
     auto concreteConformanceSymbol = Symbol::forConcreteConformance(
         concreteType, substitutions, proto, Context);
 

lib/AST/RequirementMachine/GenericSignatureQueries.cpp

@@ -53,7 +53,7 @@ RequirementMachine::getLocalRequirements(
     result.superclass = props->getSuperclassBound({}, term, Map);
   }
 
-  for (const auto *proto : props->getConformsToExcludingSuperclassConformances())
+  for (const auto *proto : props->getConformsTo())
     result.protos.push_back(const_cast<ProtocolDecl *>(proto));
 
   result.layout = props->getLayoutConstraint();

lib/AST/RequirementMachine/HomotopyReduction.cpp

@@ -198,6 +198,41 @@ void RewriteSystem::propagateExplicitBits() {
   }
 }
 
+/// After propagating the 'explicit' bit on rules, process pairs of
+/// conflicting rules, marking one or both of the rules as conflicting,
+/// which instructs minimization to drop them.
+void RewriteSystem::processConflicts() {
+  for (auto pair : ConflictingRules) {
+    auto existingRuleID = pair.first;
+    auto newRuleID = pair.second;
+
+    auto *existingRule = &getRule(existingRuleID);
+    auto *newRule = &getRule(newRuleID);
+
+    auto existingKind = existingRule->isPropertyRule()->getKind();
+    auto newKind = newRule->isPropertyRule()->getKind();
+
+    // The GSB preferred to drop an explicit rule in a conflict, but
+    // only if the kinds were the same.
+    if (existingRule->isExplicit() && !newRule->isExplicit() &&
+        existingKind == newKind) {
+      std::swap(existingRule, newRule);
+    }
+
+    if (newRule->getRHS().size() >= existingRule->getRHS().size()) {
+      newRule->markConflicting();
+    } else if (existingKind != Symbol::Kind::Superclass &&
+               existingKind == newKind) {
+      // The GSB only dropped the new rule in the case of a conflicting
+      // superclass requirement, so maintain that behavior here.
+      if (existingRule->getRHS().size() >= newRule->getRHS().size())
+        existingRule->markConflicting();
+    }
+
+    // FIXME: Diagnose the conflict later.
+  }
+}
+
 /// Given a rewrite rule which appears exactly once in a loop
 /// without context, return a new definition for this rewrite rule.
 /// The new definition is the path obtained by deleting the
@@ -507,7 +542,6 @@ findRuleToDelete(llvm::function_ref<bool(unsigned)> isRedundantRuleFn) {
         // Two rules (T.[C] => T) and (T.[C'] => T) are incomparable if
         // C and C' are superclass, concrete type or concrete conformance
         // symbols.
-        found = pair;
         continue;
       }
 
@@ -641,6 +675,7 @@ void RewriteSystem::minimizeRewriteSystem() {
   Minimized = 1;
 
   propagateExplicitBits();
+  processConflicts();
 
   // First pass:
   // - Eliminate all LHS-simplified non-conformance rules.
@@ -867,8 +902,16 @@ void RewriteSystem::verifyMinimizedRules(
     const auto &rule = getRule(ruleID);
 
     // Ignore the rewrite rule if it is not part of our minimization domain.
-    if (!isInMinimizationDomain(rule.getLHS().getRootProtocol()))
+    if (!isInMinimizationDomain(rule.getLHS().getRootProtocol())) {
+      if (rule.isRedundant()) {
+        llvm::errs() << "Redundant rule outside minimization domain: "
+                     << rule << "\n\n";
+        dump(llvm::errs());
+        abort();
+      }
+
       continue;
+    }
 
     // Note that sometimes permanent rules can be simplified, but they can never
     // be redundant.

lib/AST/RequirementMachine/PropertyMap.cpp

@@ -60,41 +60,6 @@
 using namespace swift;
 using namespace rewriting;
 
-/// This papers over a behavioral difference between
-/// GenericSignature::getRequiredProtocols() and ArchetypeType::getConformsTo();
-/// the latter drops any protocols to which the superclass requirement
-/// conforms to concretely.
-llvm::TinyPtrVector<const ProtocolDecl *>
-PropertyBag::getConformsToExcludingSuperclassConformances() const {
-  llvm::TinyPtrVector<const ProtocolDecl *> result;
-
-  if (SuperclassConformances.empty()) {
-    result = ConformsTo;
-    return result;
-  }
-
-  // The conformances in SuperclassConformances should appear in the same order
-  // as the protocols in ConformsTo.
-  auto conformanceIter = SuperclassConformances.begin();
-
-  for (const auto *proto : ConformsTo) {
-    if (conformanceIter == SuperclassConformances.end()) {
-      result.push_back(proto);
-      continue;
-    }
-
-    if (proto == (*conformanceIter)->getProtocol()) {
-      ++conformanceIter;
-      continue;
-    }
-
-    result.push_back(proto);
-  }
-
-  assert(conformanceIter == SuperclassConformances.end());
-  return result;
-}
-
 void PropertyBag::dump(llvm::raw_ostream &out) const {
   out << Key << " => {";
 
@@ -205,30 +170,41 @@ void PropertyBag::copyPropertiesFrom(const PropertyBag *next,
   if (next->ConcreteType) {
     ConcreteType = next->ConcreteType->prependPrefixToConcreteSubstitutions(
         prefix, ctx);
-    ConcreteTypeRule = next->ConcreteTypeRule;
+    ConcreteTypeRules = next->ConcreteTypeRules;
+    for (auto &pair : ConcreteTypeRules) {
+      pair.first = pair.first.prependPrefixToConcreteSubstitutions(
+          prefix, ctx);
+    }
   }
 
   // Copy over class hierarchy information.
   SuperclassDecl = next->SuperclassDecl;
   if (!next->Superclasses.empty()) {
     Superclasses = next->Superclasses;
 
-    for (auto &pair : Superclasses) {
-      pair.second.SuperclassType =
-          pair.second.SuperclassType->prependPrefixToConcreteSubstitutions(
+    for (auto &req : Superclasses) {
+      req.second.SuperclassType =
+          req.second.SuperclassType->prependPrefixToConcreteSubstitutions(
               prefix, ctx);
+      for (auto &pair : req.second.SuperclassRules) {
+        pair.first = pair.first.prependPrefixToConcreteSubstitutions(
+            prefix, ctx);
+      }
     }
   }
 }
 
 Symbol PropertyBag::concretelySimplifySubstitution(const MutableTerm &mutTerm,
                                                    RewriteContext &ctx,
                                                    RewritePath *path) const {
+  assert(!ConcreteTypeRules.empty());
+  auto &pair = ConcreteTypeRules.front();
+
   // 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(
+    pair.first.prependPrefixToConcreteSubstitutions(
         prefix, ctx);
 
   // If U.V is the substitution term and V is the property map key,
@@ -238,7 +214,7 @@ Symbol PropertyBag::concretelySimplifySubstitution(const MutableTerm &mutTerm,
   if (path) {
     path->add(RewriteStep::forRewriteRule(/*startOffset=*/prefix.size(),
                                           /*endOffset=*/0,
-                                          /*ruleID=*/*ConcreteTypeRule,
+                                          /*ruleID=*/pair.second,
                                           /*inverse=*/true));
 
     if (!prefix.empty()) {
@@ -263,13 +239,13 @@ void PropertyBag::verify(const RewriteSystem &system) const {
   // FIXME: Add asserts requiring that the layout, superclass and
   // concrete type symbols match, as above
   assert(!Layout.isNull() == LayoutRule.hasValue());
-  assert(ConcreteType.hasValue() == ConcreteTypeRule.hasValue());
+  assert(ConcreteType.hasValue() == !ConcreteTypeRules.empty());
 
   assert((SuperclassDecl == nullptr) == Superclasses.empty());
   for (const auto &pair : Superclasses) {
     const auto &req = pair.second;
     assert(req.SuperclassType.hasValue());
-    assert(req.SuperclassRule.hasValue());
+    assert(!req.SuperclassRules.empty());
   }
 
 #endif
@@ -396,8 +372,7 @@ void PropertyMap::buildPropertyMap() {
 
   for (const auto &rule : System.getRules()) {
     if (rule.isLHSSimplified() ||
-        rule.isRHSSimplified() ||
-        rule.isSubstitutionSimplified())
+        rule.isRHSSimplified())
       continue;
 
     // Identity conformances ([P].[P] => [P]) are permanent rules, but we

lib/AST/RequirementMachine/PropertyMap.h

@@ -44,14 +44,14 @@ namespace rewriting {
 class MutableTerm;
 class Term;
 
-/// Records a superclass constraint at a given level in the class hierarchy.
+/// Records superclass requirements at a given level in the class hierarchy.
 struct SuperclassRequirement {
   /// The most specific superclass constraint (in type difference order) for
   /// this level in the class hierarchy.
   Optional<Symbol> SuperclassType;
 
-  /// The corresponding superclass rule for the above.
-  Optional<unsigned> SuperclassRule;
+  /// Superclass rules that apply to this key.
+  llvm::SmallVector<std::pair<Symbol, unsigned>, 1> SuperclassRules;
 };
 
 /// Stores a convenient representation of all "property-like" rewrite rules of
@@ -83,19 +83,11 @@ class PropertyBag {
   /// for the most specific substituted type.
   llvm::SmallDenseMap<const ClassDecl *, SuperclassRequirement, 2> Superclasses;
 
-  /// All concrete conformances of Superclass to the protocols in the
-  /// ConformsTo list.
-  llvm::TinyPtrVector<ProtocolConformance *> SuperclassConformances;
-
   /// The most specific concrete type constraint this type satisfies.
   Optional<Symbol> ConcreteType;
 
-  /// The corresponding layout rule for the above.
-  Optional<unsigned> ConcreteTypeRule;
-
-  /// All concrete conformances of ConcreteType to the protocols in the
-  /// ConformsTo list.
-  llvm::TinyPtrVector<ProtocolConformance *> ConcreteConformances;
+  /// Concrete type rules that apply to this key.
+  llvm::SmallVector<std::pair<Symbol, unsigned>, 1> ConcreteTypeRules;
 
   /// Cache of associated type declarations.
   llvm::SmallDenseMap<Identifier, AssociatedTypeDecl *, 2> AssocTypes;
@@ -156,9 +148,6 @@ class PropertyBag {
     return ConformsTo;
   }
 
-  llvm::TinyPtrVector<const ProtocolDecl *>
-  getConformsToExcludingSuperclassConformances() const;
-
   AssociatedTypeDecl *getAssociatedType(Identifier name);
 
   Symbol concretelySimplifySubstitution(const MutableTerm &mutTerm,
@@ -270,10 +259,14 @@ class PropertyMap {
   void addLayoutProperty(Term key, Symbol property, unsigned ruleID);
 
   void unifyConcreteTypes(Term key,
-                          Optional<Symbol> &existingProperty,
-                          Optional<unsigned> &existingRuleID,
-                          Symbol property,
-                          unsigned ruleID);
+                          Symbol lhsProperty, unsigned lhsRuleID,
+                          Symbol rhsProperty, unsigned rhsRuleID);
+
+  void unifyConcreteTypes(Term key,
+                          Optional<Symbol> &bestProperty,
+                          llvm::SmallVectorImpl<std::pair<Symbol, unsigned>> &
+                              existingRules,
+                          Symbol property, unsigned ruleID);
 
   void recordSuperclassRelation(Term key,
                                 Symbol superclassType,
@@ -293,8 +286,7 @@ class PropertyMap {
                    CanType concreteType,
                    ArrayRef<Term> substitutions,
                    ArrayRef<unsigned> conformsToRules,
-                   ArrayRef<const ProtocolDecl *> conformsTo,
-                   llvm::TinyPtrVector<ProtocolConformance *> &conformances);
+                   ArrayRef<const ProtocolDecl *> conformsTo);
 
   void concretizeTypeWitnessInConformance(
                    Term key, RequirementKind requirementKind,