GSB: Formalize the old hack where we rebuild a signature that had redundant conformance requirements

When constructing a generic signature, any redundant explicit requirements
are dropped from the final signature.

We would assume this operation is idempotent, that is, building a new
GenericSignatureBuilder from the resulting minimized signature produces
an equivalent GenericSignatureBuilder to the original one.

Unfortunately, this is not true in the case of conformance requirements.

Namely, if a conformance requirement is made redundant by a superclass
or concrete same-type requirement, then dropping the conformance
requirement changes the canonical type computation.

For example, consider the following:

    public protocol P {
        associatedtype Element
    }

    public class C<O: P>: P {
        public typealias Element = O.Element
    }

    public func toe<T, O, E>(_: T, _: O, _: E, _: T.Element)
        where T : P, O : P, O.Element == T.Element, T : C<E> {}

In the generic signature of toe(), the superclass requirement 'T : C<E>'
implies the conformance requirement 'T : P' because C conforms to P.

However, the presence of the conformance requirement makes it so that
T.Element is the canonical representative, so previously this signature
was minimized down to:

    <T : C<E>, O : P, T.Element == O.Element>

If we build the signature again from the above requirements, then we
see that T.Element is no longer the canonical representative; instead,
T.Element canonicalizes as E.Element.

For this reason, we must rebuild the signature to get the correct
canonical type computation.

I realized that this is not an artifact of incorrect design in the
current GSB; my new rewrite system formalism would produce the same
result. Rather, it is a subtle consequence of the specification of our
minimization algorithm, and therefore it must be formalized in this
manner.

We used to sort-of do this with the HadAnyRedundantRequirements hack,
but it was both overly broad (we only need to rebuild if a conformance
requirement was implied by a superclass or concrete same-type
requirement) and not sufficient (when rebuilding, we need to strip any
bound associated types from our requirements to ensure the canonical
type anchors are re-computed).

Fixes rdar://problem/65263302, rdar://problem/75010156,
rdar://problem/75171977.

Author: slavapestov

include/swift/AST/GenericSignatureBuilder.h

@@ -620,7 +620,8 @@ class GenericSignatureBuilder {
   /// generic signature builder no longer has valid state.
   GenericSignature computeGenericSignature(
                       bool allowConcreteGenericParams = false,
-                      bool allowBuilderToMove = true) &&;
+                      bool buildingRequirementSignature = false,
+                      bool rebuildingWithoutRedundantConformances = false) &&;
 
   /// Compute the requirement signature for the given protocol.
   static GenericSignature computeRequirementSignature(ProtocolDecl *proto);
