Merge pull request #41592 from slavapestov/rqm-verify-generic-signatures

RequirementMachine: Fix -verify-generic-signatures mode to not violate invariants

Author: slavapestov

lib/AST/GenericSignature.cpp

@@ -994,6 +994,39 @@ void GenericSignature::verify(ArrayRef<Requirement> reqts) const {
   }
 }
 
+static Type stripBoundDependentMemberTypes(Type t) {
+  if (auto *depMemTy = t->getAs<DependentMemberType>()) {
+    return DependentMemberType::get(
+      stripBoundDependentMemberTypes(depMemTy->getBase()),
+      depMemTy->getName());
+  }
+
+  return t;
+}
+
+static Requirement stripBoundDependentMemberTypes(Requirement req) {
+  auto subjectType = stripBoundDependentMemberTypes(req.getFirstType());
+
+  switch (req.getKind()) {
+  case RequirementKind::Conformance:
+    return Requirement(RequirementKind::Conformance, subjectType,
+                       req.getSecondType());
+
+  case RequirementKind::Superclass:
+  case RequirementKind::SameType:
+    return Requirement(req.getKind(), subjectType,
+                       req.getSecondType().transform([](Type t) {
+                         return stripBoundDependentMemberTypes(t);
+                       }));
+
+  case RequirementKind::Layout:
+    return Requirement(RequirementKind::Layout, subjectType,
+                       req.getLayoutConstraint());
+  }
+
+  llvm_unreachable("Bad requirement kind");
+}
+
 void swift::validateGenericSignature(ASTContext &context,
                                      GenericSignature sig) {
   llvm::errs() << "Validating generic signature: ";
@@ -1007,7 +1040,7 @@ void swift::validateGenericSignature(ASTContext &context,
 
   SmallVector<Requirement, 2> requirements;
   for (auto requirement : sig.getRequirements())
-    requirements.push_back(requirement);
+    requirements.push_back(stripBoundDependentMemberTypes(requirement));
 
   {
     PrettyStackTraceGenericSignature debugStack("verifying", sig);
@@ -1043,7 +1076,7 @@ void swift::validateGenericSignature(ASTContext &context,
     SmallVector<Requirement, 2> newRequirements;
     for (unsigned i : indices(requirements)) {
       if (i != victimIndex)
-        newRequirements.push_back(requirements[i]);
+        newRequirements.push_back(stripBoundDependentMemberTypes(requirements[i]));
     }
 
     auto newSigWithError = evaluateOrDefault(

lib/AST/RequirementMachine/HomotopyReduction.cpp

@@ -775,12 +775,17 @@ void RewriteSystem::minimizeRewriteSystem() {
 }
 
 /// In a conformance-valid rewrite system, any rule with unresolved symbols on
-/// the left or right hand side should have been simplified by another rule.
+/// the left or right hand side should be redundant. The presence of unresolved
+/// non-redundant rules means one of the original requirements written by the
+/// user was invalid.
 bool RewriteSystem::hadError() const {
   assert(Complete);
   assert(Minimized);
 
   for (const auto &rule : Rules) {
+    if (!isInMinimizationDomain(rule.getLHS().getRootProtocol()))
+      continue;
+
     if (rule.isPermanent())
       continue;
 

lib/AST/RequirementMachine/InterfaceType.cpp

@@ -261,6 +261,16 @@ getTypeForSymbolRange(const Symbol *begin, const Symbol *end, Type root,
       // Return a sugared GenericTypeParamType if we're given an array of
       // sugared types to substitute.
       unsigned index = GenericParamKey(genericParam).findIndexIn(genericParams);
+
+      if (index == genericParams.size()) {
+        llvm::errs() << "Invalid generic parameter: " << genericParam << "\n";
+        llvm::errs() << "Valid generic parameters are";
+        for (auto *otherParam : genericParams)
+          llvm::errs() << " " << Type(otherParam);
+        llvm::errs() << "\n";
+        abort();
+      }
+
       result = genericParams[index];
       return;
     }
@@ -344,7 +354,14 @@ getTypeForSymbolRange(const Symbol *begin, const Symbol *end, Type root,
       assert(prefix.size() > 0);
 
       auto *props = map.lookUpProperties(prefix.rbegin(), prefix.rend());
-      assert(props != nullptr);
+      if (props == nullptr) {
+        llvm::errs() << "Cannot build interface type for term "
+                     << MutableTerm(begin, end) << "\n";
+        llvm::errs() << "Prefix does not conform to any protocols: "
+                     << prefix << "\n\n";
+        map.dump(llvm::errs());
+        abort();
+      }
 
       // Assert that the associated type's protocol appears among the set
       // of protocols that the prefix conforms to.
@@ -356,6 +373,18 @@ getTypeForSymbolRange(const Symbol *begin, const Symbol *end, Type root,
   #endif
 
       assocType = props->getAssociatedType(symbol.getName());
+      if (assocType == nullptr) {
+        llvm::errs() << "Cannot build interface type for term "
+                     << MutableTerm(begin, end) << "\n";
+        llvm::errs() << "Prefix term does not not have a nested type named "
+                     << symbol.getName() << ": "
+                     << prefix << "\n";
+        llvm::errs() << "Property map entry: ";
+        props->dump(llvm::errs());
+        llvm::errs() << "\n\n";
+        map.dump(llvm::errs());
+        abort();
+      }
     }
 
     result = DependentMemberType::get(result, assocType);

lib/AST/RequirementMachine/MinimalConformances.cpp

@@ -470,7 +470,9 @@ void MinimalConformances::collectConformanceRules() {
 /// That is, we can choose to eliminate <X>.[P], but not <Y>.[P], or vice
 /// versa; but it is never valid to eliminate both.
 void MinimalConformances::computeCandidateConformancePaths() {
-  for (const auto &loop : System.getLoops()) {
+  for (unsigned loopID : indices(System.getLoops())) {
+    const auto &loop = System.getLoops()[loopID];
+
     if (loop.isDeleted())
       continue;
 
@@ -483,7 +485,7 @@ void MinimalConformances::computeCandidateConformancePaths() {
       continue;
 
     if (Debug.contains(DebugFlags::MinimalConformances)) {
-      llvm::dbgs() << "Candidate homotopy generator: ";
+      llvm::dbgs() << "Candidate homotopy generator: (#" << loopID << ") ";
       loop.dump(llvm::dbgs(), System);
       llvm::dbgs() << "\n";
     }

test/Generics/validate_stdlib_generic_signatures.swift

@@ -1,4 +1,4 @@
 // Verifies that all of the generic signatures in the standard library are
 // minimal and canonical.
 
-// RUN: %target-typecheck-verify-swift -verify-generic-signatures Swift
+// RUN: %target-typecheck-verify-swift -verify-generic-signatures Swift -requirement-machine-abstract-signatures=on