GSB: When rebuilding a signature, use the as-written requirements instead of the minimized ones

We rebuild a signature after dropping redundant conformance requirements,
since conformance requirements change the canonical type and conformance
access path computation.

When doing this, instead of using the canonical requirements from the
signature, use the original as-written requirements.

This fixes some weirdness around concrete same-type requirements.

There's a philosophical benefit here too -- since we rebuild the
signature without ever having built the old signature, we never create
an invalid GenericSignature in the ASTContext.

Fixes rdar://problem/75690903.

Author: slavapestov

include/swift/AST/GenericSignatureBuilder.h

@@ -614,6 +614,10 @@ class GenericSignatureBuilder {
   /// because the type \c Dictionary<K,V> cannot be formed without it.
   void inferRequirements(ModuleDecl &module, ParameterList *params);
 
+  GenericSignature rebuildSignatureWithoutRedundantRequirements(
+                      bool allowConcreteGenericParams,
+                      bool buildingRequirementSignature) &&;
+
   /// Finalize the set of requirements and compute the generic
   /// signature.
   ///

lib/AST/GenericSignatureBuilder.cpp

@@ -8366,19 +8366,97 @@ static Requirement stripBoundDependentMemberTypes(Requirement req) {
   llvm_unreachable("Bad requirement kind");
 }
 
+GenericSignature GenericSignatureBuilder::rebuildSignatureWithoutRedundantRequirements(
+                                          bool allowConcreteGenericParams,
+                                          bool buildingRequirementSignature) && {
+  NumSignaturesRebuiltWithoutRedundantRequirements++;
+
+  GenericSignatureBuilder newBuilder(Context);
+
+  for (auto param : getGenericParams())
+    newBuilder.addGenericParameter(param);
+
+  auto newSource = FloatingRequirementSource::forAbstract();
+
+  for (const auto &req : Impl->ExplicitRequirements) {
+    assert(req.getKind() != RequirementKind::SameType &&
+           "Should not see same-type requirement here");
+
+    if (isRedundantExplicitRequirement(req))
+      continue;
+
+    auto subjectType = req.getSource()->getStoredType();
+    subjectType = stripBoundDependentMemberTypes(subjectType);
+    subjectType =
+        resolveDependentMemberTypes(*this, subjectType,
+                                    ArchetypeResolutionKind::WellFormed);
+
+    if (auto optReq = createRequirement(req.getKind(), subjectType, req.getRHS(),
+                                        getGenericParams())) {
+      auto newReq = stripBoundDependentMemberTypes(*optReq);
+      newBuilder.addRequirement(newReq, newSource, nullptr);
+    }
+  }
+
+  for (const auto &req : Impl->ExplicitSameTypeRequirements) {
+    auto resolveType = [this](Type t) -> Type {
+      t = stripBoundDependentMemberTypes(t);
+      if (t->is<GenericTypeParamType>()) {
+        return t;
+      } else if (auto *depMemTy = t->getAs<DependentMemberType>()) {
+        auto resolvedBaseTy =
+            resolveDependentMemberTypes(*this, depMemTy->getBase(),
+                                        ArchetypeResolutionKind::WellFormed);
+
+
+        if (resolvedBaseTy->isTypeParameter()) {
+          return DependentMemberType::get(resolvedBaseTy, depMemTy->getName());
+        } else {
+          return resolveDependentMemberTypes(*this, t,
+                                             ArchetypeResolutionKind::WellFormed);
+        }
+      } else {
+        return t;
+      }
+    };
+
+    auto subjectType = resolveType(req.getFirstType());
+    auto constraintType = resolveType(req.getSecondType());
+
+    auto newReq = stripBoundDependentMemberTypes(
+        Requirement(RequirementKind::SameType,
+                    subjectType, constraintType));
+
+    newBuilder.addRequirement(newReq, newSource, nullptr);
+  }
+
+  // Wipe out the internal state of the old builder, since we don't need it anymore.
+  Impl.reset();
+
+  // Build a new signature using the new builder.
+  return std::move(newBuilder).computeGenericSignature(
+      allowConcreteGenericParams,
+      buildingRequirementSignature,
+      /*rebuildingWithoutRedundantConformances=*/true);
+}
+
 GenericSignature GenericSignatureBuilder::computeGenericSignature(
                                           bool allowConcreteGenericParams,
                                           bool buildingRequirementSignature,
                                           bool rebuildingWithoutRedundantConformances) && {
   // Finalize the builder, producing any necessary diagnostics.
   finalize(getGenericParams(), allowConcreteGenericParams);
 
-  // Collect the requirements placed on the generic parameter types.
-  SmallVector<Requirement, 4> requirements;
-  enumerateRequirements(getGenericParams(), requirements);
+  if (rebuildingWithoutRedundantConformances) {
+    assert(!buildingRequirementSignature &&
+           "Rebuilding a requirement signature?");
 
-  // Form the generic signature.
-  auto sig = GenericSignature::get(getGenericParams(), requirements);
+    assert(!Impl->HadAnyError &&
+           "Rebuilt signature had errors");
+
+    assert(!hasExplicitConformancesImpliedByConcrete() &&
+           "Rebuilt signature still had redundant conformance requirements");
+  }
 
   // If any of our explicit conformance requirements were implied by
   // superclass or concrete same-type requirements, we have to build the
@@ -8389,34 +8467,22 @@ GenericSignature GenericSignatureBuilder::computeGenericSignature(
   // we might end up emitting duplicate diagnostics.
   //
   // Also, don't do this when building a requirement signature.
-  if (!buildingRequirementSignature &&
+  if (!rebuildingWithoutRedundantConformances &&
+      !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(
+    return std::move(*this).rebuildSignatureWithoutRedundantRequirements(
         allowConcreteGenericParams,
-        buildingRequirementSignature,
-        /*rebuildingWithoutRedundantConformances=*/true);
+        buildingRequirementSignature);
   }
 
+  // Collect the requirements placed on the generic parameter types.
+  SmallVector<Requirement, 4> requirements;
+  enumerateRequirements(getGenericParams(), requirements);
+
+  // Form the generic signature.
+  auto sig = GenericSignature::get(getGenericParams(), requirements);
+
 #ifndef NDEBUG
   if (!Impl->HadAnyError) {
     checkGenericSignature(sig.getCanonicalSignature(), *this);
@@ -8526,9 +8592,6 @@ void GenericSignatureBuilder::verifyGenericSignature(ASTContext &context,
       context.Diags.diagnose(SourceLoc(), diag::generic_signature_not_minimal,
                              reqString, sig->getAsString());
     }
-
-    // Canonicalize the signature to check that it is canonical.
-    (void)newSig.getCanonicalSignature();
   }
 }
 

test/Generics/rdar75656022.swift

@@ -36,3 +36,48 @@ struct UnresolvedWithConcreteBase<A, B> {
           D == C.T,
           E == D.T { }
 }
+
+// Make sure that we drop the conformance requirement
+// 'A : P2' and rebuild the generic signature with the
+// correct same-type requirements.
+//
+// The original test case in the bug report (correctly)
+// produces two warnings about redundant requirements.
+struct OriginalExampleWithWarning<A, B> where A : P2, B : P2, A.T == B.T {
+  // CHECK-LABEL: Generic signature: <A, B, C, D, E where A == S1<C, E, S2<D>>, B : P2, C : P1, D == B.T, E == D.T, B.T == C.T>
+  init<C, D, E>(_: C)
+    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>>,
+          C.T == D,
+          E == D.T { }
+}
+
+// Same as above but without the warnings.
+struct OriginalExampleWithoutWarning<A, B> where A : P2, B : P2, A.T == B.T {
+  // CHECK-LABEL: Generic signature: <A, B, C, D, E where A == S1<C, E, S2<D>>, B : P2, C : P1, D == B.T, E == D.T, B.T == C.T>
+  init<C, D, E>(_: C)
+    where C : P1,
+          A == S1<C, C.T.T, S2<C.T>>,
+          C.T == D,
+          E == D.T { }
+}
+
+// Same as above but without unnecessary generic parameters.
+struct WithoutBogusGenericParametersWithWarning<A, B> where A : P2, B : P2, A.T == B.T {
+  // CHECK-LABEL: Generic signature: <A, B, C where A == S1<C, B.T.T, S2<B.T>>, B : P2, C : P1, B.T == C.T>
+  init<C>(_: C)
+    where C : P1,
+          C.T : P1, // expected-warning {{redundant conformance constraint 'C.T': 'P1'}}
+          A == S1<C, C.T.T, S2<C.T>> {}
+}
+
+// Same as above but without unnecessary generic parameters
+// or the warning.
+struct WithoutBogusGenericParametersWithoutWarning<A, B> where A : P2, B : P2, A.T == B.T {
+  // CHECK-LABEL: Generic signature: <A, B, C where A == S1<C, B.T.T, S2<B.T>>, B : P2, C : P1, B.T == C.T>
+  init<C>(_: C)
+    where C : P1,
+          A == S1<C, C.T.T, S2<C.T>> {}
+}

test/Generics/requirement_inference.swift

@@ -270,7 +270,7 @@ struct X18: P18, P17 {
 }
 
 // CHECK-LABEL: .X19.foo@
-// CHECK: Generic signature: <T, U where T == X18>
+// CHECK: Generic signature: <T, U where T == X18.A>
 struct X19<T: P18> where T == T.A {
   func foo<U>(_: U) where T == X18 { }
 }

validation-test/compiler_crashers_2_fixed/sr12812.swift renamed to test/Generics/sr12812.swift

@@ -1,4 +1,5 @@
-// RUN: %target-swift-frontend -emit-ir %s
+// RUN: %target-typecheck-verify-swift
+// RUN: %target-swift-frontend -debug-generic-signatures -emit-ir %s 2>&1 | %FileCheck %s
 
 public protocol DefaultInitializable {
   init()
@@ -74,6 +75,7 @@ public extension GeneralAdjacencyList {
   }
 }
 
+// CHECK-LABEL: Generic signature: <Spine, VertexIDToIndex where Spine : RangeReplaceableCollection, VertexIDToIndex == IdentityVertexIDToIndex<Spine>, Spine.Element : AdjacencyVertex_, Spine.Index == Spine.Element.Edges.Element.VertexID, Spine.Element.Edges : BidirectionalCollection, Spine.Element.Edges : RangeReplaceableCollection>
 public extension GeneralAdjacencyList
   where VertexIDToIndex == IdentityVertexIDToIndex<Spine>,
         Spine: RangeReplaceableCollection,