[clang] 7437a94 - [SemaObjC] Add a warning for @selector expressions that potentially refer to objc_direct methods
Erik Pilkington via cfe-commits
cfe-commits at lists.llvm.org
Tue Jul 7 10:30:04 PDT 2020
Author: Erik Pilkington
Date: 2020-07-07T13:29:54-04:00
New Revision: 7437a9496528b838e6939dbcbb69a0acb5e1332d
URL: https://github.com/llvm/llvm-project/commit/7437a9496528b838e6939dbcbb69a0acb5e1332d
DIFF: https://github.com/llvm/llvm-project/commit/7437a9496528b838e6939dbcbb69a0acb5e1332d.diff
LOG: [SemaObjC] Add a warning for @selector expressions that potentially refer to objc_direct methods
By default, only warn when the selector matches a direct method in the current
class. This commit also adds a more strict off-by-default warning when there
isn't a non-direct method in the current class.
rdar://64621668
Differential revision: https://reviews.llvm.org/D82611
Added:
clang/test/SemaObjC/potentially-direct-selector.m
Modified:
clang/include/clang/Basic/DiagnosticGroups.td
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/lib/Sema/SemaExprObjC.cpp
Removed:
################################################################################
diff --git a/clang/include/clang/Basic/DiagnosticGroups.td b/clang/include/clang/Basic/DiagnosticGroups.td
index 37e0b77e79ed..76c38686a050 100644
--- a/clang/include/clang/Basic/DiagnosticGroups.td
+++ b/clang/include/clang/Basic/DiagnosticGroups.td
@@ -1099,6 +1099,11 @@ def ObjCSignedCharBool : DiagGroup<"objc-signed-char-bool",
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
index c935545610e0..e99fd3d1f105 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -1378,8 +1378,14 @@ def warn_multiple_selectors: Warning<
"several methods with selector %0 of mismatched types are found "
"for the @selector expression">,
InGroup<SelectorTypeMismatch>, 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<ObjCPotentiallyDirectSelector>;
+def warn_strict_potentially_direct_selector_expression : Warning<
+ warn_potentially_direct_selector_expression.Text>,
+ InGroup<ObjCStrictPotentiallyDirectSelector>, 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
index 192e4184d144..228a1ec3ba1f 100644
--- a/clang/lib/Sema/SemaExprObjC.cpp
+++ b/clang/lib/Sema/SemaExprObjC.cpp
@@ -1228,33 +1228,66 @@ static void DiagnoseMismatchedSelectors(Sema &S, SourceLocation AtLoc,
}
}
-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 @@ ExprResult Sema::ParseObjCSelectorExpression(Selector Sel,
} 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
index 000000000000..04f9d2111c9b
--- /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))
+ at interface Dummies
+-(void)inBase;
+-(void)inBaseImpl;
+-(void)inBaseCat;
+-(void)inBaseCatImpl;
+-(void)inDerived;
+-(void)inDerivedImpl;
+-(void)inDerivedCat;
+-(void)inDerivedCatImpl;
++(void)inBaseClass;
++(void)inDerivedClass;
++(void)inDerivedCatClass;
+ at end
+
+__attribute__((objc_root_class))
+ at interface Base
+-(void)inBase NS_DIRECT; // expected-note + {{direct method}}
++(void)inBaseClass NS_DIRECT; // expected-note + {{direct method}}
+ at end
+
+ at implementation Base
+-(void)inBaseImpl NS_DIRECT { // expected-note + {{direct method}}
+}
+-(void)inBase {}
++(void)inBaseClass {}
+ at end
+
+ at interface Base (Cat)
+-(void)inBaseCat NS_DIRECT; // expected-note + {{direct method}}
+ at end
+
+ at implementation Base (Cat)
+-(void)inBaseCatImpl NS_DIRECT { // expected-note + {{direct method}}
+}
+-(void)inBaseCat {}
+ at end
+
+ at interface Derived : Base
+-(void)inDerived NS_DIRECT; // expected-note + {{direct method}}
++(void)inDerivedClass NS_DIRECT; // expected-note + {{direct method}}
+ at end
+
+ at implementation Derived
+-(void)inDerivedImpl NS_DIRECT { // expected-note + {{direct method}}
+}
+-(void)inDerived {}
++(void)inDerivedClass {}
+ at end
+
+ at interface Derived (Cat)
+-(void)inDerivedCat NS_DIRECT; // expected-note + {{direct method}}
++(void)inDerivedCatClass NS_DIRECT; // expected-note + {{direct method}}
+ at end
+
+ at 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}}
+}
+ at 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}}
+}
+
+ at interface OnlyBase : Base @end
+ at 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}}
+}
+ at end
+
+__attribute__((objc_root_class))
+ at interface Unrelated @end
+ at 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}}
+}
+ at end
+
+ at 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);
+}
+ at end
More information about the cfe-commits
mailing list