GSB: Stop computing 'derived via concrete' sources

slavapestov · slavapestov · commit aa865a2133fa · 2021-05-07T18:43:52.000-04:00
diff --git a/include/swift/AST/GenericSignatureBuilder.h b/include/swift/AST/GenericSignatureBuilder.h
@@ -1241,8 +1241,7 @@ class GenericSignatureBuilder::RequirementSource final
   /// requirement redundant, because without said original requirement, the
   /// derived requirement ceases to hold.
   bool isSelfDerivedSource(GenericSignatureBuilder &builder,
-                           Type type,
-                           bool &derivedViaConcrete) const;
+                           Type type) const;
 
   /// For a requirement source that describes the requirement \c type:proto,
   /// retrieve the minimal subpath of this requirement source that will
@@ -1254,8 +1253,7 @@ class GenericSignatureBuilder::RequirementSource final
   const RequirementSource *getMinimalConformanceSource(
                                             GenericSignatureBuilder &builder,
                                             Type type,
-                                            ProtocolDecl *proto,
-                                            bool &derivedViaConcrete) const;
+                                            ProtocolDecl *proto) const;
 
   /// Retrieve a source location that corresponds to the requirement.
   SourceLoc getLoc() const;
diff --git a/lib/AST/GenericSignatureBuilder.cpp b/lib/AST/GenericSignatureBuilder.cpp
@@ -936,10 +936,8 @@ bool RequirementSource::shouldDiagnoseRedundancy(bool primary) const {
 }
 
 bool RequirementSource::isSelfDerivedSource(GenericSignatureBuilder &builder,
-                                            Type type,
-                                            bool &derivedViaConcrete) const {
-  return getMinimalConformanceSource(builder, type, /*proto=*/nullptr,
-                                     derivedViaConcrete)
+                                            Type type) const {
+  return getMinimalConformanceSource(builder, type, /*proto=*/nullptr)
     != this;
 }
 
