GSB: Diagnose redundant requirements on the rebuilt signature

We rebuild a generic signature after dropping conformance requirements
made redundant by a superclass or concrete type requirement.

When rebuilding the signature, preserve the source locations of the
original requirements, and only perform diagnostics on the rebuilt
signature.

This fixes an issue where we would emit a redundant requirement
warning even though the requirement in question was not actually
redundant.

This also avoids some unnecessary work. Most of the code in
finalize() does not need to be run twice, once before and once after
rebuilding the signature. Now we only run it after rebuilding the
signature.

Note that this regresses diagnostics in one narrow case where we
would previously diagnose a conflicting concrete type requirement.
This will be fixed once concrete type diagnostics are moved over
to use the new ExplicitRequirement infrastructure, just like all
other kinds already do.

Fixes rdar://problem/77462797.

Author: slavapestov

include/swift/AST/GenericSignatureBuilder.h

@@ -685,7 +685,8 @@ class GenericSignatureBuilder {
   void diagnoseProtocolRefinement(
       const ProtocolDecl *requirementSignatureSelfProto);
 
-  void diagnoseRedundantRequirements() const;
+  void diagnoseRedundantRequirements(
+      bool onlyDiagnoseExplicitConformancesImpliedByConcrete=false) const;
 
   void diagnoseConflictingConcreteTypeRequirements(
       const ProtocolDecl *requirementSignatureSelfProto);

lib/AST/GenericSignatureBuilder.cpp

@@ -6349,8 +6349,9 @@ void
 GenericSignatureBuilder::finalize(TypeArrayView<GenericTypeParamType> genericParams,
                                   bool allowConcreteGenericParams,
                                   const ProtocolDecl *requirementSignatureSelfProto) {
-  // Process any delayed requirements that we can handle now.
-  processDelayedRequirements();
+  diagnoseProtocolRefinement(requirementSignatureSelfProto);
+  diagnoseRedundantRequirements();
+  diagnoseConflictingConcreteTypeRequirements(requirementSignatureSelfProto);
 
   {
     // In various places below, we iterate over the list of equivalence classes
@@ -6383,11 +6384,6 @@ GenericSignatureBuilder::finalize(TypeArrayView<GenericTypeParamType> genericPar
     }
   }
 
-  computeRedundantRequirements(requirementSignatureSelfProto);
-  diagnoseProtocolRefinement(requirementSignatureSelfProto);
-  diagnoseRedundantRequirements();
-  diagnoseConflictingConcreteTypeRequirements(requirementSignatureSelfProto);
-
   assert(!Impl->finalized && "Already finalized builder");
 #ifndef NDEBUG
   Impl->finalized = true;
@@ -7006,7 +7002,8 @@ void GenericSignatureBuilder::diagnoseProtocolRefinement(
   }
 }
 
-void GenericSignatureBuilder::diagnoseRedundantRequirements() const {
+void GenericSignatureBuilder::diagnoseRedundantRequirements(
+    bool onlyDiagnoseExplicitConformancesImpliedByConcrete) const {
   for (const auto &req : Impl->ExplicitRequirements) {
     auto *source = req.getSource();
     auto loc = source->getLoc();
@@ -7024,6 +7021,10 @@ void GenericSignatureBuilder::diagnoseRedundantRequirements() const {
     if (found == Impl->RedundantRequirements.end())
       continue;
 
+    if (onlyDiagnoseExplicitConformancesImpliedByConcrete &&
+        Impl->ExplicitConformancesImpliedByConcrete.count(req) == 0)
+      continue;
+
     // Don't diagnose explicit requirements that are implied by
     // inferred requirements.
     if (llvm::all_of(found->second,
@@ -8418,14 +8419,20 @@ GenericSignature GenericSignatureBuilder::rebuildSignatureWithoutRedundantRequir
                                                    requirementSignatureSource);
   }
 
-  auto newSource = [&]() {
+  auto getRebuiltSource = [&](const RequirementSource *source) {
+    assert(!source->isDerivedRequirement());
+
     if (auto *proto = const_cast<ProtocolDecl *>(requirementSignatureSelfProto)) {
       return FloatingRequirementSource::viaProtocolRequirement(
-          requirementSignatureSource, proto, /*inferred=*/false);
+          requirementSignatureSource, proto, source->getLoc(),
+          source->isInferredRequirement());
     }
 
-    return FloatingRequirementSource::forAbstract();
-  }();
+    if (source->isInferredRequirement())
+      return FloatingRequirementSource::forInferred(source->getLoc());
+
+    return FloatingRequirementSource::forExplicit(source->getLoc());
+  };
 
   for (const auto &req : Impl->ExplicitRequirements) {
     assert(req.getKind() != RequirementKind::SameType &&
@@ -8449,7 +8456,7 @@ GenericSignature GenericSignatureBuilder::rebuildSignatureWithoutRedundantRequir
         !resolvedSubjectType->isTypeParameter()) {
       newBuilder.addRequirement(Requirement(RequirementKind::SameType,
                                             subjectType, resolvedSubjectType),
-                                newSource, nullptr);
+                                getRebuiltSource(req.getSource()), nullptr);
       continue;
     }
 
@@ -8458,7 +8465,8 @@ GenericSignature GenericSignatureBuilder::rebuildSignatureWithoutRedundantRequir
     if (auto optReq = createRequirement(req.getKind(), resolvedSubjectType,
                                         req.getRHS(), getGenericParams())) {
       auto newReq = stripBoundDependentMemberTypes(*optReq);
-      newBuilder.addRequirement(newReq, newSource, nullptr);
+      newBuilder.addRequirement(newReq, getRebuiltSource(req.getSource()),
+                                nullptr);
     }
   }
 
@@ -8491,7 +8499,8 @@ GenericSignature GenericSignatureBuilder::rebuildSignatureWithoutRedundantRequir
         Requirement(RequirementKind::SameType,
                     subjectType, constraintType));
 
-    newBuilder.addRequirement(newReq, newSource, nullptr);
+    newBuilder.addRequirement(newReq, getRebuiltSource(req.getSource()),
+                              nullptr);
   }
 
   // Wipe out the internal state of the old builder, since we don't need it anymore.
@@ -8506,22 +8515,10 @@ GenericSignature GenericSignatureBuilder::rebuildSignatureWithoutRedundantRequir
 GenericSignature GenericSignatureBuilder::computeGenericSignature(
                                           bool allowConcreteGenericParams,
                                           const ProtocolDecl *requirementSignatureSelfProto) && {
-  // Finalize the builder, producing any necessary diagnostics.
-  finalize(getGenericParams(),
-           allowConcreteGenericParams,
-           requirementSignatureSelfProto);
-
-  if (Impl->RebuildingWithoutRedundantConformances) {
-    assert(!Impl->HadAnyError &&
-           "Rebuilt signature had errors");
+  // Process any delayed requirements that we can handle now.
+  processDelayedRequirements();
 
-#ifndef NDEBUG
-    for (const auto &req : Impl->ExplicitConformancesImpliedByConcrete) {
-      assert(!isRedundantExplicitRequirement(req) &&
-             "Rebuilt signature still had redundant conformance requirements");
-    }
-#endif
-  }
+  computeRedundantRequirements(requirementSignatureSelfProto);
 
   // If any of our explicit conformance requirements were implied by
   // superclass or concrete same-type requirements, we have to build the
@@ -8535,12 +8532,30 @@ GenericSignature GenericSignatureBuilder::computeGenericSignature(
   if (!Impl->RebuildingWithoutRedundantConformances &&
       !Impl->HadAnyError &&
       !Impl->ExplicitConformancesImpliedByConcrete.empty()) {
+    diagnoseRedundantRequirements(
+        /*onlyDiagnoseExplicitConformancesImpliedByConcrete=*/true);
+
     return std::move(*this).rebuildSignatureWithoutRedundantRequirements(
         allowConcreteGenericParams,
         requirementSignatureSelfProto);
   }
 
-  // Collect the requirements placed on the generic parameter types.
+  if (Impl->RebuildingWithoutRedundantConformances) {
+#ifndef NDEBUG
+    for (const auto &req : Impl->ExplicitConformancesImpliedByConcrete) {
+      assert(!isRedundantExplicitRequirement(req) &&
+             "Rebuilt signature still had redundant conformance requirements");
+    }
+#endif
+  }
+
+  // Diagnose redundant requirements, check for recursive concrete types
+  // and compute minimized same-type requirements.
+  finalize(getGenericParams(),
+           allowConcreteGenericParams,
+           requirementSignatureSelfProto);
+
+  // Collect all non-redundant explicit requirements.
   SmallVector<Requirement, 4> requirements;
   enumerateRequirements(getGenericParams(), requirements);
 

test/Generics/derived_via_concrete.swift

@@ -26,48 +26,32 @@ func derivedViaConcreteX1<A, B>(_: A, _: B)
 // expected-warning@-1 {{redundant conformance constraint 'A' : 'U'}}
 // expected-note@-2 {{conformance constraint 'A' : 'U' implied here}}
 
-// FIXME: We should not diagnose 'B : P' as redundant, even though it
-// really is, because it was made redundant by an inferred requirement.
-
 // CHECK: Generic signature: <A, B where A : X<B>, B : P>
 func derivedViaConcreteX2<A, B>(_: A, _: B)
   where A : U, B : P, A : X<B> {}
 // expected-warning@-1 {{redundant conformance constraint 'A' : 'U'}}
 // expected-note@-2 {{conformance constraint 'A' : 'U' implied here}}
-// expected-warning@-3 {{redundant conformance constraint 'B' : 'P'}}
-// expected-note@-4 {{conformance constraint 'B' : 'P' implied here}}
 
 // CHECK: Generic signature: <A, B where A : Y<B>, B : C>
 func derivedViaConcreteY1<A, B>(_: A, _: B)
   where A : V, A : Y<B> {}
 // expected-warning@-1 {{redundant conformance constraint 'A' : 'V'}}
 // expected-note@-2 {{conformance constraint 'A' : 'V' implied here}}
 
-// FIXME: We should not diagnose 'B : C' as redundant, even though it
-// really is, because it was made redundant by an inferred requirement.
-
 // CHECK: Generic signature: <A, B where A : Y<B>, B : C>
 func derivedViaConcreteY2<A, B>(_: A, _: B)
   where A : V, B : C, A : Y<B> {}
 // expected-warning@-1 {{redundant conformance constraint 'A' : 'V'}}
 // expected-note@-2 {{conformance constraint 'A' : 'V' implied here}}
-// expected-warning@-3 {{redundant superclass constraint 'B' : 'C'}}
-// expected-note@-4 {{superclass constraint 'B' : 'C' implied here}}
 
 // CHECK: Generic signature: <A, B where A : Z<B>, B : AnyObject>
 func derivedViaConcreteZ1<A, B>(_: A, _: B)
   where A : W, A : Z<B> {}
 // expected-warning@-1 {{redundant conformance constraint 'A' : 'W'}}
 // expected-note@-2 {{conformance constraint 'A' : 'W' implied here}}
 
-// FIXME: We should not diagnose 'B : AnyObject' as redundant, even
-// though it really is, because it was made redundant by an inferred
-// requirement.
-
 // CHECK: Generic signature: <A, B where A : Z<B>, B : AnyObject>
 func derivedViaConcreteZ2<A, B>(_: A, _: B)
   where A : W, B : AnyObject, A : Z<B> {}
 // expected-warning@-1 {{redundant conformance constraint 'A' : 'W'}}
 // expected-note@-2 {{conformance constraint 'A' : 'W' implied here}}
-// expected-warning@-3 {{redundant constraint 'B' : 'AnyObject'}}
-// expected-note@-4 {{constraint 'B' : 'AnyObject' implied here}}

test/Generics/rdar77462797.swift

@@ -0,0 +1,38 @@
+// RUN: %target-typecheck-verify-swift
+// RUN: %target-swift-frontend -typecheck -debug-generic-signatures %s 2>&1 | %FileCheck %s
+
+protocol P {}
+
+protocol Q {
+  associatedtype T : P
+}
+
+class S<T : P> : Q {}
+
+struct G<X, Y> where X : Q {
+  // no redundancies
+  func foo1() where X == S<Y> {}
+  // CHECK: Generic signature: <X, Y where X == S<Y>, Y : P>
+
+  // 'Y : P' is redundant, but only via an inferred requirement from S<Y>,
+  // so we should not diagnose it
+  func foo2() where X == S<Y>, Y : P {}
+  // CHECK: Generic signature: <X, Y where X == S<Y>, Y : P>
+
+  // 'X : Q' is redundant
+  func foo3() where X : Q, X == S<Y>, Y : P {}
+  // expected-warning@-1 {{redundant conformance constraint 'X' : 'Q'}}
+  // expected-note@-2 {{conformance constraint 'X' : 'Q' implied here}}
+  // CHECK: Generic signature: <X, Y where X == S<Y>, Y : P>
+
+  // 'T.T : P' is redundant
+  func foo4<T : Q>(_: T) where X == S<Y>, T.T : P {}
+  // expected-warning@-1 {{redundant conformance constraint 'T.T' : 'P'}}
+  // expected-note@-2 {{conformance constraint 'T.T' : 'P' implied here}}
+  // CHECK: Generic signature: <X, Y, T where X == S<Y>, Y : P, T : Q>
+}
+
+func foo<X, Y>(_: X, _: Y) where X : Q, X : S<Y>, Y : P {}
+// expected-warning@-1 {{redundant conformance constraint 'X' : 'Q'}}
+// expected-note@-2 {{conformance constraint 'X' : 'Q' implied here}}
+// CHECK: Generic signature: <X, Y where X : S<Y>, Y : P>

test/Generics/requirement_inference.swift

@@ -328,9 +328,7 @@ 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@
@@ -365,13 +363,8 @@ struct X26<T: X3> : P26 {
 // CHECK-NEXT: Canonical requirement signature: <τ_0_0 where τ_0_0.A == X26<τ_0_0.B>, τ_0_0.B : X3>
 protocol P27a {
   associatedtype A: P26 // expected-warning{{redundant conformance constraint 'Self.A' : 'P26'}}
-  // expected-note@-1 {{superclass constraint 'Self.B' : 'X3' implied here}}
 
   associatedtype B: X3 where A == X26<B> // expected-note{{conformance constraint 'Self.A' : 'P26' implied here}}
-  // expected-warning@-1 {{redundant superclass constraint 'Self.B' : 'X3'}}
-
-  // FIXME: The above warning should not be emitted -- while the requirement
-  // really is redundant, it is made redundant by an inferred requirement.
 }
 
 // CHECK-LABEL: .P27b@

test/Generics/sr13884.swift

@@ -1,5 +1,5 @@
 // RUN: %target-typecheck-verify-swift
-// RUN: %target-swift-frontend -emit-ir -debug-generic-signatures %s 2>&1 | %FileCheck %s
+// RUN: not %target-swift-frontend -typecheck -debug-generic-signatures %s 2>&1 | %FileCheck %s
 
 public protocol P {
   associatedtype A : Q where A.B == Self
@@ -27,8 +27,10 @@ public class D : Q {
 public func takesBoth1<T : P & C>(_: T) {}
 // expected-warning@-1 {{redundant conformance constraint 'T' : 'P'}}
 // expected-note@-2 {{conformance constraint 'T' : 'P' implied here}}
+// expected-error@-3 {{same-type requirement makes generic parameter 'T' non-generic}}
 
 // CHECK-LABEL: Generic signature: <U where U == D.B>
 public func takesBoth2<U : C & P>(_: U) {}
 // expected-warning@-1 {{redundant conformance constraint 'U' : 'P'}}
 // expected-note@-2 {{conformance constraint 'U' : 'P' implied here}}
+// expected-error@-3 {{same-type requirement makes generic parameter 'U' non-generic}}

test/attr/attr_specialize.swift

@@ -95,7 +95,7 @@ struct FloatElement : HasElt {
   typealias Element = Float
 }
 @_specialize(where T == FloatElement)
-@_specialize(where T == IntElement) // expected-error{{'T.Element' cannot be equal to both 'IntElement.Element' (aka 'Int') and 'Float'}}
+@_specialize(where T == IntElement) // FIXME e/xpected-error{{'T.Element' cannot be equal to both 'IntElement.Element' (aka 'Int') and 'Float'}}
 func sameTypeRequirement<T : HasElt>(_ t: T) where T.Element == Float {}
 
 @_specialize(where T == Sub)