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