@@ -645,6 +646,8 @@ class GenericSignatureBuilder {
 private:
   void computeRedundantRequirements();
 
+  bool hasExplicitConformancesImpliedByConcrete() const;
+
   /// Describes the relationship between a given constraint and
   /// the canonical constraint of the equivalence class.
   enum class ConstraintRelation {

lib/AST/GenericSignatureBuilder.cpp

@@ -124,6 +124,8 @@ STATISTIC(NumRewriteRhsSimplifiedToLhs,
           "# of rewrite rule right-hand sides simplified to lhs (and removed)");
 STATISTIC(NumRewriteRulesRedundant,
           "# of rewrite rules that are redundant (and removed)");
+STATISTIC(NumSignaturesRebuiltWithoutRedundantRequirements,
+          "# of generic signatures which had a concretized conformance requirement");
 
 namespace  {
 
@@ -4645,7 +4647,7 @@ ConstraintResult GenericSignatureBuilder::addTypeRequirement(
                                unresolvedHandling);
   }
 
-  // If the resolved subject is a type, there may be things we can infer (if it
+  // If the resolved subject is concrete, there may be things we can infer (if it
   // conditionally conforms to the protocol), and we can probably perform
   // diagnostics here.
   if (auto subjectType = resolvedSubject.getAsConcreteType()) {
@@ -8172,9 +8174,60 @@ 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(
+      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::Conformance, subjectType,
+                       req.getLayoutConstraint());
+  }
+
+  llvm_unreachable("Bad requirement kind");
+}
+
 GenericSignature GenericSignatureBuilder::computeGenericSignature(
                                           bool allowConcreteGenericParams,
-                                          bool allowBuilderToMove) && {
+                                          bool buildingRequirementSignature,
+                                          bool rebuildingWithoutRedundantConformances) && {
   // Finalize the builder, producing any necessary diagnostics.
   finalize(getGenericParams(), allowConcreteGenericParams);
 
@@ -8185,6 +8238,43 @@ GenericSignature GenericSignatureBuilder::computeGenericSignature(
   // Form the generic signature.
   auto sig = GenericSignature::get(getGenericParams(), requirements);
 
+  // If any of our explicit conformance requirements were implied by
+  // superclass or concrete same-type requirements, we have to build the
+  // signature again, since dropping the redundant conformance requirements
+  // changes the canonical type computation.
+  //
+  // However, if we already diagnosed an error, don't do this, because
+  // we might end up emitting duplicate diagnostics.
+  //
+  // Also, don't do this when building a requirement signature.
+  if (!buildingRequirementSignature &&
+      !Impl->HadAnyError &&
+      hasExplicitConformancesImpliedByConcrete()) {
+    NumSignaturesRebuiltWithoutRedundantRequirements++;
+
+    if (rebuildingWithoutRedundantConformances) {
+      llvm::errs() << "Rebuilt signature still has "
+                   << "redundant conformance requirements: ";
+      llvm::errs() << sig << "\n";
+      abort();
+    }
+
+    GenericSignatureBuilder newBuilder(Context);
+
+    for (auto param : sig->getGenericParams())
+      newBuilder.addGenericParameter(param);
+
+    for (auto &req : sig->getRequirements()) {
+      newBuilder.addRequirement(stripBoundDependentMemberTypes(req),
+                                FloatingRequirementSource::forAbstract(), nullptr);
+    }
+
+    return std::move(newBuilder).computeGenericSignature(
+        allowConcreteGenericParams,
+        buildingRequirementSignature,
+        /*rebuildingWithoutRedundantConformances=*/true);
+  }
+
 #ifndef NDEBUG
   if (!Impl->HadAnyError) {
     checkGenericSignature(sig.getCanonicalSignature(), *this);
@@ -8196,7 +8286,9 @@ GenericSignature GenericSignatureBuilder::computeGenericSignature(
   // will produce the same thing.
   //
   // We cannot do this when there were errors.
-  if (allowBuilderToMove && !Impl->HadAnyError) {
+  //
+  // Also, we cannot do this when building a requirement signature.
+  if (!buildingRequirementSignature && !Impl->HadAnyError) {
     // Register this generic signature builder as the canonical builder for the
     // given signature.
     Context.registerGenericSignatureBuilder(sig, std::move(*this));

lib/Sema/TypeCheckDecl.cpp

@@ -939,7 +939,7 @@ RequirementSignatureRequest::evaluate(Evaluator &evaluator,
 
   auto reqSignature = std::move(builder).computeGenericSignature(
                         /*allowConcreteGenericParams=*/false,
-                        /*allowBuilderToMove=*/false);
+                        /*buildingRequirementSignature=*/true);
   return reqSignature->getRequirements();
 }
 

test/Generics/rdar65263302.swift

@@ -0,0 +1,24 @@
+// RUN: %target-swift-frontend -typecheck -debug-generic-signatures %s 2>&1 | %FileCheck %s
+// RUN: %target-swift-frontend -verify -emit-ir %s
+
+public protocol P {
+    associatedtype Element
+}
+
+public class C<O: P>: P {
+    public typealias Element = O.Element
+}
+
+// CHECK: Generic signature: <T, O, E where T : C<E>, O : P, E : P, O.Element == E.Element>
+public func toe1<T, O, E>(_: T, _: O, _: E, _: T.Element)
+    where T : P, // expected-warning {{redundant conformance constraint 'T': 'P'}}
+          O : P,
+          O.Element == T.Element,
+          T : C<E> {} // expected-note {{conformance constraint 'T': 'P' implied here}}
+
+// CHECK: Generic signature: <T, O, E where T : C<E>, O : P, E : P, O.Element == E.Element>
+public func toe2<T, O, E>(_: T, _: O, _: E, _: T.Element)
+    where O : P,
+          O.Element == T.Element,
+          T : C<E> {}
+

test/Generics/rdar75171977.swift

@@ -0,0 +1,17 @@
+// RUN: %target-swift-frontend -verify -emit-ir %s
+
+public protocol P1 {}
+
+public protocol P2 {
+    associatedtype A : P1
+    associatedtype B : P2
+    func f()
+}
+
+public extension P2 where B.A == A {
+    func f() {}
+}
+
+public class C<B: P2>: P2 {
+    public typealias A = B.A
+}