Skip to content

Commit b66e65c

Browse files
DougGregortkremenek
authored andcommitted
SE-0022: Deprecate string-literal-as-selector in favor of #selector.
Introduce Fix-Its to aid migration from selectors spelled as string literals ("foo:bar:", which is deprecated), as well as from construction of Selector instances from string literals (Selector("foo:bar"), which is still acceptable but not recommended), to the #selector syntax. Jump through some hoops to disambiguate method references if there are overloads: fixits.swift:51:7: warning: use of string literal for Objective-C selectors is deprecated; use '#selector' instead _ = "overloadedWithInt:" as Selector ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ #selector(Bar.overloaded(_:) as (Bar) -> (Int) -> ()) In the cases where we cannot provide a Fix-It to a #selector expression, we wrap the string literal in a Selector(...) construction to suppress the deprecation warning. These are also easily searchable in the code base. This also means we're doing more validation of the string literals that go into Selector, i.e., that they are well-formed selectors and that we know about some method that is @objc and has that selector. We'll warn if either is untrue.
1 parent b8f3e29 commit b66e65c

File tree

18 files changed

+677
-6
lines changed

18 files changed

+677
-6
lines changed

include/swift/AST/DeclContext.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -405,6 +405,11 @@ class alignas(1 << DeclContextAlignInBits) DeclContext {
405405
LazyResolver *typeResolver,
406406
SmallVectorImpl<ValueDecl *> &decls) const;
407407

408+
/// Look up Objective-C methods with the given selector.
409+
void lookupObjCMethods(
410+
ObjCSelector selector,
411+
SmallVectorImpl<AbstractFunctionDecl *> &results) const;
412+
408413
/// Return the ASTContext for a specified DeclContext by
409414
/// walking up to the enclosing module and returning its ASTContext.
410415
ASTContext &getASTContext() const;

include/swift/AST/DiagnosticsSema.def

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -336,7 +336,6 @@ ERROR(noescape_functiontype_mismatch,none,
336336
"invalid conversion from non-escaping function of type %0 to "
337337
"potentially escaping function type %1", (Type, Type))
338338

339-
340339
// Selector expressions.
341340
ERROR(expr_selector_no_objc_runtime,none,
342341
"'#selector' can only be used with the Objective-C runtime", ())
@@ -356,6 +355,20 @@ NOTE(expr_selector_make_objc,none,
356355
"add '@objc' to expose this %select{method|initializer}0 to Objective-C",
357356
(bool))
358357

358+
// Selectors-as-string-literals.
359+
WARNING(selector_literal_invalid,none,
360+
"string literal is not a valid Objective-C selector", ())
361+
WARNING(selector_literal_undeclared,none,
362+
"no method declared with Objective-C selector %0", (ObjCSelector))
363+
WARNING(selector_literal_deprecated,none,
364+
"use of string literal for Objective-C selectors is deprecated; "
365+
"use '#selector' or explicitly construct a 'Selector'", ())
366+
WARNING(selector_literal_deprecated_suggest,none,
367+
"use of string literal for Objective-C selectors is deprecated; "
368+
"use '#selector' instead", ())
369+
WARNING(selector_construction_suggest,none,
370+
"use '#selector' instead of explicitly constructing a 'Selector'", ())
371+
359372
ERROR(cannot_return_value_from_void_func,none,
360373
"unexpected non-void return value in void function", ())
361374

include/swift/AST/Module.h

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -409,6 +409,11 @@ class ModuleDecl : public TypeDecl, public DeclContext {
409409
DeclContext *container, DeclName name,
410410
Identifier privateDiscriminator) const;
411411

