Skip to content

Commit 6583acd

Browse files
dan-zhengdan-zheng
committed
[NFC] Gardening for @dynamicMemberLookup.
- Unify code style of `@dynamicMemberLookup` and `@dynamicCallable` implementations. - Use consistent variable names, doc comments, etc. - Move `@dynamicMemberLookup` test to `test/attr` directory.
1 parent a1ae75b commit 6583acd

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
@@ -1054,9 +1054,10 @@ ERROR(missing_dynamic_callable_kwargs_method,none,
10541054
"@dynamicCallable type %0 cannot be applied with keyword arguments; "
10551055
"missing 'dynamicCall(withKeywordArguments:)' method", (Type))
10561056

1057-
ERROR(type_invalid_dml,none,
1057+
ERROR(invalid_dynamic_member_lookup_type,none,
10581058
"@dynamicMemberLookup attribute requires %0 to have a "
1059-
"'subscript(dynamicMember:)' member with a string index", (Type))
1059+
"'subscript(dynamicMember:)' method with an "
1060+
"'ExpressibleByStringLiteral' argument", (Type))
10601061

10611062
ERROR(string_index_not_integer,none,
10621063
"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,
@@ -4393,16 +4393,17 @@ namespace {
43934393
// through the subscript(dynamicMember:) member, restore the
43944394
// openedType and origComponent to its full reference as if the user
43954395
// wrote out the subscript manually.
4396-
if (foundDecl->choice.getKind()
4397-
== OverloadChoiceKind::DynamicMemberLookup) {
4396+
if (foundDecl->choice.getKind() ==
4397+
OverloadChoiceKind::DynamicMemberLookup) {
43984398
foundDecl->openedType = foundDecl->openedFullType
4399-
->castTo<AnyFunctionType>()->getResult();
4399+
->castTo<AnyFunctionType>()->getResult();
44004400

44014401
auto &ctx = cs.TC.Context;
44024402
auto loc = origComponent.getLoc();
44034403
auto fieldName =
44044404
foundDecl->choice.getName().getBaseIdentifier().str();
4405-
auto index = getDMLIndexExpr(fieldName, indexType, loc, dc, cs);
4405+
auto index = buildDynamicMemberLookupIndexExpr(fieldName, indexType,
4406+
loc, dc, cs);
44064407

44074408
origComponent = KeyPathExpr::Component::
44084409
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
@@ -3053,17 +3053,17 @@ getArgumentLabels(ConstraintSystem &cs, ConstraintLocatorBuilder locator) {
30533053
/// conformances, but this is fine because it doesn't get invoked in the normal
30543054
/// name lookup path (only when lookup is about to fail).
30553055
static bool hasDynamicMemberLookupAttribute(CanType ty,
3056-
llvm::DenseMap<CanType, bool> &IsDynamicMemberLookupCache) {
3057-
auto it = IsDynamicMemberLookupCache.find(ty);
3058-
if (it != IsDynamicMemberLookupCache.end()) return it->second;
3056+
llvm::DenseMap<CanType, bool> &DynamicMemberLookupCache) {
3057+
auto it = DynamicMemberLookupCache.find(ty);
3058+
if (it != DynamicMemberLookupCache.end()) return it->second;
30593059

30603060
auto calculate = [&]()-> bool {
30613061
// If this is a protocol composition, check to see if any of the protocols
30623062
// have the attribute on them.
30633063
if (auto protocolComp = ty->getAs<ProtocolCompositionType>()) {
30643064
for (auto p : protocolComp->getMembers())
30653065
if (hasDynamicMemberLookupAttribute(p->getCanonicalType(),
3066-
IsDynamicMemberLookupCache))
3066+
DynamicMemberLookupCache))
30673067
return true;
30683068
return false;
30693069
}
@@ -3072,8 +3072,7 @@ static bool hasDynamicMemberLookupAttribute(CanType ty,
30723072
auto nominal = ty->getAnyNominal();
30733073
if (!nominal) return false; // Dynamic lookups don't exist on tuples, etc.
30743074

3075-
// If any of the protocols this type conforms to has the attribute, then
3076-
// yes.
3075+
// If this type conforms to a protocol with the attribute, then return true.
30773076
for (auto p : nominal->getAllProtocols())
30783077
if (p->getAttrs().hasAttribute<DynamicMemberLookupAttr>())
30793078
return true;
@@ -3105,7 +3104,7 @@ static bool hasDynamicMemberLookupAttribute(CanType ty,
31053104

31063105
// Cache this if we can.
31073106
if (!ty->hasTypeVariable())
3108-
IsDynamicMemberLookupCache[ty] = result;
3107+
DynamicMemberLookupCache[ty] = result;
31093108

31103109
return result;
31113110
}
@@ -3515,32 +3514,32 @@ performMemberLookup(ConstraintKind constraintKind, DeclName memberName,
35153514
}
35163515

35173516
// If we're about to fail lookup, but we are looking for members in a type
3518-
// that has the @dynamicMemberLookup attribute, then we resolve the reference
3519-
// to the subscript(dynamicMember:) member, and pass the member name as a
3520-
// string.
3517+
// with the @dynamicMemberLookup attribute, then we resolve a reference
3518+
// to a `subscript(dynamicMember:)` method and pass the member name as a
3519+
// string parameter.
35213520
if (result.ViableCandidates.empty() &&
35223521
constraintKind == ConstraintKind::ValueMember &&
35233522
memberName.isSimpleName() && !memberName.isSpecial()) {
35243523
auto name = memberName.getBaseIdentifier();
35253524
if (hasDynamicMemberLookupAttribute(instanceTy->getCanonicalType(),
3526-
IsDynamicMemberLookupCache)) {
3525+
DynamicMemberLookupCache)) {
35273526
auto &ctx = getASTContext();
3528-
// Recursively look up the subscript(dynamicMember:)'s in this type.
3527+
3528+
// Recursively look up `subscript(dynamicMember:)` methods in this type.
35293529
auto subscriptName =
35303530
DeclName(ctx, DeclBaseName::createSubscript(), ctx.Id_dynamicMember);
3531-
35323531
auto subscripts = performMemberLookup(constraintKind,
35333532
subscriptName,
35343533
baseTy, functionRefKind,
35353534
memberLocator,
35363535
includeInaccessibleMembers);
35373536

3538-
// Reflect the candidates found as DynamicMemberLookup results.
3537+
// Reflect the candidates found as `DynamicMemberLookup` results.
35393538
for (auto candidate : subscripts.ViableCandidates) {
35403539
auto decl = cast<SubscriptDecl>(candidate.getDecl());
3541-
if (isAcceptableDynamicMemberLookupSubscript(decl, DC, TC))
3542-
result.addViable(OverloadChoice::getDynamicMemberLookup(baseTy,
3543-
decl, name));
3540+
if (isValidDynamicMemberLookupSubscript(decl, DC, TC))
3541+
result.addViable(
3542+
OverloadChoice::getDynamicMemberLookup(baseTy, decl, name));
35443543
}
35453544
for (auto candidate : subscripts.UnviableCandidates) {
35463545
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
@@ -2158,19 +2158,18 @@ class EncodedDiagnosticMessage {
21582158
const StringRef Message;
21592159
};
21602160

2161-
/// Returns true if a method is an valid implementation of a @dynamicCallable
2162-
/// attribute requirement. The method is given to be defined as one of the
2163-
/// following: `dynamicallyCall(withArguments:)` or
2161+
/// Returns true if the given method is an valid implementation of a
2162+
/// @dynamicCallable attribute requirement. The method is given to be defined
2163+
/// as one of the following: `dynamicallyCall(withArguments:)` or
21642164
/// `dynamicallyCall(withKeywordArguments:)`.
2165-
bool isValidDynamicCallableMethod(FuncDecl *funcDecl, DeclContext *DC,
2165+
bool isValidDynamicCallableMethod(FuncDecl *decl, DeclContext *DC,
21662166
TypeChecker &TC, bool hasKeywordArguments);
21672167

2168-
/// Given a subscript defined as "subscript(dynamicMember:)->T", return true if
2169-
/// it is an acceptable implementation of the @dynamicMemberLookup attribute's
2170-
/// requirement.
2171-
bool isAcceptableDynamicMemberLookupSubscript(SubscriptDecl *decl,
2172-
DeclContext *DC,
2173-
TypeChecker &TC);
2168+
/// Returns true if the given subscript method is an valid implementation of
2169+
/// the `subscript(dynamicMember:)` requirement for @dynamicMemberLookup.
2170+
/// The method is given to be defined as `subscript(dynamicMember:)`.
2171+
bool isValidDynamicMemberLookupSubscript(SubscriptDecl *decl, DeclContext *DC,
2172+
TypeChecker &TC);
21742173

21752174
/// Whether an overriding declaration requires the 'override' keyword.
21762175
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' argument}}
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' argument}}
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' argument}}
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)