[GSB] Clean up decision-making logic for redundant-constraint warnings.

Lots of DRY violations to refactor.

Author: DougGregor

include/swift/AST/GenericSignatureBuilder.h

@@ -1198,6 +1198,14 @@ class GenericSignatureBuilder::RequirementSource final
   /// path.
   bool isDerivedRequirement() const;
 
+  /// Whether we should diagnose a redundant constraint based on this
+  /// requirement source.
+  ///
+  /// \param primary Whether this is the "primary" requirement source, on which
+  /// a "redundant constraint" warning would be emitted vs. the requirement
+  /// source that would be used for the accompanying note.
+  bool shouldDiagnoseRedundancy(bool primary) const;
+
   /// Determine whether the given derived requirement \c source, when rooted at
   /// the potential archetype \c pa, is actually derived from the same
   /// requirement. Such "self-derived" requirements do not make the original

lib/AST/GenericSignatureBuilder.cpp

@@ -573,6 +573,11 @@ bool RequirementSource::isDerivedRequirement() const {
   llvm_unreachable("Unhandled RequirementSourceKind in switch.");
 }
 
+bool RequirementSource::shouldDiagnoseRedundancy(bool primary) const {
+  return !isInferredRequirement() && getLoc().isValid() &&
+         (!primary || !isDerivedRequirement());
+}
+
 bool RequirementSource::isSelfDerivedSource(PotentialArchetype *pa,
                                             bool &derivedViaConcrete) const {
   return getMinimalConformanceSource(pa, /*proto=*/nullptr, derivedViaConcrete)
@@ -5090,10 +5095,9 @@ Constraint<T> GenericSignatureBuilder::checkConstraintList(
       // If this requirement is not derived or inferred (but has a useful
       // location) complain that it is redundant.
       Impl->HadAnyRedundantConstraints = true;
-      if (!constraint.source->isDerivedRequirement() &&
-          !constraint.source->isInferredRequirement() &&
-          constraint.source->getLoc().isValid() &&
-          !representativeConstraint->source->isInferredRequirement()) {
+      if (constraint.source->shouldDiagnoseRedundancy(true) &&
+          representativeConstraint &&
+          representativeConstraint->source->shouldDiagnoseRedundancy(false)) {
         Diags.diagnose(constraint.source->getLoc(),
                        redundancyDiag,
                        constraint.archetype->getDependentType(genericParams),
@@ -5710,9 +5714,7 @@ void GenericSignatureBuilder::checkSameTypeConstraints(
     for (const auto &constraint : constraints) {
       // If the source/destination are identical, complain.
       if (constraint.archetype == constraint.value) {
-        if (!constraint.source->isDerivedRequirement() &&
-            !constraint.source->isInferredRequirement() &&
-            constraint.source->getLoc().isValid()) {
+        if (constraint.source->shouldDiagnoseRedundancy(true)) {
           Diags.diagnose(constraint.source->getLoc(),
                          diag::redundant_same_type_constraint,
                          constraint.archetype->getDependentType(genericParams),
@@ -5822,15 +5824,9 @@ void GenericSignatureBuilder::checkSameTypeConstraints(
         if (lhs.source != rhs.source || lhs.target != rhs.target)
           return false;
 
-        // We have two edges connected the same components. If both
-        // have locations, diagnose them.
-        if (lhs.constraint.source->getLoc().isInvalid() ||
-            rhs.constraint.source->getLoc().isInvalid())
-          return true;
-
-        // If the constraint source is inferred, don't diagnose it.
-        if (lhs.constraint.source->isInferredRequirement() ||
-            rhs.constraint.source->isInferredRequirement())
+        // Check whethe we should diagnose redundancy for both constraints.
+        if (!lhs.constraint.source->shouldDiagnoseRedundancy(true) ||
+            !rhs.constraint.source->shouldDiagnoseRedundancy(false))
           return true;
 
         Diags.diagnose(lhs.constraint.source->getLoc(),
@@ -5859,10 +5855,8 @@ void GenericSignatureBuilder::checkSameTypeConstraints(
       // If both the source and target are already connected, this edge is
       // not part of the spanning tree.
       if (connected[edge.source] && connected[edge.target]) {
-        if (edge.constraint.source->getLoc().isValid() &&
-            !edge.constraint.source->isInferredRequirement() &&
-            firstEdge.constraint.source->getLoc().isValid() &&
-            !firstEdge.constraint.source->isInferredRequirement()) {
+        if (edge.constraint.source->shouldDiagnoseRedundancy(true) &&
+            firstEdge.constraint.source->shouldDiagnoseRedundancy(false)) {
           Diags.diagnose(edge.constraint.source->getLoc(),
                          diag::redundant_same_type_constraint,
                          edge.constraint.archetype->getDependentType(
@@ -5975,47 +5969,47 @@ void GenericSignatureBuilder::checkSuperclassConstraints(
   // If we have a concrete type, check it.
   // FIXME: Substitute into the concrete type.
   if (equivClass->concreteType) {
+    auto existing = equivClass->findAnyConcreteConstraintAsWritten(
+                                           representativeConstraint.archetype);
     // Make sure the concrete type fulfills the superclass requirement.
-    if (!equivClass->superclass->isExactSuperclassOf(equivClass->concreteType)) {
-      if (auto existing = equivClass->findAnyConcreteConstraintAsWritten(
-                            representativeConstraint.archetype)) {
-        Impl->HadAnyError = true;
-
+    if (!equivClass->superclass->isExactSuperclassOf(equivClass->concreteType)){
+      Impl->HadAnyError = true;
+      if (existing) {
         Diags.diagnose(existing->source->getLoc(), diag::type_does_not_inherit,
                        existing->archetype->getDependentType(
                                                    genericParams),
                        existing->value, equivClass->superclass);
 
-        // FIXME: Note the representative constraint.
+        if (representativeConstraint.source->getLoc().isValid()) {
+          Diags.diagnose(representativeConstraint.source->getLoc(),
+                         diag::superclass_redundancy_here,
+                         representativeConstraint.source->classifyDiagKind(),
+                         representativeConstraint.archetype->getDependentType(
+                                                                 genericParams),
+                         equivClass->superclass);
+        }
       } else if (representativeConstraint.source->getLoc().isValid()) {
-        Impl->HadAnyError = true;
-
         Diags.diagnose(representativeConstraint.source->getLoc(),
                        diag::type_does_not_inherit,
                        representativeConstraint.archetype->getDependentType(
                                                     genericParams),
                        equivClass->concreteType, equivClass->superclass);
       }
-    } else if (representativeConstraint.source->getLoc().isValid() &&
-               !representativeConstraint.source->isDerivedRequirement() &&
-               !representativeConstraint.source->isInferredRequirement()) {
+    } else if (representativeConstraint.source->shouldDiagnoseRedundancy(true)
+               && existing &&
+               existing->source->shouldDiagnoseRedundancy(false)) {
       // It does fulfill the requirement; diagnose the redundancy.
       Diags.diagnose(representativeConstraint.source->getLoc(),
                      diag::redundant_superclass_constraint,
                      representativeConstraint.archetype->getDependentType(
                                                   genericParams),
                      representativeConstraint.value);
 
-      if (auto existing = equivClass->findAnyConcreteConstraintAsWritten(
-                            representativeConstraint.archetype)) {
-        if (!existing->source->isInferredRequirement()) {
-          Diags.diagnose(existing->source->getLoc(),
-                         diag::same_type_redundancy_here,
-                         existing->source->classifyDiagKind(),
-                         existing->archetype->getDependentType(genericParams),
-                         existing->value);
-        }
-      }
+      Diags.diagnose(existing->source->getLoc(),
+                     diag::same_type_redundancy_here,
+                     existing->source->classifyDiagKind(),
+                     existing->archetype->getDependentType(genericParams),
+                     existing->value);
     }
   }
 }