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

In a protocol requirement signature, a conformance requirement on 'Self'
can only be considered redundant if it is derived entirely from other
sources only involving 'Self' as a subject type.

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.

What used to happen is that we would emit a bogus 'redundant conformance'
warning if a refined protocol conformance was also reachable via a
same-type requirement involving 'Self'. Fixing this warning by removing
the requirement could produce code that type checked, however it could
crash at runtime.

In addition to the runtime issue, such 'overly minimized' protocols
could also break name lookup at compile-time, since name lookup
only consults the inheritance clause of a protocol declaration instead
of computing its generic signature to determine the set of refined
protocols.

Now, we will no longer emit the bogus redundant conformance warnings.
Instead, we produce a warning if a protocol is 'overly minimized'.

Since the 'overly minimized' protocols that were obtained by fixing
the bogus warning would in some cases still work, we cannot diagnose
the new violation as an error, to preserve source and ABI compatibility.

Currently this warning fires for the SIMDScalar protocol in the
standard library, and _ErrorCodeProtocol in the Foundation overlay.
We should see if we can fix the protocols in an ABI-compatible way,
or just add a hack to muffle the warning.

Fixes https://bugs.swift.org/browse/SR-8198 / rdar://problem/77358480,
https://bugs.swift.org/browse/SR-10941 / rdar://problem/77358477,
https://bugs.swift.org/browse/SR-11670 / rdar://problem/77358476.

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)