412+
/// Find all Objective-C methods with the given selector.
413+
void lookupObjCMethods(
414+
ObjCSelector selector,
415+
SmallVectorImpl<AbstractFunctionDecl *> &results) const;
416+
412417
/// \sa getImportedModules
413418
enum class ImportFilter {
414419
All,
@@ -639,6 +644,11 @@ class FileUnit : public DeclContext {
639644
DeclName name,
640645
SmallVectorImpl<ValueDecl*> &results) const {}
641646

647+
/// Find all Objective-C methods with the given selector.
648+
virtual void lookupObjCMethods(
649+
ObjCSelector selector,
650+
SmallVectorImpl<AbstractFunctionDecl *> &results) const = 0;
651+
642652
/// Returns the comment attached to the given declaration.
643653
///
644654
/// This function is an implementation detail for comment serialization.
@@ -804,6 +814,10 @@ class DerivedFileUnit final : public FileUnit {
804814

805815
void getTopLevelDecls(SmallVectorImpl<Decl*> &results) const override;
806816

817+
void lookupObjCMethods(
818+
ObjCSelector selector,
819+
SmallVectorImpl<AbstractFunctionDecl *> &results) const override;
820+
807821
Identifier
808822
getDiscriminatorForPrivateValue(const ValueDecl *D) const override {
809823
llvm_unreachable("no private decls in the derived file unit");
@@ -903,6 +917,10 @@ class SourceFile final : public FileUnit {
903917
/// complete, we diagnose.
904918
std::map<DeclAttrKind, const DeclAttribute *> AttrsRequiringFoundation;
905919

920+
/// A mapping from Objective-C selectors to the methods that have
921+
/// those selectors.
922+
llvm::DenseMap<ObjCSelector, std::vector<AbstractFunctionDecl *>> ObjCMethods;
923+
906924
template <typename T>
907925
using OperatorMap = llvm::DenseMap<Identifier,llvm::PointerIntPair<T,1,bool>>;
908926

@@ -959,6 +977,10 @@ class SourceFile final : public FileUnit {
959977
lookupClassMember(ModuleDecl::AccessPathTy accessPath, DeclName name,
960978
SmallVectorImpl<ValueDecl*> &results) const override;
961979

980+
void lookupObjCMethods(
981+
ObjCSelector selector,
982+
SmallVectorImpl<AbstractFunctionDecl *> &results) const override;
983+
962984
virtual void getTopLevelDecls(SmallVectorImpl<Decl*> &results) const override;
963985

964986
virtual void
@@ -1120,6 +1142,11 @@ class BuiltinUnit final : public FileUnit {
11201142
NLKind lookupKind,
11211143
SmallVectorImpl<ValueDecl*> &result) const override;
11221144

1145+
/// Find all Objective-C methods with the given selector.
1146+
void lookupObjCMethods(
1147+
ObjCSelector selector,
1148+
SmallVectorImpl<AbstractFunctionDecl *> &results) const override;
1149+
11231150
Identifier
11241151
getDiscriminatorForPrivateValue(const ValueDecl *D) const override {
11251152
llvm_unreachable("no private values in the Builtin module");

include/swift/ClangImporter/ClangModule.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,10 @@ class ClangModuleUnit final : public LoadedFile {
7373
lookupClassMember(ModuleDecl::AccessPathTy accessPath, DeclName name,
7474
SmallVectorImpl<ValueDecl*> &decls) const override;
7575

76+
void lookupObjCMethods(
77+
ObjCSelector selector,
78+
SmallVectorImpl<AbstractFunctionDecl *> &results) const override;
79+
7680
virtual void getTopLevelDecls(SmallVectorImpl<Decl*> &results) const override;
7781

7882
virtual void getDisplayDecls(SmallVectorImpl<Decl*> &results) const override;

include/swift/Serialization/ModuleFile.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -553,6 +553,11 @@ class ModuleFile : public LazyMemberLoader {
553553
DeclName name,
554554
SmallVectorImpl<ValueDecl*> &results);
555555

556+
/// Find all Objective-C methods with the given selector.
557+
void lookupObjCMethods(
558+
ObjCSelector selector,
559+
SmallVectorImpl<AbstractFunctionDecl *> &results);
560+
556561
/// Reports all link-time dependencies.
557562
void collectLinkLibraries(Module::LinkLibraryCallback callback) const;
558563

include/swift/Serialization/SerializedModuleLoader.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -131,6 +131,11 @@ class SerializedASTFile final : public LoadedFile {
131131
lookupClassMember(Module::AccessPathTy accessPath, DeclName name,
132132
SmallVectorImpl<ValueDecl*> &decls) const override;
133133

134+
/// Find all Objective-C methods with the given selector.
135+
void lookupObjCMethods(
136+
ObjCSelector selector,
137+
SmallVectorImpl<AbstractFunctionDecl *> &results) const override;
138+
134139
Optional<BriefAndRawComment> getCommentForDecl(const Decl *D) const override;
135140

136141
virtual void getTopLevelDecls(SmallVectorImpl<Decl*> &results) const override;

lib/AST/Module.cpp

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -519,12 +519,24 @@ void Module::lookupMember(SmallVectorImpl<ValueDecl*> &results,
519519
}
520520
}
521521

522+
void Module::lookupObjCMethods(
523+
ObjCSelector selector,
524+
SmallVectorImpl<AbstractFunctionDecl *> &results) const {
525+
FORWARD(lookupObjCMethods, (selector, results));
526+
}
527+
522528
void BuiltinUnit::lookupValue(Module::AccessPathTy accessPath, DeclName name,
523529
NLKind lookupKind,
524530
SmallVectorImpl<ValueDecl*> &result) const {
525531
getCache().lookupValue(name.getBaseName(), lookupKind, *this, result);
526532
}
527533

534+
void BuiltinUnit::lookupObjCMethods(
535+
ObjCSelector selector,
536+
SmallVectorImpl<AbstractFunctionDecl *> &results) const {
537+
// No @objc methods in the Builtin module.
538+
}
539+
528540
DerivedFileUnit::DerivedFileUnit(Module &M)
529541
: FileUnit(FileUnitKind::Derived, M) {
530542
M.getASTContext().addDestructorCleanup(*this);
@@ -561,6 +573,17 @@ void DerivedFileUnit::lookupVisibleDecls(Module::AccessPathTy accessPath,
561573
}
562574
}
563575

576+
void DerivedFileUnit::lookupObjCMethods(
577+
ObjCSelector selector,
578+
SmallVectorImpl<AbstractFunctionDecl *> &results) const {
579+
for (auto D : DerivedDecls) {
580+
if (auto func = dyn_cast<AbstractFunctionDecl>(D)) {
581+
if (func->isObjC() && func->getObjCSelector() == selector)
582+
results.push_back(func);
583+
}
584+
}
585+
}
586+
564587
void DerivedFileUnit::getTopLevelDecls(SmallVectorImpl<swift::Decl *> &results)
565588
const {
566589
results.append(DerivedDecls.begin(), DerivedDecls.end());
@@ -606,6 +629,15 @@ void SourceFile::lookupClassMember(Module::AccessPathTy accessPath,
606629
getCache().lookupClassMember(accessPath, name, results, *this);
607630
}
608631

632+
void SourceFile::lookupObjCMethods(
633+
ObjCSelector selector,
634+
SmallVectorImpl<AbstractFunctionDecl *> &results) const {
635+
// FIXME: Make sure this table is complete, somehow.
636+
auto known = ObjCMethods.find(selector);
637+
if (known == ObjCMethods.end()) return;
638+
results.append(known->second.begin(), known->second.end());
639+
}
640+
609641
void Module::getLocalTypeDecls(SmallVectorImpl<TypeDecl*> &Results) const {
610642
FORWARD(getLocalTypeDecls, (Results));
611643
}

lib/AST/NameLookup.cpp

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1374,3 +1374,21 @@ bool DeclContext::lookupQualified(Type type,
13741374
// We're done. Report success/failure.
13751375
return !decls.empty();
13761376
}
1377+
1378+
void DeclContext::lookupObjCMethods(
1379+
ObjCSelector selector,
1380+
SmallVectorImpl<AbstractFunctionDecl *> &results) const {
1381+
// Collect all of the methods with this selector.
1382+
forAllVisibleModules(this, [&](Module::ImportedModule import) {
1383+
import.second->lookupObjCMethods(selector, results);
1384+
});
1385+
1386+
// Filter out duplicates.
1387+
llvm::SmallPtrSet<AbstractFunctionDecl *, 8> visited;
1388+
results.erase(
1389+
std::remove_if(results.begin(), results.end(),
1390+
[&](AbstractFunctionDecl *func) -> bool {
1391+
return !visited.insert(func).second;
1392+
}),
1393+
results.end());
1394+
}

lib/ClangImporter/ClangImporter.cpp

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3809,6 +3809,59 @@ void ClangModuleUnit::lookupClassMembers(Module::AccessPathTy accessPath,
38093809
}
38103810
}
38113811

3812+
void ClangModuleUnit::lookupObjCMethods(
3813+
ObjCSelector selector,
3814+
SmallVectorImpl<AbstractFunctionDecl *> &results) const {
3815+
// FIXME: Ignore submodules, which are empty for now.
3816+
if (clangModule && clangModule->isSubModule())
3817+
return;
3818+
3819+
// Map the selector into a Clang selector.
3820+
auto clangSelector = owner.Impl.exportSelector(selector);
3821+
if (clangSelector.isNull()) return;
3822+
3823+
// Collect all of the Objective-C methods with this selector.
3824+
SmallVector<clang::ObjCMethodDecl *, 8> objcMethods;
3825+
auto &clangSema = owner.Impl.getClangSema();
3826+
clangSema.CollectMultipleMethodsInGlobalPool(clangSelector,
3827+
objcMethods,
3828+
/*instance=*/true);
3829+
clangSema.CollectMultipleMethodsInGlobalPool(clangSelector,
3830+
objcMethods,
3831+
/*instance=*/false);
3832+
3833+
// Import the methods.
3834+
auto &clangCtx = clangSema.getASTContext();
3835+
for (auto objcMethod : objcMethods) {
3836+
// Verify that this method came from this module.
3837+
auto owningClangModule = getClangOwningModule(objcMethod, clangCtx);
3838+
if (owningClangModule)
3839+
owningClangModule = owningClangModule->getTopLevelModule();
3840+
3841+
if (owningClangModule != clangModule) continue;
3842+
3843+
// If we found a property accessor, import the property.
3844+
if (objcMethod->isPropertyAccessor())
3845+
(void)owner.Impl.importDecl(objcMethod->findPropertyDecl(true));
3846+
3847+
// Import it.
3848+
// FIXME: Retrying a failed import works around recursion bugs in the Clang
3849+
// importer.
3850+
auto imported = owner.Impl.importDecl(objcMethod);
3851+
if (!imported) imported = owner.Impl.importDecl(objcMethod);
3852+
if (!imported) continue;
3853+
3854+
if (auto func = dyn_cast<AbstractFunctionDecl>(imported))
3855+
results.push_back(func);
3856+
3857+
// If there is an alternate declaration, also look at it.
3858+
if (auto alternate = owner.Impl.getAlternateDecl(imported)) {
3859+
if (auto func = dyn_cast<AbstractFunctionDecl>(alternate))
3860+
results.push_back(func);
3861+
}
3862+
}
3863+
}
3864+
38123865
void ClangModuleUnit::collectLinkLibraries(
38133866
Module::LinkLibraryCallback callback) const {
38143867
if (!clangModule)

0 commit comments

Comments
 (0)