Merge pull request #36598 from slavapestov/enumerate-requirements-explicitly

slavapestov · web-flow · commit 8f52c2683fe3 · 2021-03-27T00:34:17.000-04:00
GSB: Use explicit requirement list in enumerateRequirements()
diff --git a/lib/AST/GenericSignatureBuilder.cpp b/lib/AST/GenericSignatureBuilder.cpp
@@ -698,7 +698,7 @@ struct GenericSignatureBuilder::Implementation {
   bool HadAnyError = false;
 
   /// All explicit non-same type requirements that were added to the builder.
-  SmallVector<ExplicitRequirement, 2> ExplicitRequirements;
+  SmallSetVector<ExplicitRequirement, 2> ExplicitRequirements;
 
   /// All explicit same-type requirements that were added to the builder.
   SmallVector<Requirement, 2> ExplicitSameTypeRequirements;
@@ -2607,21 +2607,22 @@ GenericSignatureBuilder::resolveConcreteConformance(ResolvedType type,
     concreteSource = concreteSource->viaConcrete(*this, concrete);
   } else {
     concreteSource = concreteSource->viaConcrete(*this, conformance);
-    equivClass->recordConformanceConstraint(*this, type, proto, concreteSource);
-
-    // Only infer conditional requirements from explicit sources.
-    bool hasExplicitSource = llvm::any_of(
-        equivClass->concreteTypeConstraints,
-        [](const ConcreteConstraint &constraint) {
-          return (!constraint.source->isDerivedRequirement() &&
-                  constraint.source->getLoc().isValid());
-        });
+  }
 
-    if (hasExplicitSource) {
-      if (addConditionalRequirements(conformance, /*inferForModule=*/nullptr,
-                                     concreteSource->getLoc()))
-        return nullptr;
-    }
+  equivClass->recordConformanceConstraint(*this, type, proto, concreteSource);
+
+  // Only infer conditional requirements from explicit sources.
+  bool hasExplicitSource = llvm::any_of(
+      equivClass->concreteTypeConstraints,
+      [](const ConcreteConstraint &constraint) {
+        return (!constraint.source->isDerivedRequirement() &&
+                constraint.source->getLoc().isValid());
+      });
+
+  if (hasExplicitSource) {
+    if (addConditionalRequirements(conformance, /*inferForModule=*/nullptr,
+                                   concreteSource->getLoc()))
+      return nullptr;
   }
 
   return concreteSource;
@@ -4507,8 +4508,9 @@ ConstraintResult GenericSignatureBuilder::addConformanceRequirement(
   auto resolvedSource = source.getSource(*this, type);
 
   if (!resolvedSource->isDerivedRequirement()) {
-    Impl->ExplicitRequirements.emplace_back(RequirementKind::Conformance,
-                                            resolvedSource, proto);
+    Impl->ExplicitRequirements.insert(
+        ExplicitRequirement(RequirementKind::Conformance,
+                            resolvedSource, proto));
   }
 
   // Add the conformance requirement, bailing out earlier if we've already
@@ -4529,8 +4531,9 @@ ConstraintResult GenericSignatureBuilder::addLayoutRequirementDirect(
   auto resolvedSource = source.getSource(*this, type);
 
   if (!resolvedSource->isDerivedRequirement()) {
-    Impl->ExplicitRequirements.emplace_back(RequirementKind::Layout,
-                                            resolvedSource, layout);
+    Impl->ExplicitRequirements.insert(
+        ExplicitRequirement(RequirementKind::Layout,
+                            resolvedSource, layout));
   }
 
   auto equivClass = type.getEquivalenceClass(*this);
@@ -4670,8 +4673,9 @@ ConstraintResult GenericSignatureBuilder::addSuperclassRequirementDirect(
   auto resolvedSource = source.getSource(*this, type);
 
   if (!resolvedSource->isDerivedRequirement()) {
-    Impl->ExplicitRequirements.emplace_back(RequirementKind::Superclass,
-                                            resolvedSource, superclass);
+    Impl->ExplicitRequirements.insert(
+        ExplicitRequirement(RequirementKind::Superclass,
+                            resolvedSource, superclass));
   }
 
   // Record the constraint.
@@ -6466,14 +6470,16 @@ GenericSignatureBuilder::finalize(TypeArrayView<GenericTypeParamType> genericPar
     checkLayoutConstraints(genericParams, &equivClass);
   };
 
-  // FIXME: Expand all conformance requirements. This is expensive :(
-  for (auto &equivClass : Impl->EquivalenceClasses) {
+  if (!Impl->ExplicitSameTypeRequirements.empty()) {
+    // FIXME: Expand all conformance requirements. This is expensive :(
+    for (auto &equivClass : Impl->EquivalenceClasses) {
       expandSameTypeConstraints(*this, &equivClass);
-  }
+    }
 
-  // Check same-type constraints.
-  for (auto &equivClass : Impl->EquivalenceClasses) {
-    checkSameTypeConstraints(genericParams, &equivClass);
+    // Check same-type constraints.
+    for (auto &equivClass : Impl->EquivalenceClasses) {
+      checkSameTypeConstraints(genericParams, &equivClass);
+    }
   }
 
   // Check for generic parameters which have been made concrete or equated
@@ -7977,26 +7983,6 @@ bool GenericSignatureBuilder::isRedundantExplicitRequirement(
   return (redundantReqs.find(req) != redundantReqs.end());
 }
 
-namespace {
-  template<typename T>
-  bool hasNonRedundantRequirementSource(ArrayRef<Constraint<T>> constraints,
-                                        RequirementKind kind,
-                                        GenericSignatureBuilder &builder) {
-    for (auto constraint : constraints) {
-      if (constraint.source->isDerivedRequirement())
-        continue;
-
-      auto req = ExplicitRequirement::fromExplicitConstraint(kind, constraint);
-      if (builder.isRedundantExplicitRequirement(req))
-        continue;
-
-      return true;
-    }
-
-    return false;
-  }
-} // end anonymous namespace
-
 static Optional<Requirement> createRequirement(RequirementKind kind,
                                                Type depTy,
                                                RequirementRHS rhs,
@@ -8075,116 +8061,89 @@ void GenericSignatureBuilder::enumerateRequirements(
       requirements.push_back(*req);
   };
 
-  // Collect all of the subject types that will be involved in constraints.
-  for (auto &equivClass : Impl->EquivalenceClasses) {
-    if (equivClass.derivedSameTypeComponents.empty()) {
-      checkSameTypeConstraints(genericParams, &equivClass);
-    }
-
-    for (unsigned i : indices(equivClass.derivedSameTypeComponents)) {
-      // Dig out the subject type and its corresponding component.
-      auto &component = equivClass.derivedSameTypeComponents[i];
-      Type subjectType = component.type;
-
-      assert(!subjectType->hasError());
-      assert(!subjectType->findUnresolvedDependentMemberType());
+  // Collect all non-same type requirements.
+  for (auto &req : Impl->ExplicitRequirements) {
+    if (isRedundantExplicitRequirement(req))
+      continue;
 
-      // If this equivalence class is bound to a concrete type, equate the
-      // anchor with a concrete type.
-      if (Type concreteType = equivClass.concreteType) {
-        concreteType = getCanonicalTypeInContext(concreteType, genericParams);
+    auto depTy = getCanonicalTypeInContext(
+        req.getSource()->getStoredType(), { });
 
-        // If the parent of this anchor is also a concrete type, don't
-        // create a requirement.
-        if (!subjectType->is<GenericTypeParamType>() &&
-            maybeResolveEquivalenceClass(
-              subjectType->castTo<DependentMemberType>()->getBase(),
-              ArchetypeResolutionKind::WellFormed,
-              /*wantExactPotentialArchetype=*/false)
-              .getEquivalenceClass(*this)->concreteType)
-          continue;
+    // FIXME: This should be an assert once we ensure that concrete
+    // same-type requirements always mark other requirements on the
+    // same subject type as redundant or conflicting.
+    if (!depTy->isTypeParameter())
+      continue;
 
-        // Drop recursive and invalid concrete-type constraints.
-        if (equivClass.recursiveConcreteType ||
-            equivClass.invalidConcreteType)
-          continue;
+    auto rhs = req.getRHS();
+    if (auto constraintType = rhs.dyn_cast<Type>()) {
+      rhs = getCanonicalTypeInContext(constraintType, genericParams);
+    }
 
-        // Filter out derived requirements... except for concrete-type
-        // requirements on generic parameters. The exception is due to
-        // the canonicalization of generic signatures, which never
-        // eliminates generic parameters even when they have been
-        // mapped to a concrete type.
-        if (subjectType->is<GenericTypeParamType>() ||
-            component.concreteTypeSource == nullptr ||
-            !component.concreteTypeSource->isDerivedRequirement()) {
-          recordRequirement(RequirementKind::SameType,
-                            subjectType, concreteType);
-        }
-        continue;
-      }
+    recordRequirement(req.getKind(), depTy, rhs);
+  }
 
-      std::function<void()> deferredSameTypeRequirement;
-
-      // If we're at the last anchor in the component, do nothing;
-      if (i + 1 != equivClass.derivedSameTypeComponents.size()) {
-        // Form a same-type constraint from this anchor within the component
-        // to the next.
-        // FIXME: Distinguish between explicit and inferred here?
-        auto &nextComponent = equivClass.derivedSameTypeComponents[i + 1];
-        Type otherSubjectType = nextComponent.type;
-        deferredSameTypeRequirement =
-          [&recordRequirement, subjectType, otherSubjectType] {
-            recordRequirement(RequirementKind::SameType,
-                              subjectType, otherSubjectType);
-          };
+  // Collect all same type requirements.
+  if (!Impl->ExplicitSameTypeRequirements.empty()) {
+    for (auto &equivClass : Impl->EquivalenceClasses) {
+      if (equivClass.derivedSameTypeComponents.empty()) {
+        checkSameTypeConstraints(genericParams, &equivClass);
       }
 
-      SWIFT_DEFER {
-        if (deferredSameTypeRequirement) deferredSameTypeRequirement();
-      };
-
-      // If this is not the first component anchor in its equivalence class,
-      // we're done.
-      if (i > 0)
-        continue;
-
-      // If we have a superclass, produce a superclass requirement
-      if (auto superclass = equivClass.superclass) {
-        superclass = getCanonicalTypeInContext(superclass, genericParams);
+      for (unsigned i : indices(equivClass.derivedSameTypeComponents)) {
+        // Dig out the subject type and its corresponding component.
+        auto &component = equivClass.derivedSameTypeComponents[i];
+        Type subjectType = component.type;
+
+        assert(!subjectType->hasError());
+        assert(!subjectType->findUnresolvedDependentMemberType());
+
+        // If this equivalence class is bound to a concrete type, equate the
+        // anchor with a concrete type.
+        if (Type concreteType = equivClass.concreteType) {
+          concreteType = getCanonicalTypeInContext(concreteType, genericParams);
+
+          // If the parent of this anchor is also a concrete type, don't
+          // create a requirement.
+          if (!subjectType->is<GenericTypeParamType>() &&
+              maybeResolveEquivalenceClass(
+                subjectType->castTo<DependentMemberType>()->getBase(),
+                ArchetypeResolutionKind::WellFormed,
+                /*wantExactPotentialArchetype=*/false)
+                .getEquivalenceClass(*this)->concreteType)
+            continue;
 
-        if (!equivClass.recursiveSuperclassType &&
-            hasNonRedundantRequirementSource<Type>(
-              equivClass.superclassConstraints,
-              RequirementKind::Superclass, *this)) {
-          recordRequirement(RequirementKind::Superclass,
-                            subjectType, superclass);
-        }
-      }
+          // Drop recursive and invalid concrete-type constraints.
+          if (equivClass.recursiveConcreteType ||
+              equivClass.invalidConcreteType)
+            continue;
 
-      // If we have a layout constraint, produce a layout requirement.
-      if (equivClass.layout) {
-        if (hasNonRedundantRequirementSource<LayoutConstraint>(
-              equivClass.layoutConstraints,
-              RequirementKind::Layout, *this)) {
-          recordRequirement(RequirementKind::Layout,
-                            subjectType, equivClass.layout);
+          // Filter out derived requirements... except for concrete-type
+          // requirements on generic parameters. The exception is due to
+          // the canonicalization of generic signatures, which never
+          // eliminates generic parameters even when they have been
+          // mapped to a concrete type.
+          if (subjectType->is<GenericTypeParamType>() ||
+              component.concreteTypeSource == nullptr ||
+              !component.concreteTypeSource->isDerivedRequirement()) {
+            recordRequirement(RequirementKind::SameType,
+                              subjectType, concreteType);
+          }
+          continue;
         }
-      }
 
-      // Enumerate conformance requirements.
-      SmallVector<ProtocolDecl *, 4> protocols;
+        // If we're at the last anchor in the component, do nothing;
+        if (i + 1 != equivClass.derivedSameTypeComponents.size()) {
+          // Form a same-type constraint from this anchor within the component
+          // to the next.
+          // FIXME: Distinguish between explicit and inferred here?
+          auto &nextComponent = equivClass.derivedSameTypeComponents[i + 1];
+          Type otherSubjectType = nextComponent.type;
 
-      for (const auto &conforms : equivClass.conformsTo) {
-        if (hasNonRedundantRequirementSource<ProtocolDecl *>(
-              conforms.second, RequirementKind::Conformance, *this)) {
-          protocols.push_back(conforms.first);
+          recordRequirement(RequirementKind::SameType,
+                            subjectType, otherSubjectType);
         }
       }
-
-      // Enumerate the conformance requirements.
-      for (auto proto : protocols) {
-        recordRequirement(RequirementKind::Conformance, subjectType, proto);
-      }
     }
   }
 
@@ -8310,6 +8269,14 @@ static void checkGenericSignature(CanGenericSignature canSig,
       }
     }
 
+    // If we have a concrete same-type requirement, we shouldn't have any
+    // other requirements on the same type.
+    if (reqt.getKind() == RequirementKind::SameType &&
+        !reqt.getSecondType()->isTypeParameter()) {
+      assert(compareLHS < 0 &&
+             "Concrete subject type should not have any other requirements");
+    }
+
     assert(compareRequirements(&prevReqt, &reqt) < 0 &&
            "Out-of-order requirements");
   }
diff --git a/test/Generics/indirectly_concrete_generic_param.swift b/test/Generics/indirectly_concrete_generic_param.swift
@@ -0,0 +1,17 @@
+// RUN: %target-typecheck-verify-swift
+// RUN: %target-swift-frontend -typecheck -debug-generic-signatures %s 2>&1 | %FileCheck %s
+
+class S<T, U> where T : P, U == T.T {}
+
+protocol P {
+  associatedtype T
+}
+
+struct G<X, T, U> {}
+
+class C : P {
+  typealias T = Int
+}
+
+// CHECK-LABEL: Generic signature: <X, T, U where X : S<T, C.T>, T : C, U == C.T>
+extension G where X : S<T, U>, T : C {}
diff --git a/test/Generics/redundant_requirement_self_conforming_protocol.swift b/test/Generics/redundant_requirement_self_conforming_protocol.swift
@@ -0,0 +1,9 @@
+// RUN: %target-typecheck-verify-swift
+// RUN: %target-swift-frontend -typecheck -debug-generic-signatures %s 2>&1 | %FileCheck %s
+
+struct G<T> {}
+
+// CHECK-LABEL: Generic signature: <T where T == Error>
+extension G where T : Error, T == Error {}
+// expected-warning@-1 {{redundant conformance constraint 'T': 'Error'}}
+// expected-note@-2 {{conformance constraint 'T': 'Error' implied here}}
diff --git a/test/IDE/print_ast_tc_decls.swift b/test/IDE/print_ast_tc_decls.swift
@@ -619,7 +619,7 @@ struct d0200_EscapedIdentifiers {
 
   func `func`<`let`: `protocol`, `where`>(
       class: Int, struct: `protocol`, foo: `let`, bar: `where`) where `where` : `protocol` {}
-// PASS_COMMON-NEXT: {{^}}  func `func`<`let`, `where`>(class: Int, struct: {{(d0200_EscapedIdentifiers.)?}}`protocol`, foo: `let`, bar: `where`) where `let` : {{(d0200_EscapedIdentifiers.)?}}`protocol`, `where` : {{(d0200_EscapedIdentifiers.)?}}`protocol`{{$}}
+// PASS_COMMON-NEXT: {{^}}  func `func`<`let`, `where`>(class: Int, struct: {{(d0200_EscapedIdentifiers.)?}}`protocol`, foo: `let`, bar: `where`) where `let` : {{(d0200_EscapedIdentifiers.)?}}`class`, `where` : {{(d0200_EscapedIdentifiers.)?}}`class`{{$}}
 
   var `var`: `struct` = `struct`()
 // PASS_COMMON-NEXT: {{^}}  @_hasInitialValue var `var`: {{(d0200_EscapedIdentifiers.)?}}`struct`{{$}}
