Skip to content

Commit 1d393eb

Browse files
committed
[ConstraintSystem] Guard against infinite recursion in key path dynamic member lookup
It's possible to construct subscript member responsible for key path dynamic member lookup in a way which is going to be self-recursive and an attempt to lookup any non-existent member is going to trigger infine recursion. Let's guard against that by making sure that the base type of the member lookup is different from root type of the key path. Resolves: rdar://problem/50420029 Resolves: rdar://problem/57410798
1 parent 57f7f07 commit 1d393eb

File tree

4 files changed

+117
-7
lines changed

4 files changed

+117
-7
lines changed

lib/Sema/CSSimplify.cpp

Lines changed: 50 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5291,6 +5291,52 @@ allFromConditionalConformances(DeclContext *DC, Type baseTy,
52915291
});
52925292
}
52935293

5294+
// Check whether given key path dynamic member lookup is self-recursive,
5295+
// which happens when root type of the key path is the same as base type
5296+
// of the member and lookup is attempted on non-existing property e.g.
5297+
//
5298+
// @dynamicMemberLookup
5299+
// struct Recurse<T> {
5300+
// subscript<U>(dynamicMember member: KeyPath<Recurse<T>, U>) -> Int {
5301+
// return 1
5302+
// }
5303+
// }
5304+
//
5305+
// If we going to lookup any no-existent property or member on `Recursive`
5306+
// using key path dynamic member lookup it would attempt to lookup such
5307+
// member on root type which is also `Recursive` which leads to an infinite
5308+
// recursion.
5309+
static bool isSelfRecursiveKeyPathDynamicMemberLookup(
5310+
ConstraintSystem &cs, Type keyPathRootTy, ConstraintLocator *locator) {
5311+
// Let's check whether this is a recursive call to keypath
5312+
// dynamic member lookup on the same type.
5313+
if (!locator->isLastElement<LocatorPathElt::KeyPathDynamicMember>())
5314+
return false;
5315+
5316+
auto path = locator->getPath();
5317+
auto *choiceLoc =
5318+
cs.getConstraintLocator(locator->getAnchor(), path.drop_back());
5319+
5320+
if (auto *overload = cs.findSelectedOverloadFor(choiceLoc)) {
5321+
auto baseTy = overload->Choice.getBaseType();
5322+
5323+
// If it's `Foo<Int>` vs. `Foo<String>` it doesn't really matter
5324+
// for dynamic lookup because it's going to be performed on `Foo`.
5325+
if (baseTy->is<BoundGenericType>() &&
5326+
keyPathRootTy->is<BoundGenericType>()) {
5327+
auto *baseDecl = baseTy->castTo<BoundGenericType>()->getDecl();
5328+
auto *keyPathRootDecl =
5329+
keyPathRootTy->castTo<BoundGenericType>()->getDecl();
5330+
return baseDecl == keyPathRootDecl;
5331+
}
5332+
5333+
if (baseTy->isEqual(keyPathRootTy))
5334+
return true;
5335+
}
5336+
5337+
return false;
5338+
}
5339+
52945340
/// Given a ValueMember, UnresolvedValueMember, or TypeMember constraint,
52955341
/// perform a lookup into the specified base type to find a candidate list.
52965342
/// The list returned includes the viable candidates as well as the unviable
@@ -5758,8 +5804,10 @@ performMemberLookup(ConstraintKind constraintKind, DeclName memberName,
57585804
::hasDynamicMemberLookupAttribute(instanceTy, DynamicMemberLookupCache)) {
57595805
const auto &candidates = result.ViableCandidates;
57605806

5761-
if (candidates.empty() ||
5762-
allFromConditionalConformances(DC, instanceTy, candidates)) {
5807+
if ((candidates.empty() ||
5808+
allFromConditionalConformances(DC, instanceTy, candidates)) &&
5809+
!isSelfRecursiveKeyPathDynamicMemberLookup(*this, baseTy,
5810+
memberLocator)) {
57635811
auto &ctx = getASTContext();
57645812

57655813
// Recursively look up `subscript(dynamicMember:)` methods in this type.

lib/Sema/ConstraintSystem.cpp

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2029,11 +2029,18 @@ std::pair<Type, bool> ConstraintSystem::adjustTypeOfOverloadReference(
20292029
DeclName memberName =
20302030
isSubscriptRef ? DeclBaseName::createSubscript() : choice.getName();
20312031

2032-
addValueMemberConstraint(LValueType::get(rootTy), memberName, memberTy,
2033-
useDC,
2034-
isSubscriptRef ? FunctionRefKind::DoubleApply
2035-
: FunctionRefKind::Unapplied,
2036-
/*outerAlternatives=*/{}, keyPathLoc);
2032+
auto *memberRef = Constraint::createMember(
2033+
*this, ConstraintKind::ValueMember, LValueType::get(rootTy), memberTy,
2034+
memberName, useDC,
2035+
isSubscriptRef ? FunctionRefKind::DoubleApply
2036+
: FunctionRefKind::Unapplied,
2037+
keyPathLoc);
2038+
2039+
// Delay simplication of this constraint until after the overload choice
2040+
// has been bound for this key path dynamic member. This helps to identify
2041+
// recursive calls with the same base.
2042+
addUnsolvedConstraint(memberRef);
2043+
activateConstraint(memberRef);
20372044

20382045
// In case of subscript things are more compicated comparing to "dot"
20392046
// syntax, because we have to get "applicable function" constraint

test/attr/attr_dynamic_member_lookup.swift

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -753,3 +753,15 @@ struct SR11877 {
753753
}
754754

755755
_ = \SR11877.okay
756+
757+
func test_infinite_self_recursion() {
758+
@dynamicMemberLookup
759+
struct Recurse<T> {
760+
subscript<U>(dynamicMember member: KeyPath<Recurse<T>, U>) -> Int {
761+
return 1
762+
}
763+
}
764+
765+
_ = Recurse<Int>().foo
766+
// expected-error@-1 {{value of type 'Recurse<Int>' has no dynamic member 'foo' using key path from root type 'Recurse<Int>'}}
767+
}
Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
// RUN: %target-typecheck-verify-swift -target x86_64-apple-macosx10.15 -swift-version 5
2+
// REQUIRES: objc_interop
3+
// REQUIRES: OS=macosx
4+
5+
import SwiftUI
6+
7+
enum ColorScheme: CaseIterable, Hashable, Equatable, Identifiable {
8+
// expected-error@-1 {{type 'ColorScheme' does not conform to protocol 'Identifiable'}}
9+
case `default`
10+
case pink
11+
12+
var foreground: Color {
13+
switch self {
14+
case .default:
15+
return .primary
16+
case .pink:
17+
return .pink
18+
}
19+
}
20+
}
21+
22+
struct IconPicker : View {
23+
var body: some View {
24+
Text("hello")
25+
}
26+
}
27+
28+
struct CountdownEditor : View {
29+
@State var symbol: String = "timer"
30+
@State var selectedColor: ColorScheme = ColorScheme.pink
31+
32+
var body: some View {
33+
NavigationLink(destination: IconPicker()) {
34+
Text("Icon")
35+
Spacer()
36+
Image(systemName: symbol)
37+
.foregroundColor(selectedColor.color)
38+
// expected-error@-1 {{cannot convert value of type 'Binding<Subject>' to expected argument type 'Color?'}}
39+
// expected-error@-2 {{referencing subscript 'subscript(dynamicMember:)' requires wrapper 'Binding<ColorScheme>'}}
40+
// expected-error@-3 {{value of type 'ColorScheme' has no dynamic member 'color' using key path from root type 'ColorScheme'}}
41+
}
42+
}
43+
}

0 commit comments

Comments
 (0)