Merge pull request swiftlang#28594 from xedin/rdar-50420029

[ConstraintSystem] Guard against infinite recursion in key path dynam…

Author: xedin

lib/Sema/CSSimplify.cpp

@@ -5291,6 +5291,52 @@ allFromConditionalConformances(DeclContext *DC, Type baseTy,
   });
 }
 
+// Check whether given key path dynamic member lookup is self-recursive,
+// which happens when root type of the key path is the same as base type
+// of the member and lookup is attempted on non-existing property e.g.
+//
+// @dynamicMemberLookup
+// struct Recurse<T> {
+//   subscript<U>(dynamicMember member: KeyPath<Recurse<T>, U>) -> Int {
+//     return 1
+//   }
+// }
+//
+// If we going to lookup any no-existent property or member on `Recursive`
+// using key path dynamic member lookup it would attempt to lookup such
+// member on root type which is also `Recursive` which leads to an infinite
+// recursion.
+static bool isSelfRecursiveKeyPathDynamicMemberLookup(
+    ConstraintSystem &cs, Type keyPathRootTy, ConstraintLocator *locator) {
+  // Let's check whether this is a recursive call to keypath
+  // dynamic member lookup on the same type.
+  if (!locator->isLastElement<LocatorPathElt::KeyPathDynamicMember>())
+    return false;
+
+  auto path = locator->getPath();
+  auto *choiceLoc =
+      cs.getConstraintLocator(locator->getAnchor(), path.drop_back());
+
+  if (auto *overload = cs.findSelectedOverloadFor(choiceLoc)) {
+    auto baseTy = overload->Choice.getBaseType();
+
+    // If it's `Foo<Int>` vs. `Foo<String>` it doesn't really matter
+    // for dynamic lookup because it's going to be performed on `Foo`.
+    if (baseTy->is<BoundGenericType>() &&
+        keyPathRootTy->is<BoundGenericType>()) {
+      auto *baseDecl = baseTy->castTo<BoundGenericType>()->getDecl();
+      auto *keyPathRootDecl =
+          keyPathRootTy->castTo<BoundGenericType>()->getDecl();
+      return baseDecl == keyPathRootDecl;
+    }
+
+    if (baseTy->isEqual(keyPathRootTy))
+      return true;
+  }
+
+  return false;
+}
+
 /// Given a ValueMember, UnresolvedValueMember, or TypeMember constraint,
 /// perform a lookup into the specified base type to find a candidate list.
 /// The list returned includes the viable candidates as well as the unviable
@@ -5758,8 +5804,10 @@ performMemberLookup(ConstraintKind constraintKind, DeclName memberName,
       ::hasDynamicMemberLookupAttribute(instanceTy, DynamicMemberLookupCache)) {
     const auto &candidates = result.ViableCandidates;
 
-    if (candidates.empty() ||
-        allFromConditionalConformances(DC, instanceTy, candidates)) {
+    if ((candidates.empty() ||
+         allFromConditionalConformances(DC, instanceTy, candidates)) &&
+        !isSelfRecursiveKeyPathDynamicMemberLookup(*this, baseTy,
+                                                   memberLocator)) {
       auto &ctx = getASTContext();
 
       // Recursively look up `subscript(dynamicMember:)` methods in this type.

lib/Sema/ConstraintSystem.cpp

@@ -2029,11 +2029,18 @@ std::pair<Type, bool> ConstraintSystem::adjustTypeOfOverloadReference(
     DeclName memberName =
         isSubscriptRef ? DeclBaseName::createSubscript() : choice.getName();
 
-    addValueMemberConstraint(LValueType::get(rootTy), memberName, memberTy,
-                             useDC,
-                             isSubscriptRef ? FunctionRefKind::DoubleApply
-                                            : FunctionRefKind::Unapplied,
-                             /*outerAlternatives=*/{}, keyPathLoc);
+    auto *memberRef = Constraint::createMember(
+        *this, ConstraintKind::ValueMember, LValueType::get(rootTy), memberTy,
+        memberName, useDC,
+        isSubscriptRef ? FunctionRefKind::DoubleApply
+                       : FunctionRefKind::Unapplied,
+        keyPathLoc);
+
+    // Delay simplication of this constraint until after the overload choice
+    // has been bound for this key path dynamic member. This helps to identify
+    // recursive calls with the same base.
+    addUnsolvedConstraint(memberRef);
+    activateConstraint(memberRef);
 
     // In case of subscript things are more compicated comparing to "dot"
     // syntax, because we have to get "applicable function" constraint

test/attr/attr_dynamic_member_lookup.swift

@@ -753,3 +753,15 @@ struct SR11877 {
 }
 
 _ = \SR11877.okay
+
+func test_infinite_self_recursion() {
+  @dynamicMemberLookup
+  struct Recurse<T> {
+    subscript<U>(dynamicMember member: KeyPath<Recurse<T>, U>) -> Int {
+      return 1
+    }
+  }
+
+  _ = Recurse<Int>().foo
+  // expected-error@-1 {{value of type 'Recurse<Int>' has no dynamic member 'foo' using key path from root type 'Recurse<Int>'}}
+}

validation-test/Sema/SwiftUI/rdar57410798.swift

@@ -0,0 +1,43 @@
+// RUN: %target-typecheck-verify-swift -target x86_64-apple-macosx10.15 -swift-version 5
+// REQUIRES: objc_interop
+// REQUIRES: OS=macosx
+
+import SwiftUI
+
+enum ColorScheme: CaseIterable, Hashable, Equatable, Identifiable {
+// expected-error@-1 {{type 'ColorScheme' does not conform to protocol 'Identifiable'}}
+  case `default`
+  case pink
+
+  var foreground: Color {
+    switch self {
+    case .default:
+      return .primary
+    case .pink:
+      return .pink
+    }
+  }
+}
+
+struct IconPicker : View {
+  var body: some View {
+    Text("hello")
+  }
+}
+
+struct CountdownEditor : View {
+  @State var symbol: String = "timer"
+  @State var selectedColor: ColorScheme = ColorScheme.pink
+
+  var body: some View {
+    NavigationLink(destination: IconPicker()) {
+      Text("Icon")
+      Spacer()
+      Image(systemName: symbol)
+        .foregroundColor(selectedColor.color)
+        // expected-error@-1 {{cannot convert value of type 'Binding<Subject>' to expected argument type 'Color?'}}
+        // expected-error@-2 {{referencing subscript 'subscript(dynamicMember:)' requires wrapper 'Binding<ColorScheme>'}}
+        // expected-error@-3 {{value of type 'ColorScheme' has no dynamic member 'color' using key path from root type 'ColorScheme'}}
+    }
+  }
+}