AssociatedTypeInference: Make a cycle breaking hack more principled

We would return an ErrorType without diagnosing anything from
NormalProtocolConformance::getTypeWitnessAndDecl() to signal
a transient condition to associated type inference. But other
callers that receive an ErrorType don't expect that this can
happen, and they in turn won't diagnose anything. So we'd end
up in SILGen with a broken AST.

Instead, push the cycle break up into getWitnessTypeForMatching(),
and remove the hack from getTypeWitnessAndDecl(). If someone
else calls getTypeWitnessAndDecl() and that causes a request
cycle, we will now diagnose the request cycle via the usual
mechanism.

Fixes rdar://120388028.

Author: slavapestov

lib/AST/ProtocolConformance.cpp

@@ -587,17 +587,6 @@ NormalProtocolConformance::getTypeWitnessAndDecl(AssociatedTypeDecl *assocType,
      return { witnessType, nullptr };
   }
 
-  // If this conformance is in a state where it is inferring type witnesses but
-  // we didn't find anything, fail.
-  //
-  // FIXME: This is unsound, because we may not have diagnosed anything but
-  // still end up with an ErrorType in the AST.
-  if (getDeclContext()->getASTContext().evaluator.hasActiveRequest(
-         ResolveTypeWitnessesRequest{
-             const_cast<NormalProtocolConformance *>(this)})) {
-    return { Type(), nullptr };
-  }
-
   return evaluateOrDefault(
       assocType->getASTContext().evaluator,
       TypeWitnessRequest{const_cast<NormalProtocolConformance *>(this),

lib/Sema/AssociatedTypeInference.cpp

@@ -2034,9 +2034,27 @@ static Type getWitnessTypeForMatching(NormalProtocolConformance *conformance,
       return std::nullopt;
     });
 
+    SubstOptions options(std::nullopt);
+    options.getTentativeTypeWitness =
+      [conformance](const NormalProtocolConformance *otherConf,
+             AssociatedTypeDecl *assocType) -> TypeBase * {
+        auto thisProto = conformance->getProtocol();
+        if (conformance == otherConf ||
+            (conformance->getType()->isEqual(otherConf->getType()) &&
+             thisProto->inheritsFrom(otherConf->getProtocol()))) {
+          // Don't attempt to recursively resolve this type witness.
+          return ErrorType::get(assocType->getASTContext()).getPointer();
+        }
+
+        // Okay, this is an unrelated associated type. Proceed to the
+        // usual type witness resolution path.
+        return nullptr;
+      };
+
     // Replace Self with the concrete conforming type.
     substType = substType.subst(QueryTypeSubstitutionMap{substitutions},
-                                LookUpConformanceInModule());
+                                LookUpConformanceInModule(),
+                                options);
 
     // If we don't have enough type witnesses, leave it abstract.
     if (substType->hasError())

test/AssociatedTypeInference/type_witness_request_cycle.swift

@@ -0,0 +1,23 @@
+// RUN: %target-typecheck-verify-swift
+
+// This used to crash -- rdar://120388028
+
+struct Node {}
+
+let x = Outer(implementation: Inner<Node>())
+
+protocol Proto {
+    associatedtype Node
+    func doThings(things: Outer<Node, Self>)
+}
+
+struct Outer<Node, Impl: Proto> where Impl.Node == Node {
+    var implementation: Impl
+}
+
+struct Inner<Node>: Proto { // expected-error {{circular reference}}
+// expected-note@-1 {{through reference here}}
+    func doThings(things: Outer<Node, Inner<Node>>) {}
+    // expected-note@-1 2{{through reference here}}
+    // expected-note@-2 {{while resolving type 'Outer<Node, Inner<Node>>'}}
+}

test/decl/protocol/typealias_inference.swift

@@ -12,10 +12,13 @@ protocol BaseProtocol {
   init(closure: Closure)
 }
 
-struct Base<Value>: BaseProtocol {
+struct Base<Value>: BaseProtocol { // expected-error {{circular reference}}
+// expected-note@-1 {{through reference here}}
   private let closure: Closure
 
-  init(closure: Closure) { //expected-error {{reference to invalid type alias 'Closure' of type 'Base<Value>'}}
+  init(closure: Closure) {
+  // expected-note@-1 {{while resolving type 'Closure'}}
+  // expected-note@-2 2{{through reference here}}
     self.closure = closure
   }
 }

validation-test/compiler_crashers_2_fixed/0127-issue-48118.swift

@@ -28,12 +28,15 @@ extension P {
     }
 }
 
-public struct S: P {
+public struct S: P { // expected-error {{circular reference}}
+// expected-note@-1 {{through reference here}}
     public typealias PA = String
     public typealias PB = Int
 
     public func f1(_ x: Context, _ y: PA) {
     }
-    public func f2(_ x: Context, _ y: PB) { // expected-error {{reference to invalid type alias 'Context' of type 'S'}}
+    public func f2(_ x: Context, _ y: PB) {
+    // expected-note@-1 2{{through reference here}}
+    // expected-note@-2 {{while resolving type 'Context'}}
     }
 }