Skip to content

Fix @dynamicMemberLookup for protocol/archetype types. #20516

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.

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
Show file tree
Hide file tree
Changes from all commits
Commits
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
71 changes: 46 additions & 25 deletions lib/Sema/CSSimplify.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3083,26 +3083,45 @@ getArgumentLabels(ConstraintSystem &cs, ConstraintLocatorBuilder locator) {
/// particularly fast in the face of deep class hierarchies or lots of protocol
/// 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,
static bool hasDynamicMemberLookupAttribute(Type type,
llvm::DenseMap<CanType, bool> &DynamicMemberLookupCache) {
auto it = DynamicMemberLookupCache.find(ty);
auto canType = type->getCanonicalType();
auto it = DynamicMemberLookupCache.find(canType);
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(),
DynamicMemberLookupCache))
return true;
return false;

// Calculate @dynamicMemberLookup attribute for composite types with multiple
// components (protocol composition types and archetypes).
auto calculateForComponentTypes =
[&](ArrayRef<Type> componentTypes) -> bool {
for (auto componentType : componentTypes)
if (hasDynamicMemberLookupAttribute(componentType,
DynamicMemberLookupCache))
return true;
return false;
};

auto calculate = [&]() -> bool {
// If this is an archetype type, check if any types it conforms to
// (superclass or protocols) have the attribute.
if (auto archetype = dyn_cast<ArchetypeType>(canType)) {
SmallVector<Type, 2> componentTypes;
for (auto protocolDecl : archetype->getConformsTo())
componentTypes.push_back(protocolDecl->getDeclaredType());
if (auto superclass = archetype->getSuperclass())
componentTypes.push_back(superclass);
return calculateForComponentTypes(componentTypes);
}

// Otherwise this has to be a nominal type.
auto nominal = ty->getAnyNominal();
if (!nominal) return false; // Dynamic lookups don't exist on tuples, etc.


// If this is a protocol composition, check if any of its members have the
// attribute.
if (auto protocolComp = dyn_cast<ProtocolCompositionType>(canType))
return calculateForComponentTypes(protocolComp->getMembers());

// Otherwise, this must be a nominal type.
// Dynamic member lookup doesn't work for tuples, etc.
auto nominal = canType->getAnyNominal();
if (!nominal) return false;

// If this type conforms to a protocol with the attribute, then return true.
for (auto p : nominal->getAllProtocols())
if (p->getAttrs().hasAttribute<DynamicMemberLookupAttr>())
Expand Down Expand Up @@ -3132,11 +3151,9 @@ static bool hasDynamicMemberLookupAttribute(CanType ty,
};

auto result = calculate();

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

// Cache the result if the type does not contain type variables.
if (!type->hasTypeVariable())
DynamicMemberLookupCache[canType] = result;
return result;
}

Expand Down Expand Up @@ -3552,8 +3569,8 @@ performMemberLookup(ConstraintKind constraintKind, DeclName memberName,
constraintKind == ConstraintKind::ValueMember &&
memberName.isSimpleName() && !memberName.isSpecial()) {
auto name = memberName.getBaseIdentifier();
if (hasDynamicMemberLookupAttribute(instanceTy->getCanonicalType(),
DynamicMemberLookupCache)) {
// TC.extract
Copy link
Contributor Author

@dan-zheng dan-zheng Nov 14, 2018

Choose a reason for hiding this comment

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

Oops, accidentally committed this rogue comment.
I'll remove in my next PR, if it's not gone by then.
Removed in #20567.

if (hasDynamicMemberLookupAttribute(instanceTy, DynamicMemberLookupCache)) {
auto &ctx = getASTContext();

// Recursively look up `subscript(dynamicMember:)` methods in this type.
Expand Down Expand Up @@ -4680,7 +4697,11 @@ getDynamicCallableMethods(Type type, ConstraintSystem &CS,
}
};

return CS.DynamicCallableCache[canType] = calculate();
auto result = calculate();
// Cache the result if the type does not contain type variables.
if (!type->hasTypeVariable())
CS.DynamicCallableCache[canType] = result;
return result;
}

ConstraintSystem::SolutionKind
Expand Down
18 changes: 6 additions & 12 deletions lib/Sema/TypeCheckAttr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1074,26 +1074,20 @@ visitDynamicMemberLookupAttr(DynamicMemberLookupAttr *attr) {
auto decl = cast<NominalTypeDecl>(D);
auto type = decl->getDeclaredType();

// Lookup our subscript.
auto subscriptName =
DeclName(TC.Context, DeclBaseName::createSubscript(),
TC.Context.Id_dynamicMember);
// Look up `subscript(dynamicMember:)` candidates.
auto subscriptName = DeclName(TC.Context, DeclBaseName::createSubscript(),
TC.Context.Id_dynamicMember);
auto candidates = TC.lookupMember(decl, type, subscriptName);

auto lookupOptions = defaultMemberTypeLookupOptions;
lookupOptions -= NameLookupFlags::PerformConformanceCheck;

// Lookup the implementations of our subscript.
auto candidates = TC.lookupMember(decl, type, subscriptName, lookupOptions);

// If we have none, then the attribute is invalid.
// If there are no candidates, then the attribute is invalid.
if (candidates.empty()) {
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.
// If no candidates are valid, then reject one.
auto oneCandidate = candidates.front();
candidates.filter([&](LookupResultEntry entry, bool isOuter) -> bool {
auto cand = cast<SubscriptDecl>(entry.getValueDecl());
Expand Down
12 changes: 6 additions & 6 deletions test/attr/attr_dynamic_callable.swift
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// RUN: %target-swift-frontend -typecheck -verify %s
// RUN: %target-typecheck-verify-swift

@dynamicCallable
struct Callable {
Expand Down Expand Up @@ -64,8 +64,8 @@ func testIUO(

@dynamicCallable
struct CallableReturningFunction {
func dynamicallyCall(withArguments arguments: [Int]) -> (_ a: Int) -> Void {
return { a in () }
func dynamicallyCall(withArguments arguments: [Int]) -> (Int) -> Void {
return { x in () }
}
}

Expand Down Expand Up @@ -138,7 +138,7 @@ class InvalidDerived : InvalidBase {
}

//===----------------------------------------------------------------------===//
// Multiple `dynamicallyCall` method tests
// Multiple `dynamicallyCall` methods
//===----------------------------------------------------------------------===//

@dynamicCallable
Expand All @@ -160,7 +160,7 @@ func testOverloaded(x: OverloadedCallable) {
}

//===----------------------------------------------------------------------===//
// Existential tests
// Existentials
//===----------------------------------------------------------------------===//

@dynamicCallable
Expand Down Expand Up @@ -318,7 +318,7 @@ func testEnum() {
}

//===----------------------------------------------------------------------===//
// Generics tests
// Generics
//===----------------------------------------------------------------------===//

@dynamicCallable
Expand Down
Loading