Sema: Associated type inference skips witnesses that might trigger a request cycle

This implements a structural walk over the TypeRepr to catch
situations where we attempt to infer `A` from `func f(_: A)`,
which references the concrete `A` that will be synthesized
in the conforming type.

Fixes:
- rdar://34956654 / #48680
- rdar://38913692 / #49066
- rdar://56672411
- #50010
- rdar://81587765 / #57355
- rdar://117442510

Author: slavapestov

lib/Sema/TypeCheckProtocolInference.cpp

@@ -161,6 +161,127 @@ static bool associatedTypesAreSameEquivalenceClass(AssociatedTypeDecl *a,
   return false;
 }
 
+namespace {
+
+/// Try to avoid situations where resolving the type of a witness calls back
+/// into associated type inference.
+struct TypeReprCycleCheckWalker : ASTWalker {
+  llvm::SmallDenseSet<Identifier, 2> circularNames;
+  ValueDecl *witness;
+  bool found;
+
+  TypeReprCycleCheckWalker(
+      const llvm::SetVector<AssociatedTypeDecl *> &allUnresolved)
+    : witness(nullptr), found(false) {
+    for (auto *assocType : allUnresolved) {
+      circularNames.insert(assocType->getName());
+    }
+  }
+
+  PreWalkAction walkToTypeReprPre(TypeRepr *T) override {
+    // FIXME: We should still visit any generic arguments of this member type.
+    // However, we want to skip 'Foo.Element' because the 'Element' reference is
+    // not unqualified.
+    if (auto *memberTyR = dyn_cast<MemberTypeRepr>(T)) {
+      return Action::SkipChildren();
+    }
+
+    if (auto *identTyR = dyn_cast<SimpleIdentTypeRepr>(T)) {
+      if (circularNames.count(identTyR->getNameRef().getBaseIdentifier()) > 0) {
+        // If unqualified lookup can find a type with this name without looking
+        // into protocol members, don't skip the witness, since this type might
+        // be a candidate witness.
+        auto desc = UnqualifiedLookupDescriptor(
+            identTyR->getNameRef(), witness->getDeclContext(),
+            identTyR->getLoc(), UnqualifiedLookupOptions());
+
+        auto &ctx = witness->getASTContext();
+        auto results =
+            evaluateOrDefault(ctx.evaluator, UnqualifiedLookupRequest{desc}, {});
+
+        // Ok, resolving this name would trigger associated type inference
+        // recursively. We're going to skip this witness.
+        if (results.allResults().empty()) {
+          found = true;
+          return Action::Stop();
+        }
+      }
+    }
+
+    return Action::Continue();
+  }
+
+  bool checkForPotentialCycle(ValueDecl *witness) {
+    // Don't do this for protocol extension members, because we have a
+    // mini "solver" that avoids similar issues instead.
+    if (witness->getDeclContext()->getSelfProtocolDecl() != nullptr)
+      return false;
+
+    // If we already have an interface type, don't bother trying to
+    // avoid a cycle.
+    if (witness->hasInterfaceType())
+      return false;
+
+    // We call checkForPotentailCycle() multiple times with
+    // different witnesses.
+    found = false;
+    this->witness = witness;
+
+    auto walkInto = [&](TypeRepr *tyR) {
+      if (tyR)
+        tyR->walk(*this);
+      return found;
+    };
+
+    if (auto *AFD = dyn_cast<AbstractFunctionDecl>(witness)) {
+      for (auto *param : *AFD->getParameters()) {
+        if (walkInto(param->getTypeRepr()))
+          return true;
+      }
+
+      if (auto *FD = dyn_cast<FuncDecl>(witness)) {
+        if (walkInto(FD->getResultTypeRepr()))
+          return true;
+      }
+
+      return false;
+    }
+
+    if (auto *SD = dyn_cast<SubscriptDecl>(witness)) {
+      for (auto *param : *SD->getIndices()) {
+        if (walkInto(param->getTypeRepr()))
+          return true;
+      }
+
+      if (walkInto(SD->getElementTypeRepr()))
+        return true;
+
+      return false;
+    }
+
+    if (auto *VD = dyn_cast<VarDecl>(witness)) {
+      if (walkInto(VD->getTypeReprOrParentPatternTypeRepr()))
+        return true;
+
+      return false;
+    }
+
+    if (auto *EED = dyn_cast<EnumElementDecl>(witness)) {
+      for (auto *param : *EED->getParameterList()) {
+        if (walkInto(param->getTypeRepr()))
+          return true;
+      }
+
+      return false;
+    }
+
+    assert(false && "Should be exhaustive");
+    return false;
+  }
+};
+
+}
+
 InferredAssociatedTypesByWitnesses
 AssociatedTypeInference::inferTypeWitnessesViaValueWitnesses(
                     ConformanceChecker &checker,
@@ -176,11 +297,13 @@ AssociatedTypeInference::inferTypeWitnessesViaValueWitnesses(
     abort();
   }
 
+  TypeReprCycleCheckWalker cycleCheck(allUnresolved);
+
   InferredAssociatedTypesByWitnesses result;
 
   auto isExtensionUsableForInference = [&](const ExtensionDecl *extension) {
     // The context the conformance being checked is declared on.
-    const auto conformanceCtx = checker.Conformance->getDeclContext();
+    const auto conformanceCtx = conformance->getDeclContext();
     if (extension == conformanceCtx)
       return true;
 
@@ -250,11 +373,17 @@ AssociatedTypeInference::inferTypeWitnessesViaValueWitnesses(
     // If the potential witness came from an extension, and our `Self`
     // type can't use it regardless of what associated types we end up
     // inferring, skip the witness.
-    if (auto extension = dyn_cast<ExtensionDecl>(witness->getDeclContext()))
+    if (auto extension = dyn_cast<ExtensionDecl>(witness->getDeclContext())) {
       if (!isExtensionUsableForInference(extension)) {
         LLVM_DEBUG(llvm::dbgs() << "Extension not usable for inference\n");
         continue;
       }
+    }
+
+    if (cycleCheck.checkForPotentialCycle(witness)) {
+      LLVM_DEBUG(llvm::dbgs() << "Skipping witness to avoid request cycle\n");
+      continue;
+    }
 
     // Try to resolve the type witness via this value witness.
     auto witnessResult = inferTypeWitnessesViaValueWitness(req, witness);

test/decl/protocol/req/assoc_type_inference_cycle.swift

@@ -0,0 +1,97 @@
+// RUN: %target-typecheck-verify-swift
+// RUN: %target-swift-frontend -emit-silgen %s -parse-as-library -module-name Test -experimental-lazy-typecheck
+
+// This file should type check successfully.
+
+// rdar://117442510
+public protocol P1 {
+  associatedtype Value
+
+  func makeValue() -> Value
+  func useProducedValue(_ produceValue: () -> Value)
+}
+
+public typealias A1 = S1.Value
+
+public struct S1: P1 {
+  public func makeValue() -> Int { return 1 }
+  public func useProducedValue(_ produceValue: () -> Value) {
+    _ = produceValue()
+  }
+}
+
+// rdar://56672411
+public protocol P2 {
+  associatedtype X = Int
+  func foo(_ x: X)
+}
+
+public typealias A2 = S2.X
+
+public struct S2: P2 {
+  public func bar(_ x: X) {}
+  public func foo(_ x: X) {}
+}
+
+// https://github.com/apple/swift/issues/57355
+public protocol P3 {
+  associatedtype T
+  var a: T { get }
+  var b: T { get }
+  var c: T { get }
+}
+
+public typealias A3 = S3.T
+
+public struct S3: P3 {
+  public let a: Int
+  public let b: T
+  public let c: T
+}
+
+// Regression tests
+public protocol P4 {
+  associatedtype A
+  func f(_: A)
+}
+
+public typealias A = Int
+
+public typealias A4 = S4.A
+
+public struct S4: P4 {
+  public func f(_: A) {}
+}
+
+public typealias A5 = OuterGeneric<Int>.Inner.A
+
+public struct OuterGeneric<A> {
+  public struct Inner: P4 {
+    public func f(_: A) {  }
+  }
+}
+
+public typealias A6 = OuterNested.Inner.A
+
+public struct OuterNested {
+  public struct A {}
+
+  public struct Inner: P4 {
+    public func f(_: A) {}
+  }
+}
+
+public protocol CaseProtocol {
+  associatedtype A = Int
+  static func a(_: A) -> Self
+  static func b(_: A) -> Self
+  static func c(_: A) -> Self
+}
+
+public typealias A7 = CaseWitness.A
+
+public enum CaseWitness: CaseProtocol {
+  case a(_: A)
+  case b(_: A)
+  case c(_: A)
+}

validation-test/compiler_crashers_2_fixed/0124-issue-48395.swift

@@ -1,18 +1,18 @@
-// RUN: %target-typecheck-verify-swift
+// RUN: %target-swift-frontend -emit-ir %s
 
 // https://github.com/apple/swift/issues/48395
 
-struct DefaultAssociatedType {
+public struct DefaultAssociatedType {
 }
 
 protocol Protocol {
     associatedtype AssociatedType = DefaultAssociatedType
     init(object: AssociatedType)
 }
 
-final class Conformance: Protocol {
+public final class Conformance: Protocol {
     private let object: AssociatedType
-    init(object: AssociatedType) { // expected-error {{reference to invalid associated type 'AssociatedType' of type 'Conformance'}}
+    public init(object: AssociatedType) {
         self.object = object
     }
 }

validation-test/compiler_crashers_2_fixed/0126-issue-48464.swift

@@ -1,31 +1,30 @@
-// RUN: %target-typecheck-verify-swift
+// RUN: %target-swift-frontend -emit-ir %s
 
 // https://github.com/apple/swift/issues/48464
 
-protocol VectorIndex {
+public protocol VectorIndex {
     associatedtype Vector8 : Vector where Vector8.Index == Self, Vector8.Element == UInt8
 }
-enum VectorIndex1 : VectorIndex {
+public enum VectorIndex1 : VectorIndex {
     case i0
-    typealias Vector8 = Vector1<UInt8>
+    public typealias Vector8 = Vector1<UInt8>
 }
-protocol Vector {
+public protocol Vector {
     associatedtype Index: VectorIndex
     associatedtype Element
     init(elementForIndex: (Index) -> Element)
     subscript(index: Index) -> Element { get set }
 }
-struct Vector1<Element> : Vector {
-    //typealias Index = VectorIndex1 // Uncomment this line to workaround bug.
-    var e0: Element
-    init(elementForIndex: (VectorIndex1) -> Element) {
+public struct Vector1<Element> : Vector {
+    public var e0: Element
+    public init(elementForIndex: (VectorIndex1) -> Element) {
         e0 = elementForIndex(.i0)
     }
-    subscript(index: Index) -> Element { // expected-error {{reference to invalid associated type 'Index' of type 'Vector1<Element>'}}
+    public subscript(index: Index) -> Element {
         get { return e0 }
         set { e0 = newValue }
     }
 }
 extension Vector where Index == VectorIndex1 {
-    init(_ e0: Element) { fatalError() }
+    public init(_ e0: Element) { fatalError() }
 }