Merge pull request #36811 from slavapestov/start-purging-derived-via-concrete

GSB: Start purging the notion of "derived via concrete" requirements

Author: slavapestov

include/swift/AST/GenericSignatureBuilder.h

@@ -654,7 +654,7 @@ class GenericSignatureBuilder {
 
   class ExplicitRequirement;
 
-  bool isRedundantExplicitRequirement(ExplicitRequirement req) const;
+  bool isRedundantExplicitRequirement(const ExplicitRequirement &req) const;
 
 private:
   void computeRedundantRequirements();

lib/AST/GenericSignatureBuilder.cpp

@@ -730,6 +730,8 @@ struct GenericSignatureBuilder::Implementation {
   std::vector<ConflictingConcreteTypeRequirement>
       ConflictingConcreteTypeRequirements;
 
+  llvm::DenseSet<ExplicitRequirement> ExplicitConformancesImpliedByConcrete;
+
 #ifndef NDEBUG
   /// Whether we've already computed redundant requiremnts.
   bool computedRedundantRequirements = false;
@@ -1094,12 +1096,14 @@ const RequirementSource *RequirementSource::getMinimalConformanceSource(
                                           ArchetypeResolutionKind::WellFormed);
       assert(parentEquivClass && "Not a well-formed type?");
 
-      if (parentEquivClass->concreteType)
-        derivedViaConcrete = true;
-      else if (parentEquivClass->superclass &&
-               builder.lookupConformance(parentEquivClass->superclass,
-                                         source->getProtocolDecl()))
-        derivedViaConcrete = true;
+      if (requirementSignatureSelfProto) {
+        if (parentEquivClass->concreteType)
+          derivedViaConcrete = true;
+        else if (parentEquivClass->superclass &&
+                 builder.lookupConformance(parentEquivClass->superclass,
+                                           source->getProtocolDecl()))
+          derivedViaConcrete = true;
+      }
 
       // The parent potential archetype must conform to the protocol in which
       // this requirement resides. Add this constraint.
@@ -6256,6 +6260,20 @@ static bool typeConflictsWithLayoutConstraint(Type t, LayoutConstraint layout) {
   return false;
 }
 
+static bool isConcreteConformance(const EquivalenceClass &equivClass,
+                                  ProtocolDecl *proto,
+                                  GenericSignatureBuilder &builder) {
+  if (equivClass.concreteType &&
+      builder.lookupConformance(equivClass.concreteType, proto)) {
+    return true;
+  } else if (equivClass.superclass &&
+           builder.lookupConformance(equivClass.superclass, proto)) {
+    return true;
+  }
+
+  return false;
+}
+
 void GenericSignatureBuilder::computeRedundantRequirements() {
   assert(!Impl->computedRedundantRequirements &&
          "Already computed redundant requirements");
@@ -6289,6 +6307,14 @@ void GenericSignatureBuilder::computeRedundantRequirements() {
 
         // FIXME: Check for a conflict via the concrete type.
         exact.push_back(constraint);
+
+        if (!source->isDerivedRequirement()) {
+          if (isConcreteConformance(equivClass, entry.first, *this)) {
+            Impl->ExplicitConformancesImpliedByConcrete.insert(
+                ExplicitRequirement::fromExplicitConstraint(
+                    RequirementKind::Conformance, constraint));
+          }
+        }
       }
 
       graph.addConstraintsFromEquivClass(RequirementKind::Conformance,
@@ -7176,6 +7202,9 @@ void GenericSignatureBuilder::diagnoseRedundantRequirements() const {
 
       for (auto otherReq : found->second) {
         auto *otherSource = otherReq.getSource();
+        if (otherSource->isInferredRequirement())
+          continue;
+
         auto otherLoc = otherSource->getLoc();
         if (otherLoc.isInvalid())
           continue;
@@ -7213,6 +7242,9 @@ void GenericSignatureBuilder::diagnoseRedundantRequirements() const {
 
         for (auto otherReq : found->second) {
           auto *otherSource = otherReq.getSource();
+          if (otherSource->isInferredRequirement())
+            continue;
+
           auto otherLoc = otherSource->getLoc();
           if (otherLoc.isInvalid())
             continue;
@@ -7252,6 +7284,9 @@ void GenericSignatureBuilder::diagnoseRedundantRequirements() const {
 
         for (auto otherReq : found->second) {
           auto *otherSource = otherReq.getSource();
+          if (otherSource->isInferredRequirement())
+            continue;
+
           auto otherLoc = otherSource->getLoc();
           if (otherLoc.isInvalid())
             continue;
@@ -8159,7 +8194,7 @@ void GenericSignatureBuilder::checkLayoutConstraints(
 }
 
 bool GenericSignatureBuilder::isRedundantExplicitRequirement(
-    ExplicitRequirement req) const {
+    const ExplicitRequirement &req) const {
   assert(Impl->computedRedundantRequirements &&
          "Must ensure computeRedundantRequirements() is called first");
   auto &redundantReqs = Impl->RedundantRequirements;
@@ -8466,23 +8501,6 @@ static void checkGenericSignature(CanGenericSignature canSig,
 }
 #endif
 
-bool GenericSignatureBuilder::hasExplicitConformancesImpliedByConcrete() const {
-  for (auto pair : Impl->RedundantRequirements) {
-    if (pair.first.getKind() != RequirementKind::Conformance)
-      continue;
-
-    for (auto impliedByReq : pair.second) {
-      if (impliedByReq.getKind() == RequirementKind::Superclass)
-        return true;
-
-      if (impliedByReq.getKind() == RequirementKind::SameType)
-        return true;
-    }
-  }
-
-  return false;
-}
-
 static Type stripBoundDependentMemberTypes(Type t) {
   if (auto *depMemTy = t->getAs<DependentMemberType>()) {
     return DependentMemberType::get(
@@ -8532,7 +8550,8 @@ GenericSignature GenericSignatureBuilder::rebuildSignatureWithoutRedundantRequir
     assert(req.getKind() != RequirementKind::SameType &&
            "Should not see same-type requirement here");
 
-    if (isRedundantExplicitRequirement(req))
+    if (isRedundantExplicitRequirement(req) &&
+        Impl->ExplicitConformancesImpliedByConcrete.count(req))
       continue;
 
     auto subjectType = req.getSource()->getStoredType();
@@ -8618,8 +8637,12 @@ GenericSignature GenericSignatureBuilder::computeGenericSignature(
     assert(!Impl->HadAnyError &&
            "Rebuilt signature had errors");
 
-    assert(!hasExplicitConformancesImpliedByConcrete() &&
-           "Rebuilt signature still had redundant conformance requirements");
+#ifndef NDEBUG
+    for (const auto &req : Impl->ExplicitConformancesImpliedByConcrete) {
+      assert(!isRedundantExplicitRequirement(req) &&
+             "Rebuilt signature still had redundant conformance requirements");
+    }
+#endif
   }
 
   // If any of our explicit conformance requirements were implied by
@@ -8634,7 +8657,7 @@ GenericSignature GenericSignatureBuilder::computeGenericSignature(
   if (!rebuildingWithoutRedundantConformances &&
       !buildingRequirementSignature &&
       !Impl->HadAnyError &&
-      hasExplicitConformancesImpliedByConcrete()) {
+      !Impl->ExplicitConformancesImpliedByConcrete.empty()) {
     return std::move(*this).rebuildSignatureWithoutRedundantRequirements(
         allowConcreteGenericParams,
         buildingRequirementSignature);

test/Generics/protocol_superclass_cycle.swift

@@ -0,0 +1,8 @@
+// RUN: %target-typecheck-verify-swift
+// RUN: %target-swift-frontend -typecheck -debug-generic-signatures %s 2>&1 | %FileCheck %s
+
+protocol P : C {}
+final class C : P {}
+
+// CHECK: Generic signature: <T where T : P>
+func foo<T : P>(_: T) {}

test/Generics/rdar75656022.swift

@@ -49,7 +49,8 @@ struct OriginalExampleWithWarning<A, B> where A : P2, B : P2, A.T == B.T {
     where C : P1,
           D : P1, // expected-warning {{redundant conformance constraint 'D' : 'P1'}}
           C.T : P1, // expected-warning {{redundant conformance constraint 'C.T' : 'P1'}}
-          A == S1<C, C.T.T, S2<C.T>>,
+          A == S1<C, C.T.T, S2<C.T>>, // expected-note {{conformance constraint 'D' : 'P1' implied here}}
+          // expected-note@-1 {{conformance constraint 'C.T' : 'P1' implied here}}
           C.T == D,
           E == D.T { }
 }
@@ -71,6 +72,7 @@ struct WithoutBogusGenericParametersWithWarning<A, B> where A : P2, B : P2, A.T
     where C : P1,
           C.T : P1, // expected-warning {{redundant conformance constraint 'C.T' : 'P1'}}
           A == S1<C, C.T.T, S2<C.T>> {}
+          // expected-note@-1 {{conformance constraint 'C.T' : 'P1' implied here}}
 }
 
 // Same as above but without unnecessary generic parameters