@@ -989,10 +987,7 @@ static bool isSelfDerivedProtocolRequirementInProtocol(
 const RequirementSource *RequirementSource::getMinimalConformanceSource(
                                              GenericSignatureBuilder &builder,
                                              Type currentType,
-                                             ProtocolDecl *proto,
-                                             bool &derivedViaConcrete) const {
-  derivedViaConcrete = false;
-
+                                             ProtocolDecl *proto) const {
   // If it's not a derived requirement, it's not self-derived.
   if (!isDerivedRequirement()) return this;
 
@@ -1059,15 +1054,6 @@ const RequirementSource *RequirementSource::getMinimalConformanceSource(
                                           ArchetypeResolutionKind::WellFormed);
       assert(parentEquivClass && "Not a well-formed type?");
 
-      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.
       if (auto startOfPath =
@@ -1136,7 +1122,7 @@ const RequirementSource *RequirementSource::getMinimalConformanceSource(
                               redundantSubpath->first,
                               redundantSubpath->second);
     return shorterSource
-      ->getMinimalConformanceSource(builder, currentType, proto, derivedViaConcrete);
+      ->getMinimalConformanceSource(builder, currentType, proto);
   }
 
   // It's self-derived but we don't have a redundant subpath to eliminate.
@@ -5797,13 +5783,10 @@ static void expandSameTypeConstraints(GenericSignatureBuilder &builder,
       bool alreadyFound = false;
       const RequirementSource *conformsSource = nullptr;
       for (const auto &constraint : conforms.second) {
-        bool derivedViaConcrete = false;
-
         auto *minimal = constraint.source->getMinimalConformanceSource(
-            builder, constraint.getSubjectDependentType({ }), proto,
-            derivedViaConcrete);
+            builder, constraint.getSubjectDependentType({ }), proto);
 
-        if (minimal == nullptr || derivedViaConcrete)
+        if (minimal == nullptr)
           continue;
 
         if (minimal->getAffectedType()->isEqual(dependentType)) {
@@ -6028,18 +6011,14 @@ void GenericSignatureBuilder::computeRedundantRequirements(
           requirementSignatureSelfProto,
           [&](const Constraint<ProtocolDecl *> &constraint) {
             // FIXME: Remove this.
-            bool derivedViaConcrete;
             auto minimalSource =
               constraint.source->getMinimalConformanceSource(
                   *this,
                   constraint.getSubjectDependentType({ }),
-                  proto, derivedViaConcrete);
+                  proto);
             if (minimalSource != constraint.source)
               return true;
 
-            if (derivedViaConcrete)
-              return true;
-
             return false;
           });
 
@@ -6613,27 +6592,21 @@ void GenericSignatureBuilder::processDelayedRequirements() {
 
 namespace {
   /// Remove self-derived sources from the given vector of constraints.
-  ///
-  /// \returns true if any derived-via-concrete constraints were found.
   template<typename T>
-  bool removeSelfDerived(GenericSignatureBuilder &builder,
+  void removeSelfDerived(GenericSignatureBuilder &builder,
                          std::vector<Constraint<T>> &constraints,
                          ProtocolDecl *proto,
-                         bool dropDerivedViaConcrete = true,
                          bool allCanBeSelfDerived = false) {
     auto genericParams = builder.getGenericParams();
-    bool anyDerivedViaConcrete = false;
-    Optional<Constraint<T>> remainingConcrete;
     SmallVector<Constraint<T>, 4> minimalSources;
     constraints.erase(
       std::remove_if(constraints.begin(), constraints.end(),
         [&](const Constraint<T> &constraint) {
-          bool derivedViaConcrete;
           auto minimalSource =
             constraint.source->getMinimalConformanceSource(
                          builder,
                          constraint.getSubjectDependentType(genericParams),
-                         proto, derivedViaConcrete);
+                         proto);
           if (minimalSource != constraint.source) {
             // The minimal source is smaller than the original source, so the
             // original source is self-derived.
@@ -6650,21 +6623,8 @@ namespace {
             return true;
           }
 
-           if (!derivedViaConcrete)
-             return false;
-
-           anyDerivedViaConcrete = true;
-
-           if (!dropDerivedViaConcrete)
-             return false;
-
-           // Drop derived-via-concrete requirements.
-           if (!remainingConcrete)
-             remainingConcrete = constraint;
-
-           ++NumSelfDerived;
-           return true;
-         }),
+          return false;
+        }),
       constraints.end());
 
     // If we found any minimal sources, add them now, avoiding introducing any
@@ -6684,13 +6644,8 @@ namespace {
       }
     }
 
-    // If we only had concrete conformances, put one back.
-    if (constraints.empty() && remainingConcrete)
-      constraints.push_back(*remainingConcrete);
-
     assert((!constraints.empty() || allCanBeSelfDerived) &&
            "All constraints were self-derived!");
-    return anyDerivedViaConcrete;
   }
 } // end anonymous namespace
 
@@ -7253,9 +7208,7 @@ static void computeDerivedSameTypeComponents(
     // construction of self-derived sources really don't work, because we
     // discover more information later, so we need a more on-line or
     // iterative approach.
-    bool derivedViaConcrete;
-    if (concrete.source->isSelfDerivedSource(builder, subjectType,
-                                             derivedViaConcrete))
+    if (concrete.source->isSelfDerivedSource(builder, subjectType))
       continue;
 
     // If it has a better source than we'd seen before for this component,
@@ -7589,13 +7542,10 @@ void GenericSignatureBuilder::checkSameTypeConstraints(
   if (!equivClass->derivedSameTypeComponents.empty())
     return;
 
-  bool anyDerivedViaConcrete = false;
   // Remove self-derived constraints.
-  if (removeSelfDerived(*this, equivClass->sameTypeConstraints,
-                        /*proto=*/nullptr,
-                        /*dropDerivedViaConcrete=*/false,
-                        /*allCanBeSelfDerived=*/true))
-    anyDerivedViaConcrete = true;
+  removeSelfDerived(*this, equivClass->sameTypeConstraints,
+                    /*proto=*/nullptr,
+                    /*allCanBeSelfDerived=*/true);
 
   // Sort the constraints, so we get a deterministic ordering of diagnostics.
   llvm::array_pod_sort(equivClass->sameTypeConstraints.begin(),
@@ -7673,16 +7623,6 @@ void GenericSignatureBuilder::checkSameTypeConstraints(
       IntercomponentEdge(firstComponentIdx, secondComponentIdx, constraint));
   }
 
-  // If there were any derived-via-concrete constraints, drop them now before
-  // we emit other diagnostics.
-  if (anyDerivedViaConcrete) {
-    // Remove derived-via-concrete constraints.
-    (void)removeSelfDerived(*this, equivClass->sameTypeConstraints,
-                            /*proto=*/nullptr,
-                            /*dropDerivedViaConcrete=*/true,
-                            /*allCanBeSelfDerived=*/true);
-  }
-
   // Walk through each of the components, checking the intracomponent edges.
   // This will diagnose any explicitly-specified requirements within a
   // component, all of which are redundant.
@@ -7880,7 +7820,6 @@ void GenericSignatureBuilder::checkConcreteTypeConstraints(
 
   removeSelfDerived(*this, equivClass->concreteTypeConstraints,
                     /*proto=*/nullptr,
-                    /*dropDerivedViaConcrete=*/true,
                     /*allCanBeSelfDerived=*/true);
 
   // This can occur if the combination of a superclass requirement and
diff --git a/test/Generics/requirement_inference.swift b/test/Generics/requirement_inference.swift
@@ -328,7 +328,9 @@ struct X24<T: P20> : P24 {
 // CHECK-NEXT: Canonical requirement signature: <τ_0_0 where τ_0_0.A == X24<τ_0_0.B>, τ_0_0.B : P20>
 protocol P25a {
   associatedtype A: P24 // expected-warning{{redundant conformance constraint 'Self.A' : 'P24'}}
+  // expected-note@-1 {{conformance constraint 'Self.B' : 'P20' implied here}}
   associatedtype B: P20 where A == X24<B> // expected-note{{conformance constraint 'Self.A' : 'P24' implied here}}
+  // expected-warning@-1 {{redundant conformance constraint 'Self.B' : 'P20'}}
 }
 
 // CHECK-LABEL: .P25b@

Original file line number	Diff line number	Diff line change
`@@ -328,7 +328,9 @@ struct X24<T: P20> : P24 {`
`328`	`328`	`// CHECK-NEXT: Canonical requirement signature: <τ_0_0 where τ_0_0.A == X24<τ_0_0.B>, τ_0_0.B : P20>`
`329`	`329`	`protocol P25a {`
`330`	`330`	`associatedtype A: P24 // expected-warning{{redundant conformance constraint 'Self.A' : 'P24'}}`
	`331`	`+ // expected-note@-1 {{conformance constraint 'Self.B' : 'P20' implied here}}`
`331`	`332`	`associatedtype B: P20 where A == X24<B> // expected-note{{conformance constraint 'Self.A' : 'P24' implied here}}`
	`333`	`+ // expected-warning@-1 {{redundant conformance constraint 'Self.B' : 'P20'}}`
`332`	`334`	`}`
`333`	`335`
`334`	`336`	`// CHECK-LABEL: .P25b@`