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

[Aaron Ballman]

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.

> +  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).

> +}
> +
>  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.

> +
> +  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