Merge pull request #36460 from slavapestov/non-well-founded-conditional-requirement-inference

GSB: Narrow scope of conditional requirement inference

Author: slavapestov

include/swift/AST/GenericSignatureBuilder.h

@@ -1422,6 +1422,9 @@ class GenericSignatureBuilder::FloatingRequirementSource {
   /// Whether this is an explicitly-stated requirement.
   bool isExplicit() const;
 
+  /// Whether this is a derived requirement.
+  bool isDerived() const;
+
   /// Whether this is a top-level requirement written in source.
   /// FIXME: This is a hack because expandConformanceRequirement()
   /// is too eager; we should remove this once we fix it properly.

lib/AST/GenericSignatureBuilder.cpp

@@ -1758,6 +1758,37 @@ SourceLoc FloatingRequirementSource::getLoc() const {
   return SourceLoc();
 }
 
+bool FloatingRequirementSource::isDerived() const {
+  switch (kind) {
+  case Explicit:
+  case Inferred:
+  case NestedTypeNameMatch:
+    return false;
+
+  case AbstractProtocol:
+    switch (storage.get<const RequirementSource *>()->kind) {
+    case RequirementSource::RequirementSignatureSelf:
+      return false;
+
+    case RequirementSource::Concrete:
+    case RequirementSource::Explicit:
+    case RequirementSource::Inferred:
+    case RequirementSource::NestedTypeNameMatch:
+    case RequirementSource::Parent:
+    case RequirementSource::ProtocolRequirement:
+    case RequirementSource::InferredProtocolRequirement:
+    case RequirementSource::Superclass:
+    case RequirementSource::Layout:
+    case RequirementSource::EquivalentType:
+      return true;
+    }
+
+  case Resolved:
+    return storage.get<const RequirementSource *>()->isDerivedRequirement();
+  }
+  llvm_unreachable("unhandled kind");
+}
+
 bool FloatingRequirementSource::isExplicit() const {
   switch (kind) {
   case Explicit:
@@ -2556,9 +2587,20 @@ GenericSignatureBuilder::resolveConcreteConformance(ResolvedType type,
   } else {
     concreteSource = concreteSource->viaConcrete(*this, conformance);
     equivClass->recordConformanceConstraint(*this, type, proto, concreteSource);
-    if (addConditionalRequirements(conformance, /*inferForModule=*/nullptr,
-                                   concreteSource->getLoc()))
-      return nullptr;
+
+    // Only infer conditional requirements from explicit sources.
+    bool hasExplicitSource = llvm::any_of(
+        equivClass->concreteTypeConstraints,
+        [](const ConcreteConstraint &constraint) {
+          return (!constraint.source->isDerivedRequirement() &&
+                  constraint.source->getLoc().isValid());
+        });
+
+    if (hasExplicitSource) {
+      if (addConditionalRequirements(conformance, /*inferForModule=*/nullptr,
+                                     concreteSource->getLoc()))
+        return nullptr;
+    }
   }
 
   return concreteSource;
@@ -2590,9 +2632,20 @@ const RequirementSource *GenericSignatureBuilder::resolveSuperConformance(
 
   superclassSource = superclassSource->viaSuperclass(*this, conformance);
   equivClass->recordConformanceConstraint(*this, type, proto, superclassSource);
-  if (addConditionalRequirements(conformance, /*inferForModule=*/nullptr,
-                                 superclassSource->getLoc()))
-    return nullptr;
+
+  // Only infer conditional requirements from explicit sources.
+  bool hasExplicitSource = llvm::any_of(
+    equivClass->superclassConstraints,
+    [](const ConcreteConstraint &constraint) {
+      return (!constraint.source->isDerivedRequirement() &&
+              constraint.source->getLoc().isValid());
+    });
+
+  if (hasExplicitSource) {
+    if (addConditionalRequirements(conformance, /*inferForModule=*/nullptr,
+                                   superclassSource->getLoc()))
+      return nullptr;
+  }
 
   return superclassSource;
 }
@@ -4599,9 +4652,12 @@ ConstraintResult GenericSignatureBuilder::addTypeRequirement(
 
         // FIXME: diagnose if there's no conformance.
         if (conformance) {
-          if (addConditionalRequirements(conformance, inferForModule,
-                                         source.getLoc()))
-            return ConstraintResult::Conflicting;
+          // Only infer conditional requirements from explicit sources.
+          if (!source.isDerived()) {
+            if (addConditionalRequirements(conformance, inferForModule,
+                                           source.getLoc()))
+              return ConstraintResult::Conflicting;
+          }
         }
       }
     }

test/Generics/conditional_requirement_inference.swift

@@ -0,0 +1,53 @@
+// RUN: %target-typecheck-verify-swift
+// RUN: not %target-swift-frontend -typecheck -debug-generic-signatures %s 2>&1 | %FileCheck %s
+
+
+// Valid example
+struct EquatableBox<T : Equatable> {
+  // CHECK: Generic signature: <T, U where T == Array<U>, U : Equatable>
+  func withArray<U>(_: U) where T == Array<U> {}
+}
+
+
+// A very elaborate invalid example (see comment in mergeP1AndP2())
+struct G<T> {}
+
+protocol P {}
+extension G : P where T : P {}
+
+protocol P1 {
+  associatedtype T
+  associatedtype U where U == G<T>
+  associatedtype R : P1
+}
+
+protocol P2 {
+  associatedtype U : P
+  associatedtype R : P2
+}
+
+func takesP<T : P>(_: T.Type) {}
+// expected-note@-1 {{where 'T' = 'T.T'}}
+// expected-note@-2 {{where 'T' = 'T.R.T'}}
+// expected-note@-3 {{where 'T' = 'T.R.R.T'}}
+// expected-note@-4 {{where 'T' = 'T.R.R.R.T'}}
+
+// CHECK: Generic signature: <T where T : P1, T : P2>
+func mergeP1AndP2<T : P1 & P2>(_: T) {
+  // P1 implies that T.(R)*.U == G<T.(R)*.T>, and P2 implies that T.(R)*.U : P.
+  //
+  // These together would seem to imply that G<T.(R)*.T> : P, therefore
+  // the conditional conformance G : P should imply that T.(R)*.T : P.
+  //
+  // However, this would require us to infer an infinite number of
+  // conformance requirements in the signature of mergeP1AndP2() of the
+  // form T.(R)*.T : P.
+  //
+  // Since we're unable to represent that, make sure that a) we don't crash,
+  // b) we reject the conformance T.(R)*.T : P.
+
+  takesP(T.T.self) // expected-error {{global function 'takesP' requires that 'T.T' conform to 'P'}}
+  takesP(T.R.T.self) // expected-error {{global function 'takesP' requires that 'T.R.T' conform to 'P'}}
+  takesP(T.R.R.T.self) // expected-error {{global function 'takesP' requires that 'T.R.R.T' conform to 'P'}}
+  takesP(T.R.R.R.T.self) // expected-error {{global function 'takesP' requires that 'T.R.R.R.T' conform to 'P'}}
+}

test/Generics/validate_stdlib_generic_signatures.swift

@@ -1,9 +1,4 @@
 // Verifies that all of the generic signatures in the standard library are
 // minimal and canonical.
 
-// RUN: not %target-typecheck-verify-swift -verify-generic-signatures Swift 2>&1 | %FileCheck %s
-
-// CHECK-NOT: error:
-// CHECK-DAG: error: unexpected error produced: generic requirement 'τ_0_0.Index : Strideable' is redundant in <τ_0_0 where τ_0_0 : RandomAccessCollection, τ_0_0.Index : Strideable, τ_0_0.Indices == Range<τ_0_0.Index>, τ_0_0.Index.Stride == Int>
-// CHECK-DAG: error: diagnostic produced elsewhere: generic requirement 'τ_0_0.Index : Strideable' is redundant in <τ_0_0 where τ_0_0 : RandomAccessCollection, τ_0_0.Index : Strideable, τ_0_0.Indices == Range<τ_0_0.Index>, τ_0_0.Index.Stride == Int>
-// CHECK-NOT: error:
+// RUN: %target-typecheck-verify-swift -verify-generic-signatures Swift