AST: Fancier getPlaceholderRequirementSignature()

Author: slavapestov

lib/AST/Decl.cpp

@@ -6920,28 +6920,46 @@ ProtocolDecl::getProtocolDependencies() const {
       std::nullopt);
 }
 
-/// If we hit a request cycle, give the protocol a requirement signature where
-/// everything is Copyable and Escapable. Otherwise, we'll get spurious
-/// downstream diagnostics concerning move-only types.
+/// If we hit a request cycle, give the protocol a requirement signature that
+/// still has inherited protocol requirements on Self, and also conformances
+/// to Copyable and Escapable for all associated types. Otherwise, we'll see
+/// invariant violations from the inheritance clause mismatch, as well as
+/// spurious downstream diagnostics concerning move-only types.
 static RequirementSignature getPlaceholderRequirementSignature(
     const ProtocolDecl *proto) {
   auto &ctx = proto->getASTContext();
 
+  SmallVector<ProtocolDecl *, 2> inheritedProtos;
+  for (auto *inheritedProto : proto->getInheritedProtocols()) {
+    inheritedProtos.push_back(inheritedProto);
+  }
+
+  if (ctx.LangOpts.hasFeature(Feature::NoncopyableGenerics)) {
+    for (auto ip : InvertibleProtocolSet::full()) {
+      auto *otherProto = ctx.getProtocol(getKnownProtocolKind(ip));
+      inheritedProtos.push_back(otherProto);
+    }
+  }
+
+  ProtocolType::canonicalizeProtocols(inheritedProtos);
+
   SmallVector<Requirement, 2> requirements;
 
+  for (auto *inheritedProto : inheritedProtos) {
+    requirements.emplace_back(RequirementKind::Conformance,
+                              proto->getSelfInterfaceType(),
+                              inheritedProto->getDeclaredInterfaceType());
+  }
+
   if (ctx.LangOpts.hasFeature(Feature::NoncopyableGenerics)) {
-    auto add = [&](Type type) {
+    for (auto *assocTypeDecl : proto->getAssociatedTypeMembers()) {
       for (auto ip : InvertibleProtocolSet::full()) {
-        auto proto = ctx.getProtocol(getKnownProtocolKind(ip));
-        requirements.emplace_back(RequirementKind::Conformance, type,
-                                  proto->getDeclaredInterfaceType());
+        auto *otherProto = ctx.getProtocol(getKnownProtocolKind(ip));
+        requirements.emplace_back(RequirementKind::Conformance,
+                                  assocTypeDecl->getDeclaredInterfaceType(),
+                                  otherProto->getDeclaredInterfaceType());
       }
-    };
-
-    add(proto->getSelfInterfaceType());
-
-    for (auto *assocTypeDecl : proto->getAssociatedTypeMembers())
-      add(assocTypeDecl->getDeclaredInterfaceType());
+    }
   }
 
   // Maintain invariants.

lib/AST/Type.cpp

@@ -1527,6 +1527,11 @@ static void canonicalizeProtocols(SmallVectorImpl<ProtocolDecl *> &protocols,
     if (proto == nullptr)
       continue;
 
+    // The below algorithm assumes the inheritance graph is acyclic. Just skip
+    // it if we have invalid code.
+    if (proto->hasCircularInheritedProtocols())
+      continue;
+
     // Add the protocols we inherited.
     proto->walkInheritedProtocols([&](ProtocolDecl *inherited) {
       if (inherited == proto)

lib/Sema/TypeCheckDeclPrimary.cpp

@@ -2054,6 +2054,11 @@ static void checkProtocolRefinementRequirements(ProtocolDecl *proto) {
   const bool EnabledNoncopyableGenerics =
       ctx.LangOpts.hasFeature(Feature::NoncopyableGenerics);
 
+  // Check for circular inheritance; the HasCircularInheritedProtocolsRequest
+  // will diagnose an error in that case, and we skip all remaining checks.
+  if (proto->hasCircularInheritedProtocols())
+    return;
+
   // If we make a ~P marking but our protocol Self type still conforms to P,
   // diagnose an error.
   //
@@ -3481,9 +3486,6 @@ class DeclChecker : public DeclVisitor<DeclChecker> {
   void visitProtocolDecl(ProtocolDecl *PD) {
     checkUnsupportedNestedType(PD);
 
-    // Check for circular inheritance within the protocol.
-    (void) PD->hasCircularInheritedProtocols();
-
     TypeChecker::checkDeclAttributes(PD);
 
     // Check that all named primary associated types are valid.

test/Generics/issue-69318.swift

@@ -0,0 +1,19 @@
+// RUN: %target-typecheck-verify-swift
+
+// We used to crash -- https://github.com/apple/swift/issues/69318
+
+public protocol View {
+    associatedtype NodeChildren: ViewGraphNodeChildren
+}
+
+public protocol ViewGraphNodeChildren {
+    associatedtype ParentView: View where ParentView.NodeChildren == Self
+    // expected-note@-1 {{protocol requires nested type 'ParentView'; add nested type 'ParentView' for conformance}}
+}
+
+public protocol ChildlessView: View where NodeChildren == EmptyViewGraphNodeChildren<Self> {}
+// expected-error@-1 {{circular reference}}
+
+public struct EmptyViewGraphNodeChildren<ParentView: ChildlessView>: ViewGraphNodeChildren {}
+// expected-note@-1 3{{through reference here}}
+// expected-error@-2 {{type 'EmptyViewGraphNodeChildren<ParentView>' does not conform to protocol 'ViewGraphNodeChildren'}}

test/Generics/rdar123013710.swift

@@ -0,0 +1,21 @@
+// RUN: %target-typecheck-verify-swift
+
+public protocol P {
+    associatedtype A
+    associatedtype B: P
+
+    var b: B { get }
+    static func f() -> Any?
+}
+
+protocol Q: P where B == Never {} // expected-error {{circular reference}}
+
+extension Never: Q, P { // expected-note 2{{through reference here}}
+   public typealias A = Never
+   public static func f() -> Any? { nil }
+}
+
+extension Q {
+   public var b: Never { fatalError() } // expected-note {{through reference here}}
+}
+