diff --git a/clang/include/clang/Basic/DiagnosticGroups.td b/clang/include/clang/Basic/DiagnosticGroups.td --- a/clang/include/clang/Basic/DiagnosticGroups.td +++ b/clang/include/clang/Basic/DiagnosticGroups.td @@ -1099,6 +1099,11 @@ ObjCBoolConstantConversion, TautologicalObjCBoolCompare]>; +def ObjCPotentiallyDirectSelector : DiagGroup<"potentially-direct-selector">; +def ObjCStrictPotentiallyDirectSelector : + DiagGroup<"strict-potentially-direct-selector", + [ObjCPotentiallyDirectSelector]>; + // Inline ASM warnings. def ASMOperandWidths : DiagGroup<"asm-operand-widths">; def ASM : DiagGroup<"asm", [ diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -1378,8 +1378,14 @@ "several methods with selector %0 of mismatched types are found " "for the @selector expression">, InGroup, DefaultIgnore; -def err_direct_selector_expression: Error< +def err_direct_selector_expression : Error< "@selector expression formed with direct selector %0">; +def warn_potentially_direct_selector_expression : Warning< + "@selector expression formed with potentially direct selector %0">, + InGroup; +def warn_strict_potentially_direct_selector_expression : Warning< + warn_potentially_direct_selector_expression.Text>, + InGroup, DefaultIgnore; def err_objc_kindof_nonobject : Error< "'__kindof' specifier cannot be applied to non-object type %0">; diff --git a/clang/lib/Sema/SemaExprObjC.cpp b/clang/lib/Sema/SemaExprObjC.cpp --- a/clang/lib/Sema/SemaExprObjC.cpp +++ b/clang/lib/Sema/SemaExprObjC.cpp @@ -1228,33 +1228,66 @@ } } -static void HelperToDiagnoseDirectSelectorsExpr(Sema &S, SourceLocation AtLoc, - Selector Sel, - ObjCMethodList &MethList, - bool &onlyDirect) { +static ObjCMethodDecl *LookupDirectMethodInMethodList(Sema &S, Selector Sel, + ObjCMethodList &MethList, + bool &onlyDirect, + bool &anyDirect) { + (void)Sel; ObjCMethodList *M = &MethList; - for (M = M->getNext(); M; M = M->getNext()) { + ObjCMethodDecl *DirectMethod = nullptr; + for (; M; M = M->getNext()) { ObjCMethodDecl *Method = M->getMethod(); - if (Method->getSelector() != Sel) + if (!Method) continue; - if (!Method->isDirectMethod()) + assert(Method->getSelector() == Sel && "Method with wrong selector in method list"); + if (Method->isDirectMethod()) { + anyDirect = true; + DirectMethod = Method; + } else onlyDirect = false; } + + return DirectMethod; } -static void DiagnoseDirectSelectorsExpr(Sema &S, SourceLocation AtLoc, - Selector Sel, bool &onlyDirect) { - for (Sema::GlobalMethodPool::iterator b = S.MethodPool.begin(), - e = S.MethodPool.end(); b != e; b++) { - // first, instance methods - ObjCMethodList &InstMethList = b->second.first; - HelperToDiagnoseDirectSelectorsExpr(S, AtLoc, Sel, InstMethList, - onlyDirect); +// Search the global pool for (potentially) direct methods matching the given +// selector. If a non-direct method is found, set \param onlyDirect to false. If +// a direct method is found, set \param anyDirect to true. Returns a direct +// method, if any. +static ObjCMethodDecl *LookupDirectMethodInGlobalPool(Sema &S, Selector Sel, + bool &onlyDirect, + bool &anyDirect) { + auto Iter = S.MethodPool.find(Sel); + if (Iter == S.MethodPool.end()) + return nullptr; - // second, class methods - ObjCMethodList &ClsMethList = b->second.second; - HelperToDiagnoseDirectSelectorsExpr(S, AtLoc, Sel, ClsMethList, onlyDirect); - } + ObjCMethodDecl *DirectInstance = LookupDirectMethodInMethodList( + S, Sel, Iter->second.first, onlyDirect, anyDirect); + ObjCMethodDecl *DirectClass = LookupDirectMethodInMethodList( + S, Sel, Iter->second.second, onlyDirect, anyDirect); + + return DirectInstance ? DirectInstance : DirectClass; +} + +static ObjCMethodDecl *findMethodInCurrentClass(Sema &S, Selector Sel) { + auto *CurMD = S.getCurMethodDecl(); + if (!CurMD) + return nullptr; + ObjCInterfaceDecl *IFace = CurMD->getClassInterface(); + + // The language enforce that only one direct method is present in a given + // class, so we just need to find one method in the current class to know + // whether Sel is potentially direct in this context. + if (ObjCMethodDecl *MD = IFace->lookupMethod(Sel, /*isInstance=*/true)) + return MD; + if (ObjCMethodDecl *MD = IFace->lookupPrivateMethod(Sel, /*isInstance=*/true)) + return MD; + if (ObjCMethodDecl *MD = IFace->lookupMethod(Sel, /*isInstance=*/false)) + return MD; + if (ObjCMethodDecl *MD = IFace->lookupPrivateMethod(Sel, /*isInstance=*/false)) + return MD; + + return nullptr; } ExprResult Sema::ParseObjCSelectorExpression(Selector Sel, @@ -1280,15 +1313,38 @@ } else Diag(SelLoc, diag::warn_undeclared_selector) << Sel; } else { - bool onlyDirect = Method->isDirectMethod(); - DiagnoseDirectSelectorsExpr(*this, AtLoc, Sel, onlyDirect); DiagnoseMismatchedSelectors(*this, AtLoc, Method, LParenLoc, RParenLoc, WarnMultipleSelectors); + + bool onlyDirect = true; + bool anyDirect = false; + ObjCMethodDecl *GlobalDirectMethod = + LookupDirectMethodInGlobalPool(*this, Sel, onlyDirect, anyDirect); + if (onlyDirect) { Diag(AtLoc, diag::err_direct_selector_expression) << Method->getSelector(); Diag(Method->getLocation(), diag::note_direct_method_declared_at) << Method->getDeclName(); + } else if (anyDirect) { + // If we saw any direct methods, see if we see a direct member of the + // current class. If so, the @selector will likely be used to refer to + // this direct method. + ObjCMethodDecl *LikelyTargetMethod = findMethodInCurrentClass(*this, Sel); + if (LikelyTargetMethod && LikelyTargetMethod->isDirectMethod()) { + Diag(AtLoc, diag::warn_potentially_direct_selector_expression) << Sel; + Diag(LikelyTargetMethod->getLocation(), + diag::note_direct_method_declared_at) + << LikelyTargetMethod->getDeclName(); + } else if (!LikelyTargetMethod) { + // Otherwise, emit the "strict" variant of this diagnostic, unless + // LikelyTargetMethod is non-direct. + Diag(AtLoc, diag::warn_strict_potentially_direct_selector_expression) + << Sel; + Diag(GlobalDirectMethod->getLocation(), + diag::note_direct_method_declared_at) + << GlobalDirectMethod->getDeclName(); + } } } diff --git a/clang/test/SemaObjC/potentially-direct-selector.m b/clang/test/SemaObjC/potentially-direct-selector.m new file mode 100644 --- /dev/null +++ b/clang/test/SemaObjC/potentially-direct-selector.m @@ -0,0 +1,157 @@ +// RUN: %clang_cc1 %s -Wpotentially-direct-selector -verify +// RUN: %clang_cc1 %s -Wstrict-potentially-direct-selector -verify=expected,strict + +#define NS_DIRECT __attribute__((objc_direct)) + +__attribute__((objc_root_class)) +@interface Dummies +-(void)inBase; +-(void)inBaseImpl; +-(void)inBaseCat; +-(void)inBaseCatImpl; +-(void)inDerived; +-(void)inDerivedImpl; +-(void)inDerivedCat; +-(void)inDerivedCatImpl; ++(void)inBaseClass; ++(void)inDerivedClass; ++(void)inDerivedCatClass; +@end + +__attribute__((objc_root_class)) +@interface Base +-(void)inBase NS_DIRECT; // expected-note + {{direct method}} ++(void)inBaseClass NS_DIRECT; // expected-note + {{direct method}} +@end + +@implementation Base +-(void)inBaseImpl NS_DIRECT { // expected-note + {{direct method}} +} +-(void)inBase {} ++(void)inBaseClass {} +@end + +@interface Base (Cat) +-(void)inBaseCat NS_DIRECT; // expected-note + {{direct method}} +@end + +@implementation Base (Cat) +-(void)inBaseCatImpl NS_DIRECT { // expected-note + {{direct method}} +} +-(void)inBaseCat {} +@end + +@interface Derived : Base +-(void)inDerived NS_DIRECT; // expected-note + {{direct method}} ++(void)inDerivedClass NS_DIRECT; // expected-note + {{direct method}} +@end + +@implementation Derived +-(void)inDerivedImpl NS_DIRECT { // expected-note + {{direct method}} +} +-(void)inDerived {} ++(void)inDerivedClass {} +@end + +@interface Derived (Cat) +-(void)inDerivedCat NS_DIRECT; // expected-note + {{direct method}} ++(void)inDerivedCatClass NS_DIRECT; // expected-note + {{direct method}} +@end + +@implementation Derived (Cat) +-(void)inDerivedCatImpl NS_DIRECT { // expected-note + {{direct method}} +} +-(void)inDerivedCat {} ++(void)inDerivedCatClass {} + +-(void)test1 { + (void)@selector(inBase); // expected-warning{{@selector expression formed with potentially direct selector}} + (void)@selector(inBaseImpl); // expected-warning{{@selector expression formed with potentially direct selector}} + (void)@selector(inBaseCat); // expected-warning{{@selector expression formed with potentially direct selector}} + (void)@selector(inBaseCatImpl); // expected-warning{{@selector expression formed with potentially direct selector}} + (void)@selector(inDerived); // expected-warning{{@selector expression formed with potentially direct selector}} + (void)@selector(inDerivedImpl); // expected-warning{{@selector expression formed with potentially direct selector}} + (void)@selector(inDerivedCat); // expected-warning{{@selector expression formed with potentially direct selector}} + (void)@selector(inDerivedCatImpl); // expected-warning{{@selector expression formed with potentially direct selector}} + (void)@selector(inDerivedClass); // expected-warning{{@selector expression formed with potentially direct selector}} + (void)@selector(inBaseClass); // expected-warning{{@selector expression formed with potentially direct selector}} + (void)@selector(inDerivedCatClass); // expected-warning{{@selector expression formed with potentially direct selector}} +} +@end + +void test2() { + (void)@selector(inBase); // strict-warning{{@selector expression formed with potentially direct selector}} + (void)@selector(inBaseImpl); // strict-warning{{@selector expression formed with potentially direct selector}} + (void)@selector(inBaseCat); // strict-warning{{@selector expression formed with potentially direct selector}} + (void)@selector(inBaseCatImpl); // strict-warning{{@selector expression formed with potentially direct selector}} + (void)@selector(inDerived); // strict-warning{{@selector expression formed with potentially direct selector}} + (void)@selector(inDerivedImpl); // strict-warning{{@selector expression formed with potentially direct selector}} + (void)@selector(inDerivedCat); // strict-warning{{@selector expression formed with potentially direct selector}} + (void)@selector(inDerivedCatImpl); // strict-warning{{@selector expression formed with potentially direct selector}} + (void)@selector(inDerivedClass); // strict-warning{{@selector expression formed with potentially direct selector}} + (void)@selector(inBaseClass); // strict-warning{{@selector expression formed with potentially direct selector}} + (void)@selector(inDerivedCatClass); // strict-warning{{@selector expression formed with potentially direct selector}} +} + +@interface OnlyBase : Base @end +@implementation OnlyBase +-(void)test3 { + (void)@selector(inBase); // expected-warning{{@selector expression formed with potentially direct selector}} + (void)@selector(inBaseImpl); // expected-warning{{@selector expression formed with potentially direct selector}} + (void)@selector(inBaseCat); // expected-warning{{@selector expression formed with potentially direct selector}} + (void)@selector(inBaseCatImpl); // expected-warning{{@selector expression formed with potentially direct selector}} + (void)@selector(inDerived); // strict-warning{{@selector expression formed with potentially direct selector}} + (void)@selector(inDerivedImpl); // strict-warning{{@selector expression formed with potentially direct selector}} + (void)@selector(inDerivedCat); // strict-warning{{@selector expression formed with potentially direct selector}} + (void)@selector(inDerivedCatImpl); // strict-warning{{@selector expression formed with potentially direct selector}} + (void)@selector(inDerivedClass); // strict-warning{{@selector expression formed with potentially direct selector}} + (void)@selector(inBaseClass); // expected-warning{{@selector expression formed with potentially direct selector}} + (void)@selector(inDerivedCatClass); // strict-warning{{@selector expression formed with potentially direct selector}} +} +@end + +__attribute__((objc_root_class)) +@interface Unrelated @end +@implementation Unrelated +-(void)test4 { + (void)@selector(inBase); // strict-warning{{@selector expression formed with potentially direct selector}} + (void)@selector(inBaseImpl); // strict-warning{{@selector expression formed with potentially direct selector}} + (void)@selector(inBaseCat); // strict-warning{{@selector expression formed with potentially direct selector}} + (void)@selector(inBaseCatImpl); // strict-warning{{@selector expression formed with potentially direct selector}} + (void)@selector(inDerived); // strict-warning{{@selector expression formed with potentially direct selector}} + (void)@selector(inDerivedImpl); // strict-warning{{@selector expression formed with potentially direct selector}} + (void)@selector(inDerivedCat); // strict-warning{{@selector expression formed with potentially direct selector}} + (void)@selector(inDerivedCatImpl); // strict-warning{{@selector expression formed with potentially direct selector}} + (void)@selector(inDerivedClass); // strict-warning{{@selector expression formed with potentially direct selector}} + (void)@selector(inBaseClass); // strict-warning{{@selector expression formed with potentially direct selector}} + (void)@selector(inDerivedCatClass); // strict-warning{{@selector expression formed with potentially direct selector}} +} +@end + +@implementation Dummies +-(void)inBase {} +-(void)inBaseImpl {} +-(void)inBaseCat {} +-(void)inBaseCatImpl {} +-(void)inDerived {} +-(void)inDerivedImpl {} +-(void)inDerivedCat {} +-(void)inDerivedCatImpl {} ++(void)inBaseClass {} ++(void)inDerivedClass {} ++(void)inDerivedCatClass {} + +-(void)test5 { + (void)@selector(inBase); + (void)@selector(inBaseImpl); + (void)@selector(inBaseCat); + (void)@selector(inBaseCatImpl); + (void)@selector(inDerived); + (void)@selector(inDerivedImpl); + (void)@selector(inDerivedCat); + (void)@selector(inDerivedCatImpl); + (void)@selector(inDerivedClass); + (void)@selector(inBaseClass); + (void)@selector(inDerivedCatClass); +} +@end