r199626 - Wire up basic parser/sema support for attribute 'returns_nonnull'.

Ted Kremenek kremenek at apple.com
Mon Jan 20 22:19:40 PST 2014


Thank you Aaron!

On Jan 20, 2014, at 6:25 AM, Aaron Ballman <aaron at aaronballman.com> 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 <kremenek at apple.com> 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

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140120/008942d1/attachment.html>


More information about the cfe-commits mailing list