Skip to content

Commit 2863b6c

Browse files
dan-zhengdan-zheng
authored
Gardening for @dynamicMemberLookup. (#20498)
* Gardening for `@dynamicMemberLookup`. - Unify code style of `@dynamicMemberLookup` and `@dynamicCallable` implementations. - Use consistent variable names, diagnostic messages, doc comments, etc. - Move `@dynamicMemberLookup` test to `test/attr` directory.
1 parent 1ee3bcd commit 2863b6c

File tree

9 files changed

+84
-89
lines changed

9 files changed

+84
-89
lines changed

‎include/swift/AST/DiagnosticsSema.def

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1056,9 +1056,10 @@ ERROR(missing_dynamic_callable_kwargs_method,none,
10561056
"@dynamicCallable type %0 cannot be applied with keyword arguments; "
10571057
"missing 'dynamicCall(withKeywordArguments:)' method", (Type))
10581058

1059-
ERROR(type_invalid_dml,none,
1059+
ERROR(invalid_dynamic_member_lookup_type,none,
10601060
"@dynamicMemberLookup attribute requires %0 to have a "
1061-
"'subscript(dynamicMember:)' member with a string index", (Type))
1061+
"'subscript(dynamicMember:)' method with an "
1062+
"'ExpressibleByStringLiteral' parameter", (Type))
10621063

10631064
ERROR(string_index_not_integer,none,
10641065
"String must not be indexed with %0, it has variable size elements",

‎lib/Sema/CSApply.cpp

Lines changed: 18 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -393,20 +393,20 @@ diagnoseInvalidDynamicConstructorReferences(ConstraintSystem &cs,
393393
}
394394

395395
/// Form a type checked expression for the index of a @dynamicMemberLookup
396-
/// subscript index expression. This will have tuple type of (dynamicMember:T).
397-
static Expr *getDMLIndexExpr(StringRef name, Type ty, SourceLoc loc,
398-
DeclContext *dc, ConstraintSystem &cs) {
396+
/// subscript index parameter.
397+
/// The index expression will have a tuple type of `(dynamicMember: T)`.
398+
static Expr *buildDynamicMemberLookupIndexExpr(StringRef name, Type ty,
399+
SourceLoc loc, DeclContext *dc,
400+
ConstraintSystem &cs) {
399401
auto &ctx = cs.TC.Context;
400402

401403
// Build and type check the string literal index value to the specific
402404
// string type expected by the subscript.
403-
Expr *nameExpr = new (ctx)
404-
StringLiteralExpr(name, loc, /*implicit*/true);
405-
406-
407-
// Build a tuple so that argument has a label.
408-
Expr *tuple = TupleExpr::create(ctx, loc, nameExpr, ctx.Id_dynamicMember, loc,
409-
loc, /*hasTrailingClosure*/false,
405+
Expr *nameExpr = new (ctx) StringLiteralExpr(name, loc, /*implicit*/true);
406+
407+
// Build a tuple so that the argument has a label.
408+
Expr *tuple = TupleExpr::create(ctx, loc, nameExpr, ctx.Id_dynamicMember,
409+
loc, loc, /*hasTrailingClosure*/false,
410410
/*implicit*/true);
411411
(void)cs.TC.typeCheckExpression(tuple, dc, TypeLoc::withoutLoc(ty),
412412
CTP_CallArgument);
@@ -1524,8 +1524,7 @@ namespace {
15241524
// Check whether the base is 'super'.
15251525
bool isSuper = base->isSuperExpr();
15261526

1527-
// Use the correct kind of locator depending on how this subscript came
1528-
// to be.
1527+
// Use the correct locator kind based on the subscript kind.
15291528
auto locatorKind = ConstraintLocator::SubscriptMember;
15301529
if (choice.getKind() == OverloadChoiceKind::DynamicMemberLookup)
15311530
locatorKind = ConstraintLocator::Member;
@@ -2769,7 +2768,8 @@ namespace {
27692768
// Build and type check the string literal index value to the specific
27702769
// string type expected by the subscript.
27712770
auto fieldName = selected.choice.getName().getBaseIdentifier().str();
2772-
auto index = getDMLIndexExpr(fieldName, tupleTy, loc, dc, cs);
2771+
auto index =
2772+
buildDynamicMemberLookupIndexExpr(fieldName, tupleTy, loc, dc, cs);
27732773

27742774
// Build and return a subscript that uses this string as the index.
27752775
return buildSubscript(base, index, ctx.Id_dynamicMember,
@@ -4388,16 +4388,17 @@ namespace {
43884388
// through the subscript(dynamicMember:) member, restore the
43894389
// openedType and origComponent to its full reference as if the user
43904390
// wrote out the subscript manually.
4391-
if (foundDecl->choice.getKind()
4392-
== OverloadChoiceKind::DynamicMemberLookup) {
4391+
if (foundDecl->choice.getKind() ==
4392+
OverloadChoiceKind::DynamicMemberLookup) {
43934393
foundDecl->openedType = foundDecl->openedFullType
4394-
->castTo<AnyFunctionType>()->getResult();
4394+
->castTo<AnyFunctionType>()->getResult();
43954395

43964396
auto &ctx = cs.TC.Context;
43974397
auto loc = origComponent.getLoc();
43984398
auto fieldName =
43994399
foundDecl->choice.getName().getBaseIdentifier().str();
4400-
auto index = getDMLIndexExpr(fieldName, indexType, loc, dc, cs);
4400+
auto index = buildDynamicMemberLookupIndexExpr(fieldName, indexType,
4401+
loc, dc, cs);
44014402

44024403
origComponent = KeyPathExpr::Component::
44034404
forUnresolvedSubscript(ctx, loc, index, {}, loc, loc,

‎lib/Sema/CSDiagnostics.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -664,7 +664,8 @@ bool RValueTreatedAsLValueFailure::diagnoseAsError() {
664664
}
665665

666666
if (auto resolvedOverload = getResolvedOverload(getLocator()))
667-
if (resolvedOverload->Choice.getKind() == OverloadChoiceKind::DynamicMemberLookup)
667+
if (resolvedOverload->Choice.getKind() ==
668+
OverloadChoiceKind::DynamicMemberLookup)
668669
subElementDiagID = diag::assignment_dynamic_property_has_immutable_base;
669670
} else if (auto sub = dyn_cast<SubscriptExpr>(diagExpr)) {
670671
subElementDiagID = diag::assignment_subscript_has_immutable_base;

‎lib/Sema/CSSimplify.cpp

Lines changed: 16 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -3084,17 +3084,17 @@ getArgumentLabels(ConstraintSystem &cs, ConstraintLocatorBuilder locator) {
30843084
/// conformances, but this is fine because it doesn't get invoked in the normal
30853085
/// name lookup path (only when lookup is about to fail).
30863086
static bool hasDynamicMemberLookupAttribute(CanType ty,
3087-
llvm::DenseMap<CanType, bool> &IsDynamicMemberLookupCache) {
3088-
auto it = IsDynamicMemberLookupCache.find(ty);
3089-
if (it != IsDynamicMemberLookupCache.end()) return it->second;
3087+
llvm::DenseMap<CanType, bool> &DynamicMemberLookupCache) {
3088+
auto it = DynamicMemberLookupCache.find(ty);
3089+
if (it != DynamicMemberLookupCache.end()) return it->second;
30903090

30913091
auto calculate = [&]()-> bool {
30923092
// If this is a protocol composition, check to see if any of the protocols
30933093
// have the attribute on them.
30943094
if (auto protocolComp = ty->getAs<ProtocolCompositionType>()) {
30953095
for (auto p : protocolComp->getMembers())
30963096
if (hasDynamicMemberLookupAttribute(p->getCanonicalType(),
3097-
IsDynamicMemberLookupCache))
3097+
DynamicMemberLookupCache))
30983098
return true;
30993099
return false;
31003100
}
@@ -3103,8 +3103,7 @@ static bool hasDynamicMemberLookupAttribute(CanType ty,
31033103
auto nominal = ty->getAnyNominal();
31043104
if (!nominal) return false; // Dynamic lookups don't exist on tuples, etc.
31053105

3106-
// If any of the protocols this type conforms to has the attribute, then
3107-
// yes.
3106+
// If this type conforms to a protocol with the attribute, then return true.
31083107
for (auto p : nominal->getAllProtocols())
31093108
if (p->getAttrs().hasAttribute<DynamicMemberLookupAttr>())
31103109
return true;
@@ -3136,7 +3135,7 @@ static bool hasDynamicMemberLookupAttribute(CanType ty,
31363135

31373136
// Cache this if we can.
31383137
if (!ty->hasTypeVariable())
3139-
IsDynamicMemberLookupCache[ty] = result;
3138+
DynamicMemberLookupCache[ty] = result;
31403139

31413140
return result;
31423141
}
@@ -3546,32 +3545,32 @@ performMemberLookup(ConstraintKind constraintKind, DeclName memberName,
35463545
}
35473546

35483547
// If we're about to fail lookup, but we are looking for members in a type
3549-
// that has the @dynamicMemberLookup attribute, then we resolve the reference
3550-
// to the subscript(dynamicMember:) member, and pass the member name as a
3551-
// string.
3548+
// with the @dynamicMemberLookup attribute, then we resolve a reference
3549+
// to a `subscript(dynamicMember:)` method and pass the member name as a
3550+
// string parameter.
35523551
if (result.ViableCandidates.empty() &&
35533552
constraintKind == ConstraintKind::ValueMember &&
35543553
memberName.isSimpleName() && !memberName.isSpecial()) {
35553554
auto name = memberName.getBaseIdentifier();
35563555
if (hasDynamicMemberLookupAttribute(instanceTy->getCanonicalType(),
3557-
IsDynamicMemberLookupCache)) {
3556+
DynamicMemberLookupCache)) {
35583557
auto &ctx = getASTContext();
3559-
// Recursively look up the subscript(dynamicMember:)'s in this type.
3558+
3559+
// Recursively look up `subscript(dynamicMember:)` methods in this type.
35603560
auto subscriptName =
35613561
DeclName(ctx, DeclBaseName::createSubscript(), ctx.Id_dynamicMember);
3562-
35633562
auto subscripts = performMemberLookup(constraintKind,
35643563
subscriptName,
35653564
baseTy, functionRefKind,
35663565
memberLocator,
35673566
includeInaccessibleMembers);
35683567

3569-
// Reflect the candidates found as DynamicMemberLookup results.
3568+
// Reflect the candidates found as `DynamicMemberLookup` results.
35703569
for (auto candidate : subscripts.ViableCandidates) {
35713570
auto decl = cast<SubscriptDecl>(candidate.getDecl());
3572-
if (isAcceptableDynamicMemberLookupSubscript(decl, DC, TC))
3573-
result.addViable(OverloadChoice::getDynamicMemberLookup(baseTy,
3574-
decl, name));
3571+
if (isValidDynamicMemberLookupSubscript(decl, DC, TC))
3572+
result.addViable(
3573+
OverloadChoice::getDynamicMemberLookup(baseTy, decl, name));
35753574
}
35763575
for (auto candidate : subscripts.UnviableCandidates) {
35773576
auto decl = candidate.first.getDecl();

‎lib/Sema/ConstraintSystem.h

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1085,10 +1085,9 @@ class ConstraintSystem {
10851085
/// types.
10861086
llvm::DenseMap<CanType, DynamicCallableMethods> DynamicCallableCache;
10871087

1088-
/// This is a cache that keeps track of whether a given type is known (or not)
1089-
/// to be a @dynamicMemberLookup type.
1090-
///
1091-
llvm::DenseMap<CanType, bool> IsDynamicMemberLookupCache;
1088+
/// A cache that stores whether types are valid @dynamicMemberLookup types.
1089+
llvm::DenseMap<CanType, bool> DynamicMemberLookupCache;
1090+
10921091
private:
10931092
/// \brief Describe the candidate expression for partial solving.
10941093
/// This class used by shrink & solve methods which apply

‎lib/Sema/OverloadChoice.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -183,7 +183,8 @@ class OverloadChoice {
183183
}
184184

185185
/// Retrieve an overload choice for a declaration that was found via
186-
/// dynamic lookup. The ValueDecl is the subscript(dynamicMember:)
186+
/// dynamic member lookup. The `ValueDecl` is a `subscript(dynamicMember:)`
187+
/// method.
187188
static OverloadChoice getDynamicMemberLookup(Type base, ValueDecl *value,
188189
Identifier name) {
189190
OverloadChoice result;

‎lib/Sema/TypeCheckAttr.cpp

Lines changed: 28 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -949,11 +949,11 @@ static bool isRelaxedIBAction(TypeChecker &TC) {
949949
return isiOS(TC) || iswatchOS(TC);
950950
}
951951

952-
/// Returns true if a method is an valid implementation of a @dynamicCallable
953-
/// attribute requirement. The method is given to be defined as one of the
954-
/// following: `dynamicallyCall(withArguments:)` or
952+
/// Returns true if the given method is an valid implementation of a
953+
/// @dynamicCallable attribute requirement. The method is given to be defined
954+
/// as one of the following: `dynamicallyCall(withArguments:)` or
955955
/// `dynamicallyCall(withKeywordArguments:)`.
956-
bool swift::isValidDynamicCallableMethod(FuncDecl *funcDecl, DeclContext *DC,
956+
bool swift::isValidDynamicCallableMethod(FuncDecl *decl, DeclContext *DC,
957957
TypeChecker &TC,
958958
bool hasKeywordArguments) {
959959
// There are two cases to check.
@@ -967,8 +967,8 @@ bool swift::isValidDynamicCallableMethod(FuncDecl *funcDecl, DeclContext *DC,
967967
// `ExpressibleByStringLiteral`.
968968
// `D.Value` and the return type can be arbitrary.
969969

970-
TC.validateDeclForNameLookup(funcDecl);
971-
auto paramList = funcDecl->getParameters();
970+
TC.validateDeclForNameLookup(decl);
971+
auto paramList = decl->getParameters();
972972
if (paramList->size() != 1 || paramList->get(0)->isVariadic()) return false;
973973
auto argType = paramList->get(0)->getType();
974974

@@ -998,8 +998,8 @@ bool swift::isValidDynamicCallableMethod(FuncDecl *funcDecl, DeclContext *DC,
998998
ConformanceCheckOptions()).hasValue();
999999
}
10001000

1001-
/// Returns true if a declaration has a valid implementation of a
1002-
/// @dynamicCallable attribute requirement.
1001+
/// Returns true if the given nominal type has a valid implementation of a
1002+
/// @dynamicCallable attribute requirement with the given argument name.
10031003
static bool hasValidDynamicCallableMethod(TypeChecker &TC,
10041004
NominalTypeDecl *decl,
10051005
Identifier argumentName,
@@ -1041,23 +1041,23 @@ visitDynamicCallableAttr(DynamicCallableAttr *attr) {
10411041
}
10421042
}
10431043

1044-
/// Given a subscript defined as "subscript(dynamicMember:)->T", return true if
1045-
/// it is an acceptable implementation of the @dynamicMemberLookup attribute's
1046-
/// requirement.
1047-
bool swift::isAcceptableDynamicMemberLookupSubscript(SubscriptDecl *decl,
1048-
DeclContext *DC,
1049-
TypeChecker &TC) {
1050-
// The only thing that we care about is that the index list has exactly one
1051-
// non-variadic entry. The type must conform to ExpressibleByStringLiteral.
1044+
/// Returns true if the given subscript method is an valid implementation of
1045+
/// the `subscript(dynamicMember:)` requirement for @dynamicMemberLookup.
1046+
/// The method is given to be defined as `subscript(dynamicMember:)`.
1047+
bool swift::isValidDynamicMemberLookupSubscript(SubscriptDecl *decl,
1048+
DeclContext *DC,
1049+
TypeChecker &TC) {
1050+
// There are two requirements:
1051+
// - The subscript method has exactly one, non-variadic parameter.
1052+
// - The parameter type conforms to `ExpressibleByStringLiteral`.
10521053
auto indices = decl->getIndices();
1053-
1054-
auto EBSL =
1054+
1055+
auto stringLitProto =
10551056
TC.Context.getProtocol(KnownProtocolKind::ExpressibleByStringLiteral);
10561057

1057-
return indices->size() == 1 &&
1058-
!indices->get(0)->isVariadic() &&
1058+
return indices->size() == 1 && !indices->get(0)->isVariadic() &&
10591059
TC.conformsToProtocol(indices->get(0)->getType(),
1060-
EBSL, DC, ConformanceCheckOptions());
1060+
stringLitProto, DC, ConformanceCheckOptions());
10611061
}
10621062

10631063
/// The @dynamicMemberLookup attribute is only allowed on types that have at
@@ -1066,9 +1066,8 @@ bool swift::isAcceptableDynamicMemberLookupSubscript(SubscriptDecl *decl,
10661066
/// subscript<KeywordType: ExpressibleByStringLiteral, LookupValue>
10671067
/// (dynamicMember name: KeywordType) -> LookupValue { get }
10681068
///
1069-
/// ... but doesn't care about the mutating'ness of the getter/setter. We just
1070-
/// manually check the requirements here.
1071-
///
1069+
/// ... but doesn't care about the mutating'ness of the getter/setter.
1070+
/// We just manually check the requirements here.
10721071
void AttributeChecker::
10731072
visitDynamicMemberLookupAttr(DynamicMemberLookupAttr *attr) {
10741073
// This attribute is only allowed on nominal types.
@@ -1086,25 +1085,25 @@ visitDynamicMemberLookupAttr(DynamicMemberLookupAttr *attr) {
10861085
// Lookup the implementations of our subscript.
10871086
auto candidates = TC.lookupMember(decl, type, subscriptName, lookupOptions);
10881087

1089-
// If we have none, then there is no attribute.
1088+
// If we have none, then the attribute is invalid.
10901089
if (candidates.empty()) {
1091-
TC.diagnose(attr->getLocation(), diag::type_invalid_dml, type);
1090+
TC.diagnose(attr->getLocation(), diag::invalid_dynamic_member_lookup_type,
1091+
type);
10921092
attr->setInvalid();
10931093
return;
10941094
}
10951095

1096-
10971096
// If none of the ones we find are acceptable, then reject one.
10981097
auto oneCandidate = candidates.front();
10991098
candidates.filter([&](LookupResultEntry entry, bool isOuter) -> bool {
11001099
auto cand = cast<SubscriptDecl>(entry.getValueDecl());
11011100
TC.validateDeclForNameLookup(cand);
1102-
return isAcceptableDynamicMemberLookupSubscript(cand, decl, TC);
1101+
return isValidDynamicMemberLookupSubscript(cand, decl, TC);
11031102
});
11041103

11051104
if (candidates.empty()) {
11061105
TC.diagnose(oneCandidate.getValueDecl()->getLoc(),
1107-
diag::type_invalid_dml, type);
1106+
diag::invalid_dynamic_member_lookup_type, type);
11081107
attr->setInvalid();
11091108
}
11101109
}

‎lib/Sema/TypeChecker.h

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2167,19 +2167,18 @@ class EncodedDiagnosticMessage {
21672167
const StringRef Message;
21682168
};
21692169

2170-
/// Returns true if a method is an valid implementation of a @dynamicCallable
2171-
/// attribute requirement. The method is given to be defined as one of the
2172-
/// following: `dynamicallyCall(withArguments:)` or
2170+
/// Returns true if the given method is an valid implementation of a
2171+
/// @dynamicCallable attribute requirement. The method is given to be defined
2172+
/// as one of the following: `dynamicallyCall(withArguments:)` or
21732173
/// `dynamicallyCall(withKeywordArguments:)`.
2174-
bool isValidDynamicCallableMethod(FuncDecl *funcDecl, DeclContext *DC,
2174+
bool isValidDynamicCallableMethod(FuncDecl *decl, DeclContext *DC,
21752175
TypeChecker &TC, bool hasKeywordArguments);
21762176

2177-
/// Given a subscript defined as "subscript(dynamicMember:)->T", return true if
2178-
/// it is an acceptable implementation of the @dynamicMemberLookup attribute's
2179-
/// requirement.
2180-
bool isAcceptableDynamicMemberLookupSubscript(SubscriptDecl *decl,
2181-
DeclContext *DC,
2182-
TypeChecker &TC);
2177+
/// Returns true if the given subscript method is an valid implementation of
2178+
/// the `subscript(dynamicMember:)` requirement for @dynamicMemberLookup.
2179+
/// The method is given to be defined as `subscript(dynamicMember:)`.
2180+
bool isValidDynamicMemberLookupSubscript(SubscriptDecl *decl, DeclContext *DC,
2181+
TypeChecker &TC);
21832182

21842183
/// Whether an overriding declaration requires the 'override' keyword.
21852184
enum class OverrideRequiresKeyword {

‎test/NameBinding/dynamic-member-lookup.swift renamed to ‎test/attr/attr_dynamic_member_lookup.swift

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -167,7 +167,7 @@ func test_iuo_result(x : IUOResultTest) {
167167
// Subscript index must be ExpressibleByStringLiteral.
168168
@dynamicMemberLookup
169169
struct Invalid1 {
170-
// expected-error @+1 {{@dynamicMemberLookup attribute requires 'Invalid1' to have a 'subscript(dynamicMember:)' member with a string index}}
170+
// expected-error @+1 {{@dynamicMemberLookup attribute requires 'Invalid1' to have a 'subscript(dynamicMember:)' method with an 'ExpressibleByStringLiteral' parameter}}
171171
subscript(dynamicMember member: Int) -> Int {
172172
return 42
173173
}
@@ -176,7 +176,7 @@ struct Invalid1 {
176176
// Subscript may not be variadic.
177177
@dynamicMemberLookup
178178
struct Invalid2 {
179-
// expected-error @+1 {{@dynamicMemberLookup attribute requires 'Invalid2' to have a 'subscript(dynamicMember:)' member with a string index}}
179+
// expected-error @+1 {{@dynamicMemberLookup attribute requires 'Invalid2' to have a 'subscript(dynamicMember:)' method with an 'ExpressibleByStringLiteral' parameter}}
180180
subscript(dynamicMember member: String...) -> Int {
181181
return 42
182182
}
@@ -217,7 +217,7 @@ func NotAllowedOnFunc() {
217217
// @dynamicMemberLookup cannot be declared on a base class and fulfilled with a
218218
// derived class.
219219

220-
// expected-error @+1 {{@dynamicMemberLookup attribute requires 'InvalidBase' to have a 'subscript(dynamicMember:)' member with a string index}}
220+
// expected-error @+1 {{@dynamicMemberLookup attribute requires 'InvalidBase' to have a 'subscript(dynamicMember:)' method with an 'ExpressibleByStringLiteral' parameter}}
221221
@dynamicMemberLookup
222222
class InvalidBase {}
223223

@@ -391,8 +391,3 @@ class C {
391391
}
392392
_ = \C.[dynamicMember: "hi"]
393393
_ = \C.testLookup
394-
395-
396-
397-
398-

0 commit comments

Comments
 (0)