Skip to content

Commit 30e14ee

Browse files
authored
Merge pull request #31937 from hamishknight/an-introspective-lookup
2 parents 73e5fc3 + bc618a9 commit 30e14ee

File tree

5 files changed

+81
-64
lines changed

5 files changed

+81
-64
lines changed

include/swift/AST/ASTContext.h

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -748,11 +748,13 @@ class ASTContext final {
748748
/// \param methods The list of @objc methods in this class that have this
749749
/// selector and are instance/class methods as requested. This list will be
750750
/// extended with any methods found in subsequent generations.
751-
void loadObjCMethods(ClassDecl *classDecl,
752-
ObjCSelector selector,
753-
bool isInstanceMethod,
754-
unsigned previousGeneration,
755-
llvm::TinyPtrVector<AbstractFunctionDecl *> &methods);
751+
///
752+
/// \param swiftOnly If true, only loads methods from imported Swift modules,
753+
/// skipping the Clang importer.
754+
void loadObjCMethods(ClassDecl *classDecl, ObjCSelector selector,
755+
bool isInstanceMethod, unsigned previousGeneration,
756+
llvm::TinyPtrVector<AbstractFunctionDecl *> &methods,
757+
bool swiftOnly = false);
756758

757759
/// Load derivative function configurations for the given
758760
/// AbstractFunctionDecl.

lib/AST/ASTContext.cpp

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1484,14 +1484,16 @@ void ASTContext::loadExtensions(NominalTypeDecl *nominal,
14841484
}
14851485

14861486
void ASTContext::loadObjCMethods(
1487-
ClassDecl *classDecl,
1488-
ObjCSelector selector,
1489-
bool isInstanceMethod,
1490-
unsigned previousGeneration,
1491-
llvm::TinyPtrVector<AbstractFunctionDecl *> &methods) {
1487+
ClassDecl *classDecl, ObjCSelector selector, bool isInstanceMethod,
1488+
unsigned previousGeneration,
1489+
llvm::TinyPtrVector<AbstractFunctionDecl *> &methods, bool swiftOnly) {
14921490
PrettyStackTraceSelector stackTraceSelector("looking for", selector);
14931491
PrettyStackTraceDecl stackTraceDecl("...in", classDecl);
14941492
for (auto &loader : getImpl().ModuleLoaders) {
1493+
// Ignore the Clang importer if we've been asked for Swift-only results.
1494+
if (swiftOnly && loader.get() == getClangModuleLoader())
1495+
continue;
1496+
14951497
loader->loadObjCMethods(classDecl, selector, isInstanceMethod,
14961498
previousGeneration, methods);
14971499
}

lib/ClangImporter/ClangImporter.cpp

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -3045,11 +3045,6 @@ void ClangImporter::loadObjCMethods(
30453045
bool isInstanceMethod,
30463046
unsigned previousGeneration,
30473047
llvm::TinyPtrVector<AbstractFunctionDecl *> &methods) {
3048-
// If we're currently looking for this selector, don't load any Objective-C
3049-
// methods.
3050-
if (Impl.ActiveSelectors.count({selector, isInstanceMethod}))
3051-
return;
3052-
30533048
const auto *objcClass =
30543049
dyn_cast_or_null<clang::ObjCInterfaceDecl>(classDecl->getClangDecl());
30553050
if (!objcClass)
@@ -3082,10 +3077,7 @@ void ClangImporter::loadObjCMethods(
30823077
// earlier, because we aren't tracking generation counts for Clang modules.
30833078
// Filter out the duplicates.
30843079
// FIXME: We shouldn't need to do this.
3085-
llvm::SmallPtrSet<AbstractFunctionDecl *, 4> known;
3086-
known.insert(methods.begin(), methods.end());
3087-
3088-
if (known.insert(method).second)
3080+
if (!llvm::is_contained(methods, method))
30893081
methods.push_back(method);
30903082
}
30913083

lib/ClangImporter/ImportDecl.cpp

Lines changed: 66 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -2191,10 +2191,13 @@ namespace {
21912191
if (fd->isInstanceMember() != decl->isInstanceProperty())
21922192
continue;
21932193

2194-
assert(fd->getName().getArgumentNames().empty());
2194+
// We only care about methods with no arguments, because they can
2195+
// shadow imported properties.
2196+
if (!fd->getName().getArgumentNames().empty())
2197+
continue;
2198+
21952199
foundMethod = true;
2196-
} else {
2197-
auto *var = cast<VarDecl>(result);
2200+
} else if (auto *var = dyn_cast<VarDecl>(result)) {
21982201
if (var->isInstanceMember() != decl->isInstanceProperty())
21992202
continue;
22002203

@@ -2280,7 +2283,7 @@ namespace {
22802283
return getVersion() == getActiveSwiftVersion();
22812284
}
22822285

2283-
void recordMemberInContext(DeclContext *dc, ValueDecl *member) {
2286+
void recordMemberInContext(const DeclContext *dc, ValueDecl *member) {
22842287
assert(member && "Attempted to record null member!");
22852288
auto *nominal = dc->getSelfNominalTypeDecl();
22862289
auto name = member->getBaseName();
@@ -4170,33 +4173,51 @@ namespace {
41704173
bool isInstance, const DeclContext *dc,
41714174
llvm::function_ref<bool(AbstractFunctionDecl *fn)> filter) {
41724175
// We only need to perform this check for classes.
4173-
auto classDecl
4174-
= dc->getDeclaredInterfaceType()->getClassOrBoundGenericClass();
4176+
auto *classDecl = dc->getSelfClassDecl();
41754177
if (!classDecl)
41764178
return false;
41774179

4178-
// Make sure we don't search in Clang modules for this method.
4179-
++Impl.ActiveSelectors[{selector, isInstance}];
4180-
4181-
// Look for a matching imported or deserialized member.
4182-
bool result = false;
4183-
for (auto decl : classDecl->lookupDirect(selector, isInstance)) {
4184-
if ((decl->getClangDecl()
4185-
|| !decl->getDeclContext()->getParentSourceFile())
4186-
&& importedName.getDeclName() == decl->getName()
4187-
&& filter(decl)) {
4188-
result = true;
4189-
break;
4180+
auto matchesImportedDecl = [&](Decl *member) -> bool {
4181+
auto *afd = dyn_cast<AbstractFunctionDecl>(member);
4182+
if (!afd)
4183+
return false;
4184+
4185+
// Instance-ness must match.
4186+
if (afd->isObjCInstanceMethod() != isInstance)
4187+
return false;
4188+
4189+
// Both the selector and imported name must match.
4190+
if (afd->getObjCSelector() != selector ||
4191+
importedName.getDeclName() != afd->getName()) {
4192+
return false;
41904193
}
4191-
}
41924194

4193-
// Restore the previous active count in the active-selector mapping.
4194-
auto activeCount = Impl.ActiveSelectors.find({selector, isInstance});
4195-
--activeCount->second;
4196-
if (activeCount->second == 0)
4197-
Impl.ActiveSelectors.erase(activeCount);
4195+
// Finally, the provided filter must match.
4196+
return filter(afd);
4197+
};
41984198

4199-
return result;
4199+
// First check to see if we've already imported a method with the same
4200+
// selector.
4201+
auto importedMembers = Impl.MembersForNominal.find(classDecl);
4202+
if (importedMembers != Impl.MembersForNominal.end()) {
4203+
auto baseName = importedName.getDeclName().getBaseName();
4204+
auto membersForName = importedMembers->second.find(baseName);
4205+
if (membersForName != importedMembers->second.end()) {
4206+
return llvm::any_of(membersForName->second, matchesImportedDecl);
4207+
}
4208+
}
4209+
4210+
// Then, for a deserialized Swift class, check to see if it has brought in
4211+
// any matching @objc methods.
4212+
if (classDecl->wasDeserialized()) {
4213+
auto &ctx = Impl.SwiftContext;
4214+
TinyPtrVector<AbstractFunctionDecl *> deserializedMethods;
4215+
ctx.loadObjCMethods(classDecl, selector, isInstance,
4216+
/*prevGeneration*/ 0, deserializedMethods,
4217+
/*swiftOnly*/ true);
4218+
return llvm::any_of(deserializedMethods, matchesImportedDecl);
4219+
}
4220+
return false;
42004221
}
42014222

42024223
Decl *importObjCMethodDecl(const clang::ObjCMethodDecl *decl,
@@ -4448,6 +4469,9 @@ namespace {
44484469
// If this method overrides another method, mark it as such.
44494470
recordObjCOverride(result);
44504471

4472+
// Make a note that we've imported this method into this context.
4473+
recordMemberInContext(dc, result);
4474+
44514475
// Record the error convention.
44524476
if (errorConvention) {
44534477
result->setForeignErrorConvention(*errorConvention);
@@ -4484,14 +4508,6 @@ namespace {
44844508
Impl.addAlternateDecl(result, cast<ValueDecl>(imported));
44854509
}
44864510
}
4487-
4488-
// We only care about recording methods with no arguments here, because
4489-
// they can shadow imported properties.
4490-
if (!isa<AccessorDecl>(result) &&
4491-
result->getName().getArgumentNames().empty()) {
4492-
recordMemberInContext(dc, result);
4493-
}
4494-
44954511
return result;
44964512
}
44974513

@@ -6461,6 +6477,7 @@ ConstructorDecl *SwiftDeclConverter::importConstructor(
64616477
/*GenericParams=*/nullptr, const_cast<DeclContext *>(dc));
64626478

64636479
addObjCAttribute(result, selector);
6480+
recordMemberInContext(dc, result);
64646481

64656482
Impl.recordImplicitUnwrapForDecl(result,
64666483
importedType.isImplicitlyUnwrapped());
@@ -6668,14 +6685,24 @@ SwiftDeclConverter::importSubscript(Decl *decl,
66686685
};
66696686

66706687
auto findCounterpart = [&](clang::Selector sel) -> FuncDecl * {
6671-
// If the declaration we're starting from is in a class, first
6672-
// look for a class member with the appropriate selector.
6688+
// If the declaration we're starting from is in a class, first check to see
6689+
// if we've already imported an instance method with a matching selector.
66736690
if (auto classDecl = decl->getDeclContext()->getSelfClassDecl()) {
66746691
auto swiftSel = Impl.importSelector(sel);
6675-
for (auto found : classDecl->lookupDirect(swiftSel, true)) {
6676-
if (auto foundFunc = dyn_cast<FuncDecl>(found))
6677-
if (foundFunc->hasClangNode())
6678-
return foundFunc;
6692+
auto importedMembers = Impl.MembersForNominal.find(classDecl);
6693+
if (importedMembers != Impl.MembersForNominal.end()) {
6694+
for (auto membersForName : importedMembers->second) {
6695+
for (auto *member : membersForName.second) {
6696+
// Must be an instance method.
6697+
auto *afd = dyn_cast<FuncDecl>(member);
6698+
if (!afd || !afd->isInstanceMember())
6699+
continue;
6700+
6701+
// Selector must match.
6702+
if (afd->getObjCSelector() == swiftSel)
6703+
return afd;
6704+
}
6705+
}
66796706
}
66806707
}
66816708

lib/ClangImporter/ImporterImpl.h

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -439,12 +439,6 @@ class LLVM_LIBRARY_VISIBILITY ClangImporter::Implementation
439439
llvm::DenseMap<const clang::MacroInfo *, std::pair<clang::APValue, Type>>
440440
ImportedMacroConstants;
441441

442-
/// Keeps track of active selector-based lookups, so that we don't infinitely
443-
/// recurse when checking whether a method with a given selector has already
444-
/// been imported.
445-
llvm::DenseMap<std::pair<ObjCSelector, char>, unsigned>
446-
ActiveSelectors;
447-
448442
// Mapping from imported types to their raw value types.
449443
llvm::DenseMap<const NominalTypeDecl *, Type> RawTypes;
450444

0 commit comments

Comments
 (0)