<div dir="ltr">Awesome! This attribute has been incredibly awkward to use without this.</div><div class="gmail_extra"><br><br><div class="gmail_quote">On Thu, Jan 16, 2014 at 10:24 PM, Ted Kremenek <span dir="ltr"><<a href="mailto:kremenek@apple.com" target="_blank">kremenek@apple.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: kremenek<br>
Date: Fri Jan 17 00:24:56 2014<br>
New Revision: 199467<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=199467&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=199467&view=rev</a><br>
Log:<br>
Enhance attribute 'nonnull' to be applicable to parameters directly (infix).<br>
<br>
This allows the following syntax:<br>
<br>
void baz(__attribute__((nonnull)) const char *str);<br>
<br>
instead of:<br>
<br>
void baz(const char *str) __attribute__((nonnull(1)));<br>
<br>
This also extends to Objective-C methods.<br>
<br>
The checking logic in Sema is not as clean as I would like. Effectively<br>
now we need to check both the FunctionDecl/ObjCMethodDecl and the parameters,<br>
so the point of truth is spread in two places, but the logic isn't that<br>
cumbersome.<br>
<br>
Implements <rdar://problem/14691443>.<br>
<br>
Modified:<br>
cfe/trunk/include/clang/Basic/Attr.td<br>
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td<br>
cfe/trunk/lib/Sema/SemaChecking.cpp<br>
cfe/trunk/lib/Sema/SemaDeclAttr.cpp<br>
cfe/trunk/test/Sema/nonnull.c<br>
cfe/trunk/test/SemaObjC/nonnull.m<br>
<br>
Modified: cfe/trunk/include/clang/Basic/Attr.td<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Attr.td?rev=199467&r1=199466&r2=199467&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Attr.td?rev=199467&r1=199466&r2=199467&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/include/clang/Basic/Attr.td (original)<br>
+++ cfe/trunk/include/clang/Basic/Attr.td Fri Jan 17 00:24:56 2014<br>
@@ -679,8 +679,9 @@ def NoMips16 : InheritableAttr, TargetSp<br>
<br>
def NonNull : InheritableAttr {<br>
let Spellings = [GNU<"nonnull">, CXX11<"gnu", "nonnull">];<br>
- let Subjects = SubjectList<[ObjCMethod, FunctionLike, HasFunctionProto],<br>
- WarnDiag, "ExpectedFunctionOrMethod">;<br>
+ let Subjects = SubjectList<[ObjCMethod, FunctionLike, HasFunctionProto,<br>
+ ParmVar],<br>
+ WarnDiag, "ExpectedFunctionMethodOrParameter">;<br>
let Args = [VariadicUnsignedArgument<"Args">];<br>
let AdditionalMembers =<br>
[{bool isNonNull(unsigned idx) const {<br>
<br>
Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=199467&r1=199466&r2=199467&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=199467&r1=199466&r2=199467&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)<br>
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Fri Jan 17 00:24:56 2014<br>
@@ -2351,6 +2351,9 @@ def err_attr_wrong_decl : Error<<br>
def warn_attribute_nonnull_no_pointers : Warning<<br>
"'nonnull' attribute applied to function with no pointer arguments">,<br>
InGroup<IgnoredAttributes>;<br>
+def warn_attribute_nonnull_parm_no_args : Warning<<br>
+ "'nonnull' attribute when used on parameters takes no arguments">,<br>
+ InGroup<IgnoredAttributes>;<br>
def warn_attribute_malloc_pointer_only : Warning<<br>
"'malloc' attribute only applies to functions returning a pointer type">,<br>
InGroup<IgnoredAttributes>;<br>
<br>
Modified: cfe/trunk/lib/Sema/SemaChecking.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=199467&r1=199466&r2=199467&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=199467&r1=199466&r2=199467&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/lib/Sema/SemaChecking.cpp (original)<br>
+++ cfe/trunk/lib/Sema/SemaChecking.cpp Fri Jan 17 00:24:56 2014<br>
@@ -736,6 +736,7 @@ static void CheckNonNullArguments(Sema &<br>
const NamedDecl *FDecl,<br>
const Expr * const *ExprArgs,<br>
SourceLocation CallSiteLoc) {<br>
+ // Check the attributes attached to the method/function itself.<br>
for (specific_attr_iterator<NonNullAttr><br>
I = FDecl->specific_attr_begin<NonNullAttr>(),<br>
E = FDecl->specific_attr_end<NonNullAttr>(); I != E; ++I) {<br>
@@ -747,6 +748,21 @@ static void CheckNonNullArguments(Sema &<br>
CheckNonNullArgument(S, ExprArgs[*i], CallSiteLoc);<br>
}<br>
}<br>
+<br>
+ // Check the attributes on the parameters.<br>
+ ArrayRef<ParmVarDecl*> parms;<br>
+ if (const FunctionDecl *FD = dyn_cast<FunctionDecl>(FDecl))<br>
+ parms = FD->parameters();<br>
+ else if (const ObjCMethodDecl *MD = dyn_cast<ObjCMethodDecl>(FDecl))<br>
+ parms = MD->parameters();<br>
+<br>
+ unsigned argIndex = 0;<br>
+ for (ArrayRef<ParmVarDecl*>::iterator I = parms.begin(), E = parms.end();<br>
+ I != E; ++I, ++argIndex) {<br>
+ const ParmVarDecl *PVD = *I;<br>
+ if (PVD->hasAttr<NonNullAttr>())<br>
+ CheckNonNullArgument(S, ExprArgs[argIndex], CallSiteLoc);<br>
+ }<br>
}<br>
<br>
/// Handles the checks for format strings, non-POD arguments to vararg<br>
<br>
Modified: cfe/trunk/lib/Sema/SemaDeclAttr.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclAttr.cpp?rev=199467&r1=199466&r2=199467&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclAttr.cpp?rev=199467&r1=199466&r2=199467&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/lib/Sema/SemaDeclAttr.cpp (original)<br>
+++ cfe/trunk/lib/Sema/SemaDeclAttr.cpp Fri Jan 17 00:24:56 2014<br>
@@ -1156,6 +1156,36 @@ static void possibleTransparentUnionPoin<br>
}<br>
}<br>
<br>
+static bool attrNonNullArgCheck(Sema &S, QualType T, const AttributeList &Attr,<br>
+ SourceRange R) {<br>
+ T = T.getNonReferenceType();<br>
+ possibleTransparentUnionPointerType(T);<br>
+<br>
+ if (!T->isAnyPointerType() && !T->isBlockPointerType()) {<br>
+ S.Diag(Attr.getLoc(), diag::warn_attribute_pointers_only)<br>
+ << Attr.getName() << R;<br>
+ return false;<br>
+ }<br>
+ return true;<br>
+}<br>
+<br>
+static void handleNonNullAttrParameter(Sema &S, ParmVarDecl *D,<br>
+ const AttributeList &Attr) {<br>
+ // Is the argument a pointer type?<br>
+ if (!attrNonNullArgCheck(S, D->getType(), Attr, D->getSourceRange()))<br>
+ return;<br>
+<br>
+ if (Attr.getNumArgs() > 0) {<br>
+ S.Diag(Attr.getLoc(), diag::warn_attribute_nonnull_parm_no_args)<br>
+ << D->getSourceRange();<br>
+ return;<br>
+ }<br>
+<br>
+ D->addAttr(::new (S.Context)<br>
+ NonNullAttr(Attr.getRange(), S.Context, 0, 0,<br>
+ Attr.getAttributeSpellingListIndex()));<br>
+}<br>
+<br>
static void handleNonNullAttr(Sema &S, Decl *D, const AttributeList &Attr) {<br>
SmallVector<unsigned, 8> NonNullArgs;<br>
for (unsigned i = 0; i < Attr.getNumArgs(); ++i) {<br>
@@ -1165,15 +1195,10 @@ static void handleNonNullAttr(Sema &S, D<br>
return;<br>
<br>
// Is the function argument a pointer type?<br>
- QualType T = getFunctionOrMethodArgType(D, Idx).getNonReferenceType();<br>
- possibleTransparentUnionPointerType(T);<br>
-<br>
- if (!T->isAnyPointerType() && !T->isBlockPointerType()) {<br>
- // FIXME: Should also highlight argument in decl.<br>
- S.Diag(Attr.getLoc(), diag::warn_attribute_pointers_only)<br>
- << Attr.getName() << Ex->getSourceRange();<br>
+ // FIXME: Should also highlight argument in decl in the diagnostic.<br>
+ if (!attrNonNullArgCheck(S, getFunctionOrMethodArgType(D, Idx),<br>
+ Attr, Ex->getSourceRange()))<br>
continue;<br>
- }<br>
<br>
NonNullArgs.push_back(Idx);<br>
}<br>
@@ -3947,7 +3972,12 @@ static void ProcessDeclAttribute(Sema &S<br>
case AttributeList::AT_Mode: handleModeAttr (S, D, Attr); break;<br>
case AttributeList::AT_NoCommon:<br>
handleSimpleAttribute<NoCommonAttr>(S, D, Attr); break;<br>
- case AttributeList::AT_NonNull: handleNonNullAttr (S, D, Attr); break;<br>
+ case AttributeList::AT_NonNull:<br>
+ if (ParmVarDecl *PVD = dyn_cast<ParmVarDecl>(D))<br>
+ handleNonNullAttrParameter(S, PVD, Attr);<br>
+ else<br>
+ handleNonNullAttr(S, D, Attr);<br>
+ break;<br>
case AttributeList::AT_Overloadable:<br>
handleSimpleAttribute<OverloadableAttr>(S, D, Attr); break;<br>
case AttributeList::AT_Ownership: handleOwnershipAttr (S, D, Attr); break;<br>
<br>
Modified: cfe/trunk/test/Sema/nonnull.c<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/nonnull.c?rev=199467&r1=199466&r2=199467&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/nonnull.c?rev=199467&r1=199466&r2=199467&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/test/Sema/nonnull.c (original)<br>
+++ cfe/trunk/test/Sema/nonnull.c Fri Jan 17 00:24:56 2014<br>
@@ -21,3 +21,14 @@ int main(void) {<br>
<br>
void foo(const char *str) __attribute__((nonnull("foo"))); // expected-error{{'nonnull' attribute requires parameter 1 to be an integer constant}}<br>
void bar(int i) __attribute__((nonnull(1))); // expected-warning {{'nonnull' attribute only applies to pointer arguments}} expected-warning {{'nonnull' attribute applied to function with no pointer arguments}}<br>
+<br>
+void baz(__attribute__((nonnull)) const char *str);<br>
+void baz2(__attribute__((nonnull(1))) const char *str); // expected-warning {{'nonnull' attribute when used on parameters takes no arguments}}<br>
+void baz3(__attribute__((nonnull)) int x); // expected-warning {{'nonnull' attribute only applies to pointer arguments}}<br>
+<br>
+void test_baz() {<br>
+ baz(0); // expected-warning {{null passed to a callee which requires a non-null argument}}<br>
+ baz2(0); // no-warning<br>
+ baz3(0); // no-warning<br>
+}<br>
+<br>
<br>
Modified: cfe/trunk/test/SemaObjC/nonnull.m<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjC/nonnull.m?rev=199467&r1=199466&r2=199467&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjC/nonnull.m?rev=199467&r1=199466&r2=199467&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/test/SemaObjC/nonnull.m (original)<br>
+++ cfe/trunk/test/SemaObjC/nonnull.m Fri Jan 17 00:24:56 2014<br>
@@ -95,3 +95,17 @@ extern void DoSomethingNotNull(void *db)<br>
}<br>
@end<br>
<br>
+__attribute__((objc_root_class))<br>
+ @interface TestNonNullParameters<br>
+- (void) doNotPassNullParameterNonPointerArg:(int)__attribute__((nonnull))x; // expected-warning {{'nonnull' attribute only applies to pointer arguments}}<br>
+- (void) doNotPassNullParameter:(id)__attribute__((nonnull))x;<br>
+- (void) doNotPassNullParameterArgIndex:(id)__attribute__((nonnull(1)))x; // expected-warning {{'nonnull' attribute when used on parameters takes no arguments}}<br>
+- (void) doNotPassNullOnMethod:(id)x __attribute__((nonnull(1)));<br>
+@end<br>
+<br>
+void test(TestNonNullParameters *f) {<br>
+ [f doNotPassNullParameter:0]; // expected-warning {{null passed to a callee which requires a non-null argument}}<br>
+ [f doNotPassNullParameterArgIndex:0]; // no-warning<br>
+ [f doNotPassNullOnMethod:0]; // expected-warning {{null passed to a callee which requires a non-null argument}}<br>
+}<br>
+<br>
<br>
<br>
_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@cs.uiuc.edu">cfe-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br>
</blockquote></div><br></div>