<html><head><meta http-equiv="Content-Type" content="text/html charset=us-ascii"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;">Thank you Aaron!<div><br><div><div>On Jan 20, 2014, at 6:25 AM, Aaron Ballman <<a href="mailto:aaron@aaronballman.com">aaron@aaronballman.com</a>> wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><div style="font-size: 14px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;">A few minor nits below, but nothing to worry about (I've fixed them up<br>in r199663).<br><br>On Mon, Jan 20, 2014 at 12:50 AM, Ted Kremenek <<a href="mailto:kremenek@apple.com">kremenek@apple.com</a>> wrote:<br><blockquote type="cite">Author: kremenek<br>Date: Sun Jan 19 23:50:47 2014<br>New Revision: 199626<br><br>URL: <a href="http://llvm.org/viewvc/llvm-project?rev=199626&view=rev">http://llvm.org/viewvc/llvm-project?rev=199626&view=rev</a><br>Log:<br>Wire up basic parser/sema support for attribute 'returns_nonnull'.<br><br>This attribute is supported by GCC. More generally it should<br>probably be a type attribute, but this behavior matches 'nonnull'.<br><br>This patch does not include warning logic for checking if a null<br>value is returned from a function annotated with this attribute.<br>That will come in subsequent patches.<br><br>Modified:<br> cfe/trunk/include/clang/Basic/Attr.td<br> cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td<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=199626&r1=199625&r2=199626&view=diff">http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Attr.td?rev=199626&r1=199625&r2=199626&view=diff</a><br>==============================================================================<br>--- cfe/trunk/include/clang/Basic/Attr.td (original)<br>+++ cfe/trunk/include/clang/Basic/Attr.td Sun Jan 19 23:50:47 2014<br>@@ -693,6 +693,12 @@ def NonNull : InheritableAttr {<br> } }];<br>}<br><br>+def ReturnsNonNull : InheritableAttr {<br>+ let Spellings = [GNU<"returns_nonnull">];<br></blockquote><br>If the attribute is in gcc 4.8 or later, there should also be a C++11<br>spelling in the gnu namespace for it for consistency.<br></div></blockquote><div><br></div>Thanks!</div><div><br><blockquote type="cite"><div style="font-size: 14px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;"><br><blockquote type="cite">+ let Subjects = SubjectList<[ObjCMethod, FunctionLike, HasFunctionProto],<br>+ WarnDiag, "ExpectedFunctionOrMethod">;<br></blockquote><br>This pointed out a nasty bug which I need to find a more satisfactory<br>solution to: HasFunctionProto is a more strict version of<br>FunctionLike, so you should not specify them both as I was doing with<br>many other attributes. If you do, something which is function-like<br>will pass the feature test, and the has proto feature test will never<br>be run (because subjects are inclusive, not exclusive).<br></div></blockquote><div><br></div><div>Makes sense!</div><br><blockquote type="cite"><div style="font-size: 14px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;"><br><blockquote type="cite">+}<br>+<br>def NoReturn : InheritableAttr {<br> let Spellings = [GNU<"noreturn">, CXX11<"gnu", "noreturn">,<br> Declspec<"noreturn">];<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=199626&r1=199625&r2=199626&view=diff">http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=199626&r1=199625&r2=199626&view=diff</a><br>==============================================================================<br>--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)<br>+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Sun Jan 19 23:50:47 2014<br>@@ -1847,6 +1847,9 @@ def warn_attribute_pointers_only : Warni<br> "%0 attribute only applies to pointer arguments">,<br> InGroup<IgnoredAttributes>;<br>def err_attribute_pointers_only : Error<warn_attribute_pointers_only.Text>;<br>+def warn_attribute_return_pointers_only : Warning<<br>+ "%0 attribute only applies to return values that are pointers">,<br>+ InGroup<IgnoredAttributes>;<br>def err_attribute_no_member_pointers : Error<<br> "%0 attribute cannot be used with pointers to members">;<br>def err_attribute_invalid_implicit_this_argument : Error<<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=199626&r1=199625&r2=199626&view=diff">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclAttr.cpp?rev=199626&r1=199625&r2=199626&view=diff</a><br>==============================================================================<br>--- cfe/trunk/lib/Sema/SemaDeclAttr.cpp (original)<br>+++ cfe/trunk/lib/Sema/SemaDeclAttr.cpp Sun Jan 19 23:50:47 2014<br>@@ -1157,12 +1157,14 @@ static void possibleTransparentUnionPoin<br>}<br><br>static bool attrNonNullArgCheck(Sema &S, QualType T, const AttributeList &Attr,<br>- SourceRange R) {<br>+ SourceRange R, bool isReturnValue = false) {<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>+ S.Diag(Attr.getLoc(),<br>+ isReturnValue ? diag::warn_attribute_return_pointers_only<br>+ : diag::warn_attribute_pointers_only)<br> << Attr.getName() << R;<br> return false;<br> }<br>@@ -1231,6 +1233,23 @@ static void handleNonNullAttr(Sema &S, D<br> Attr.getAttributeSpellingListIndex()));<br>}<br><br>+static void handleReturnsNonNullAttr(Sema &S, Decl *D,<br>+ const AttributeList &Attr) {<br>+ QualType ResultType;<br>+ if (const FunctionType *Ty = D->getFunctionType())<br>+ ResultType = Ty->getResultType();<br>+ else if (const ObjCMethodDecl *MD = dyn_cast<ObjCMethodDecl>(D))<br>+ ResultType = MD->getResultType();<br></blockquote><br>We have getFunctionOrMethodResultType for doing this in SemaDeclAttr.cpp.<br></div></blockquote><div><br></div>I was looking for something, but somehow missed it. Thanks!</div><div><br><blockquote type="cite"><div style="font-size: 14px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;"><br><blockquote type="cite">+<br>+ if (!attrNonNullArgCheck(S, ResultType, Attr, Attr.getRange(),<br>+ /* isReturnValue */ true))<br>+ return;<br>+<br>+ D->addAttr(::new (S.Context)<br>+ ReturnsNonNullAttr(Attr.getRange(), S.Context,<br>+ Attr.getAttributeSpellingListIndex()));<br>+}<br>+<br>static const char *ownershipKindToDiagName(OwnershipAttr::OwnershipKind K) {<br> switch (K) {<br> case OwnershipAttr::Holds: return "'ownership_holds'";<br>@@ -3978,6 +3997,9 @@ static void ProcessDeclAttribute(Sema &S<br> else<br> handleNonNullAttr(S, D, Attr);<br> break;<br>+ case AttributeList::AT_ReturnsNonNull:<br>+ handleReturnsNonNullAttr(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=199626&r1=199625&r2=199626&view=diff">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/nonnull.c?rev=199626&r1=199625&r2=199626&view=diff</a><br>==============================================================================<br>--- cfe/trunk/test/Sema/nonnull.c (original)<br>+++ cfe/trunk/test/Sema/nonnull.c Sun Jan 19 23:50:47 2014<br>@@ -32,4 +32,10 @@ void test_baz() {<br> baz3(0); // no-warning<br>}<br><br>+void test_void_returns_nonnull() __attribute__((returns_nonnull)); // expected-warning {{'returns_nonnull' attribute only applies to return values that are pointers}}<br>+int test_int_returns_nonnull() __attribute__((returns_nonnull)); // expected-warning {{'returns_nonnull' attribute only applies to return values that are pointers}}<br>+void *test_ptr_returns_nonnull() __attribute__((returns_nonnull)); // no-warning<br></blockquote><br>None of these tests should pass the common feature check because none<br>of them have a prototype. However, the no-proto version is a good test<br>to have, so I added one.<br><br><blockquote type="cite">+<br>int i __attribute__((nonnull)); // expected-warning {{'nonnull' attribute only applies to functions, methods, and parameters}}<br>+int j __attribute__((returns_nonnull)); // expected-warning {{'returns_nonnull' attribute only applies to functions and methods}}<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=199626&r1=199625&r2=199626&view=diff">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjC/nonnull.m?rev=199626&r1=199625&r2=199626&view=diff</a><br>==============================================================================<br>--- cfe/trunk/test/SemaObjC/nonnull.m (original)<br>+++ cfe/trunk/test/SemaObjC/nonnull.m Sun Jan 19 23:50:47 2014<br>@@ -74,6 +74,9 @@ void func6(dispatch_object_t _head) {<br>@interface NSObject<br>- (void)doSomethingWithNonNullPointer:(void *)ptr :(int)iarg : (void*)ptr1 __attribute__((nonnull(1, 3)));<br>+ (void)doSomethingClassyWithNonNullPointer:(void *)ptr __attribute__((nonnull(1)));<br>+- (void*)returnsCNonNull __attribute__((returns_nonnull)); // no-warning<br>+- (id)returnsObjCNonNull __attribute__((returns_nonnull)); // no-warning<br>+- (int)returnsIntNonNull __attribute__((returns_nonnull)); // expected-warning {{'returns_nonnull' attribute only applies to return values that are pointers}}<br>@end<br><br>extern void DoSomethingNotNull(void *db) __attribute__((nonnull(1)));<br></blockquote><br>~Aaron</div></blockquote></div><br></div></body></html>