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 / apple#48680
- rdar://38913692 / apple#49066
- rdar://56672411
- apple#50010
- rdar://81587765 / apple#57355
- rdar://117442510

lib/Sema/TypeCheckProtocolInference.cpp

@@ -161,6 +161,121 @@ 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;
+  bool found;
+
+  TypeReprCycleCheckWalker(
+      const llvm::SetVector<AssociatedTypeDecl *> &allUnresolved,
+      DeclContext *conformanceDC) : found(false) {
+    // Disallow unqualified references to associated types we're attempting
+    // to infer right now.
+    for (auto *assocType : allUnresolved) {
+      circularNames.insert(assocType->getName());
+    }
+
+    // But, allow unqualified references to generic parameters.
+    auto conformanceSig = conformanceDC->getGenericSignatureOfContext();
+    for (auto paramTy : conformanceSig.getGenericParams()) {
+      circularNames.erase(paramTy->getName());
+    }
+
+    // Also allow unqualified references to member types from the outer context.
+    while (conformanceDC->isTypeContext()) {
+      for (auto *assocType : allUnresolved) {
+        auto results = conformanceDC->getSelfNominalTypeDecl()
+            ->lookupDirect(assocType->getName());
+        if (results.size() > 0)
+          circularNames.erase(assocType->getName());
+      }
+
+      conformanceDC = conformanceDC->getParent();
+    }
+  }
+
+  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) {
+        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;
+
+    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;
+    }
+
+    assert(false && "Should be exhaustive");
+    return false;
+  }
+};
+
+}
+
 InferredAssociatedTypesByWitnesses
 AssociatedTypeInference::inferTypeWitnessesViaValueWitnesses(
                     ConformanceChecker &checker,
@@ -176,11 +291,14 @@ AssociatedTypeInference::inferTypeWitnessesViaValueWitnesses(
     abort();
   }
 
+  TypeReprCycleCheckWalker cycleCheck(
+      allUnresolved, conformance->getDeclContext());
+
   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 +368,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,44 @@
+// 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 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 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 struct S3: P3 {
+  public let a: Int
+  public let b: T
+  public let c: T
+}

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

@@ -12,7 +12,7 @@ protocol Protocol {
 
 final class Conformance: Protocol {
     private let object: AssociatedType
-    init(object: AssociatedType) { // expected-error {{reference to invalid associated type 'AssociatedType' of type 'Conformance'}}
+    init(object: AssociatedType) {
         self.object = object
     }
 }

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

@@ -16,12 +16,11 @@ protocol Vector {
     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) {
         e0 = elementForIndex(.i0)
     }
-    subscript(index: Index) -> Element { // expected-error {{reference to invalid associated type 'Index' of type 'Vector1<Element>'}}
+    subscript(index: Index) -> Element {
         get { return e0 }
         set { e0 = newValue }
     }