Skip to content

Gardening for @dynamicMemberLookup. #20498

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Nov 12, 2018
Merged

Gardening for @dynamicMemberLookup. #20498

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6583acd
[NFC] Gardening for `@dynamicMemberLookup`.
dan-zheng Nov 9, 2018
9f46a53
Rename "argument" to "parameter".
dan-zheng Nov 11, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions include/swift/AST/DiagnosticsSema.def
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1054,9 +1054,10 @@ ERROR(missing_dynamic_callable_kwargs_method,none,
"@dynamicCallable type %0 cannot be applied with keyword arguments; "
"missing 'dynamicCall(withKeywordArguments:)' method", (Type))

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

ERROR(string_index_not_integer,none,
"String must not be indexed with %0, it has variable size elements",
Expand Down
35 changes: 18 additions & 17 deletions lib/Sema/CSApply.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -393,20 +393,20 @@ diagnoseInvalidDynamicConstructorReferences(ConstraintSystem &cs,
}

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

// Build and type check the string literal index value to the specific
// string type expected by the subscript.
Expr *nameExpr = new (ctx)
StringLiteralExpr(name, loc, /*implicit*/true);


// Build a tuple so that argument has a label.
Expr *tuple = TupleExpr::create(ctx, loc, nameExpr, ctx.Id_dynamicMember, loc,
loc, /*hasTrailingClosure*/false,
Expr *nameExpr = new (ctx) StringLiteralExpr(name, loc, /*implicit*/true);

// Build a tuple so that the argument has a label.
Expr *tuple = TupleExpr::create(ctx, loc, nameExpr, ctx.Id_dynamicMember,
loc, loc, /*hasTrailingClosure*/false,
/*implicit*/true);
(void)cs.TC.typeCheckExpression(tuple, dc, TypeLoc::withoutLoc(ty),
CTP_CallArgument);
Expand Down Expand Up @@ -1524,8 +1524,7 @@ namespace {
// Check whether the base is 'super'.
bool isSuper = base->isSuperExpr();

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

// Build and return a subscript that uses this string as the index.
return buildSubscript(base, index, ctx.Id_dynamicMember,
Expand Down Expand Up @@ -4393,16 +4393,17 @@ namespace {
// through the subscript(dynamicMember:) member, restore the
// openedType and origComponent to its full reference as if the user
// wrote out the subscript manually.
if (foundDecl->choice.getKind()
== OverloadChoiceKind::DynamicMemberLookup) {
if (foundDecl->choice.getKind() ==
OverloadChoiceKind::DynamicMemberLookup) {
foundDecl->openedType = foundDecl->openedFullType
->castTo<AnyFunctionType>()->getResult();
->castTo<AnyFunctionType>()->getResult();

auto &ctx = cs.TC.Context;
auto loc = origComponent.getLoc();
auto fieldName =
foundDecl->choice.getName().getBaseIdentifier().str();
auto index = getDMLIndexExpr(fieldName, indexType, loc, dc, cs);
auto index = buildDynamicMemberLookupIndexExpr(fieldName, indexType,
loc, dc, cs);

origComponent = KeyPathExpr::Component::
forUnresolvedSubscript(ctx, loc, index, {}, loc, loc,
Expand Down
3 changes: 2 additions & 1 deletion lib/Sema/CSDiagnostics.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -664,7 +664,8 @@ bool RValueTreatedAsLValueFailure::diagnoseAsError() {
}

if (auto resolvedOverload = getResolvedOverload(getLocator()))
if (resolvedOverload->Choice.getKind() == OverloadChoiceKind::DynamicMemberLookup)
if (resolvedOverload->Choice.getKind() ==
OverloadChoiceKind::DynamicMemberLookup)
subElementDiagID = diag::assignment_dynamic_property_has_immutable_base;
} else if (auto sub = dyn_cast<SubscriptExpr>(diagExpr)) {
subElementDiagID = diag::assignment_subscript_has_immutable_base;
Expand Down
33 changes: 16 additions & 17 deletions lib/Sema/CSSimplify.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3053,17 +3053,17 @@ getArgumentLabels(ConstraintSystem &cs, ConstraintLocatorBuilder locator) {
/// conformances, but this is fine because it doesn't get invoked in the normal
/// name lookup path (only when lookup is about to fail).
static bool hasDynamicMemberLookupAttribute(CanType ty,
Copy link
Contributor Author

@dan-zheng dan-zheng Nov 11, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Deduping this function with getDynamicCallableMethods (which also defines the same "type traversal" logic) would be nice. However, the two functions use different cache types (llvm::DenseMap<CanType, bool> vs llvm::DenseMap<CanType, DynamicCallableMethods>), which makes generalization difficult. 🙁

Any generalization ideas appreciated.

llvm::DenseMap<CanType, bool> &IsDynamicMemberLookupCache) {
auto it = IsDynamicMemberLookupCache.find(ty);
if (it != IsDynamicMemberLookupCache.end()) return it->second;
llvm::DenseMap<CanType, bool> &DynamicMemberLookupCache) {
auto it = DynamicMemberLookupCache.find(ty);
if (it != DynamicMemberLookupCache.end()) return it->second;

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

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

// Cache this if we can.
if (!ty->hasTypeVariable())
IsDynamicMemberLookupCache[ty] = result;
DynamicMemberLookupCache[ty] = result;

return result;
}
Expand Down Expand Up @@ -3515,32 +3514,32 @@ performMemberLookup(ConstraintKind constraintKind, DeclName memberName,
}

// If we're about to fail lookup, but we are looking for members in a type
// that has the @dynamicMemberLookup attribute, then we resolve the reference
// to the subscript(dynamicMember:) member, and pass the member name as a
// string.
// with the @dynamicMemberLookup attribute, then we resolve a reference
// to a `subscript(dynamicMember:)` method and pass the member name as a
// string parameter.
if (result.ViableCandidates.empty() &&
constraintKind == ConstraintKind::ValueMember &&
memberName.isSimpleName() && !memberName.isSpecial()) {
auto name = memberName.getBaseIdentifier();
if (hasDynamicMemberLookupAttribute(instanceTy->getCanonicalType(),
IsDynamicMemberLookupCache)) {
DynamicMemberLookupCache)) {
auto &ctx = getASTContext();
// Recursively look up the subscript(dynamicMember:)'s in this type.

// Recursively look up `subscript(dynamicMember:)` methods in this type.
auto subscriptName =
DeclName(ctx, DeclBaseName::createSubscript(), ctx.Id_dynamicMember);

auto subscripts = performMemberLookup(constraintKind,
subscriptName,
baseTy, functionRefKind,
memberLocator,
includeInaccessibleMembers);

// Reflect the candidates found as DynamicMemberLookup results.
// Reflect the candidates found as `DynamicMemberLookup` results.
for (auto candidate : subscripts.ViableCandidates) {
auto decl = cast<SubscriptDecl>(candidate.getDecl());
if (isAcceptableDynamicMemberLookupSubscript(decl, DC, TC))
result.addViable(OverloadChoice::getDynamicMemberLookup(baseTy,
decl, name));
if (isValidDynamicMemberLookupSubscript(decl, DC, TC))
result.addViable(
OverloadChoice::getDynamicMemberLookup(baseTy, decl, name));
}
for (auto candidate : subscripts.UnviableCandidates) {
auto decl = candidate.first.getDecl();
Expand Down
7 changes: 3 additions & 4 deletions lib/Sema/ConstraintSystem.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1085,10 +1085,9 @@ class ConstraintSystem {
/// types.
llvm::DenseMap<CanType, DynamicCallableMethods> DynamicCallableCache;

/// This is a cache that keeps track of whether a given type is known (or not)
/// to be a @dynamicMemberLookup type.
///
llvm::DenseMap<CanType, bool> IsDynamicMemberLookupCache;
/// A cache that stores whether types are valid @dynamicMemberLookup types.
llvm::DenseMap<CanType, bool> DynamicMemberLookupCache;

private:
/// \brief Describe the candidate expression for partial solving.
/// This class used by shrink & solve methods which apply
Expand Down
3 changes: 2 additions & 1 deletion lib/Sema/OverloadChoice.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,8 @@ class OverloadChoice {
}

/// Retrieve an overload choice for a declaration that was found via
/// dynamic lookup. The ValueDecl is the subscript(dynamicMember:)
/// dynamic member lookup. The `ValueDecl` is a `subscript(dynamicMember:)`
/// method.
static OverloadChoice getDynamicMemberLookup(Type base, ValueDecl *value,
Identifier name) {
OverloadChoice result;
Expand Down
57 changes: 28 additions & 29 deletions lib/Sema/TypeCheckAttr.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -949,11 +949,11 @@ static bool isRelaxedIBAction(TypeChecker &TC) {
return isiOS(TC) || iswatchOS(TC);
}

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

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

Expand Down Expand Up @@ -998,8 +998,8 @@ bool swift::isValidDynamicCallableMethod(FuncDecl *funcDecl, DeclContext *DC,
ConformanceCheckOptions()).hasValue();
}

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

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

auto stringLitProto =
TC.Context.getProtocol(KnownProtocolKind::ExpressibleByStringLiteral);

return indices->size() == 1 &&
!indices->get(0)->isVariadic() &&
return indices->size() == 1 && !indices->get(0)->isVariadic() &&
TC.conformsToProtocol(indices->get(0)->getType(),
EBSL, DC, ConformanceCheckOptions());
stringLitProto, DC, ConformanceCheckOptions());
}

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

// If we have none, then there is no attribute.
// If we have none, then the attribute is invalid.
if (candidates.empty()) {
TC.diagnose(attr->getLocation(), diag::type_invalid_dml, type);
TC.diagnose(attr->getLocation(), diag::invalid_dynamic_member_lookup_type,
type);
attr->setInvalid();
return;
}


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

if (candidates.empty()) {
TC.diagnose(oneCandidate.getValueDecl()->getLoc(),
diag::type_invalid_dml, type);
diag::invalid_dynamic_member_lookup_type, type);
attr->setInvalid();
}
}
Expand Down
19 changes: 9 additions & 10 deletions lib/Sema/TypeChecker.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2158,19 +2158,18 @@ class EncodedDiagnosticMessage {
const StringRef Message;
};

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

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

/// Whether an overriding declaration requires the 'override' keyword.
enum class OverrideRequiresKeyword {
Expand Down
11 changes: 3 additions & 8 deletions test/NameBinding/dynamic-member-lookup.swift → test/attr/attr_dynamic_member_lookup.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ func test_iuo_result(x : IUOResultTest) {
// Subscript index must be ExpressibleByStringLiteral.
@dynamicMemberLookup
struct Invalid1 {
// expected-error @+1 {{@dynamicMemberLookup attribute requires 'Invalid1' to have a 'subscript(dynamicMember:)' member with a string index}}
// expected-error @+1 {{@dynamicMemberLookup attribute requires 'Invalid1' to have a 'subscript(dynamicMember:)' method with an 'ExpressibleByStringLiteral' parameter}}
subscript(dynamicMember member: Int) -> Int {
return 42
}
Expand All @@ -176,7 +176,7 @@ struct Invalid1 {
// Subscript may not be variadic.
@dynamicMemberLookup
struct Invalid2 {
// expected-error @+1 {{@dynamicMemberLookup attribute requires 'Invalid2' to have a 'subscript(dynamicMember:)' member with a string index}}
// expected-error @+1 {{@dynamicMemberLookup attribute requires 'Invalid2' to have a 'subscript(dynamicMember:)' method with an 'ExpressibleByStringLiteral' parameter}}
subscript(dynamicMember member: String...) -> Int {
return 42
}
Expand Down Expand Up @@ -217,7 +217,7 @@ func NotAllowedOnFunc() {
// @dynamicMemberLookup cannot be declared on a base class and fulfilled with a
// derived class.

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

Expand Down Expand Up @@ -391,8 +391,3 @@ class C {
}
_ = \C.[dynamicMember: "hi"]
_ = \C.testLookup