Merge pull request #37466 from slavapestov/ban-implicit-protocol-refinement

GSB: When looking at protocol refinements, only consider other sources on 'Self'

Author: slavapestov

include/swift/AST/DiagnosticsSema.def

@@ -2424,6 +2424,10 @@ NOTE(redundant_conformance_here,none,
      "conformance constraint %0 : %1 implied here",
      (Type, ProtocolDecl *))
 
+WARNING(missing_protocol_refinement, none,
+        "protocol %0 should be declared to refine %1 due to a same-type constraint on 'Self'",
+        (ProtocolDecl *, ProtocolDecl *))
+
 ERROR(unsupported_recursive_requirements, none,
       "requirement involves recursion that is not currently supported", ())
 

include/swift/AST/GenericSignatureBuilder.h

@@ -689,6 +689,9 @@ class GenericSignatureBuilder {
   void computeRedundantRequirements(
       const ProtocolDecl *requirementSignatureSelfProto);
 
+  void diagnoseProtocolRefinement(
+      const ProtocolDecl *requirementSignatureSelfProto);
+
   void diagnoseRedundantRequirements() const;
 
   void diagnoseConflictingConcreteTypeRequirements(

lib/AST/GenericSignatureBuilder.cpp

@@ -6067,6 +6067,18 @@ void GenericSignatureBuilder::checkIfRequirementCanBeDerived(
   }
 }
 
+static bool involvesNonSelfSubjectTypes(const RequirementSource *source) {
+  while (source->kind != RequirementSource::RequirementSignatureSelf) {
+    if (source->isProtocolRequirement() &&
+        !source->getStoredType()->is<GenericTypeParamType>())
+      return true;
+
+    source = source->parent;
+  }
+
+  return false;
+}
+
 void GenericSignatureBuilder::computeRedundantRequirements(
     const ProtocolDecl *requirementSignatureSelfProto) {
   assert(!Impl->computedRedundantRequirements &&
@@ -6126,10 +6138,27 @@ void GenericSignatureBuilder::computeRedundantRequirements(
       auto found = equivClass->conformsTo.find(proto);
       assert(found != equivClass->conformsTo.end());
 
+      // If this is a conformance requirement on 'Self', only consider it
+      // redundant if it can be derived from other sources involving 'Self'.
+      //
+      // What this means in practice is that we will never try to build
+      // a witness table for an inherited conformance from an associated
+      // conformance.
+      //
+      // This is important since witness tables for inherited conformances
+      // are realized eagerly before the witness table for the original
+      // conformance, and allowing more complex requirement sources for
+      // inherited conformances could introduce cycles at runtime.
+      bool isProtocolRefinement =
+          (requirementSignatureSelfProto &&
+           equivClass->getAnchor(*this, { })->is<GenericTypeParamType>());
+
       checkIfRequirementCanBeDerived(
           req, found->second,
           requirementSignatureSelfProto,
           [&](const Constraint<ProtocolDecl *> &constraint) {
+            if (isProtocolRefinement)
+              return involvesNonSelfSubjectTypes(constraint.source);
             return false;
           });
 
@@ -6351,6 +6380,7 @@ GenericSignatureBuilder::finalize(TypeArrayView<GenericTypeParamType> genericPar
   processDelayedRequirements();
 
   computeRedundantRequirements(requirementSignatureSelfProto);
+  diagnoseProtocolRefinement(requirementSignatureSelfProto);
   diagnoseRedundantRequirements();
   diagnoseConflictingConcreteTypeRequirements(requirementSignatureSelfProto);
 
@@ -6928,6 +6958,50 @@ static bool isRedundantlyInheritableObjCProtocol(
   return true;
 }
 
+/// Diagnose any conformance requirements on 'Self' that are not derived from
+/// the protocol's inheritance clause.
+void GenericSignatureBuilder::diagnoseProtocolRefinement(
+    const ProtocolDecl *requirementSignatureSelfProto) {
+  if (requirementSignatureSelfProto == nullptr)
+    return;
+
+  auto selfType = requirementSignatureSelfProto->getSelfInterfaceType();
+  auto *equivClass = resolveEquivalenceClass(
+      selfType, ArchetypeResolutionKind::AlreadyKnown);
+  assert(equivClass != nullptr);
+
+  for (auto pair : equivClass->conformsTo) {
+    auto *proto = pair.first;
+    bool found = false;
+    SourceLoc loc;
+    for (const auto &constraint : pair.second) {
+      if (!involvesNonSelfSubjectTypes(constraint.source)) {
+        found = true;
+        break;
+      }
+
+      auto *parent = constraint.source;
+      while (parent->kind != RequirementSource::RequirementSignatureSelf) {
+        loc = parent->getLoc();
+        if (loc)
+          break;
+      }
+    }
+
+    if (!found) {
+      requirementSignatureSelfProto->diagnose(
+          diag::missing_protocol_refinement,
+          const_cast<ProtocolDecl *>(requirementSignatureSelfProto),
+          proto);
+
+      if (loc.isValid()) {
+        Context.Diags.diagnose(loc, diag::redundant_conformance_here,
+                               selfType, proto);
+      }
+    }
+  }
+}
+
 void GenericSignatureBuilder::diagnoseRedundantRequirements() const {
   for (const auto &req : Impl->ExplicitRequirements) {
     auto *source = req.getSource();

test/Generics/redundant_protocol_refinement.swift

@@ -0,0 +1,24 @@
+// RUN: %target-typecheck-verify-swift
+// RUN: %target-swift-frontend -typecheck -debug-generic-signatures %s 2>&1 | %FileCheck %s
+
+protocol Base {}
+protocol Middle : Base {}
+protocol Derived : Middle, Base {}
+// expected-note@-1 {{conformance constraint 'Self' : 'Base' implied here}}
+// expected-warning@-2 {{redundant conformance constraint 'Self' : 'Base'}}
+
+protocol P1 {}
+
+protocol P2 {
+  associatedtype Assoc: P1
+}
+
+// no warning here
+protocol Good: P2, P1 where Assoc == Self {}
+// CHECK-LABEL: Requirement signature: <Self where Self : P1, Self : P2, Self == Self.Assoc>
+
+// missing refinement of 'P1'
+protocol Bad: P2 where Assoc == Self {}
+// expected-warning@-1 {{protocol 'Bad' should be declared to refine 'P1' due to a same-type constraint on 'Self'}}
+// expected-note@-2 {{conformance constraint 'Self' : 'P1' implied here}}
+// CHECK-LABEL: Requirement signature: <Self where Self : P2, Self == Self.Assoc>

test/Interpreter/sr10941.swift

@@ -0,0 +1,35 @@
+// RUN: %empty-directory(%t)
+// RUN: %target-build-swift %s -o %t/a.out
+// RUN: %target-codesign %t/a.out
+// RUN: %target-run %t/a.out
+// REQUIRES: executable_test
+
+// https://bugs.swift.org/browse/SR-10941
+
+public protocol SupportedPropertyType { }
+
+public protocol TypedSupportedType: SupportedPropertyType where FactoryType.BuildType == Self {
+    associatedtype FactoryType: TypedSupportedTypeFactory
+}
+
+public protocol TypedSupportedTypeFactory {
+    associatedtype BuildType: SupportedPropertyType
+}
+
+public class Factory<BuildType: TypedSupportedType>: TypedSupportedTypeFactory {
+}
+
+public struct ContentMode : TypedSupportedType {
+    public typealias FactoryType = Factory<ContentMode>
+}
+
+func bar<T : SupportedPropertyType>(_: T.Type) {}
+
+func baz<T : TypedSupportedType>(_ t: T.Type) {
+  bar(t)
+}
+
+// This used to recurse infinitely because the base witness table
+// for 'ContentMode : SupportedPropertyType' was incorrectly
+// minimized away.
+baz(ContentMode.self)

validation-test/compiler_crashers_2_fixed/rdar57728533.swift

@@ -0,0 +1,10 @@
+// RUN: not %target-swift-frontend -typecheck %s
+
+protocol Item {
+  associatedtype Rule
+  func isValide(valid: Rule) -> Bool
+}
+
+protocol PairableItem: Item {
+  associatedtype AssociatedItem: PairableItem where AssociatedItem.Rule: Rule
+}

validation-test/compiler_crashers_2_fixed/sr9584.swift

@@ -0,0 +1,14 @@
+// RUN: not %target-swift-frontend -typecheck %s
+
+struct S<N> {}
+
+protocol P {
+  associatedtype A: P = Self
+  static func f(_ x: A) -> A
+}
+
+extension S: P where N: P {
+  static func f<X: P>(_ x: X) -> S<X.A> where A == X, X.A == N {
+    return S<X.A>()
+  }
+}