r199663 - Making some minor improvements to r199626.

[Hal Finkel]

----- Original Message -----
> From: "Aaron Ballman" <aaron at aaronballman.com>
> To: "Hal Finkel" <hfinkel at anl.gov>, "Ted Kremenek" <kremenek at apple.com>
> Cc: "llvm cfe" <cfe-commits at cs.uiuc.edu>
> Sent: Friday, July 11, 2014 11:05:30 AM
> Subject: Re: r199663 - Making some minor improvements to r199626.
> 
> On Fri, Jul 11, 2014 at 10:48 AM, Hal Finkel <hfinkel at anl.gov> wrote:
> > ----- Original Message -----
> >> From: "Aaron Ballman" <aaron at aaronballman.com>
> >> To: cfe-commits at cs.uiuc.edu
> >> Sent: Monday, January 20, 2014 8:19:44 AM
> >> Subject: r199663 - Making some minor improvements to r199626.
> >>
> >> Author: aaronballman
> >> Date: Mon Jan 20 08:19:44 2014
> >> New Revision: 199663
> >>
> >> URL: http://llvm.org/viewvc/llvm-project?rev=199663&view=rev
> >> Log:
> >> Making some minor improvements to r199626.
> >>
> >> Modified:
> >>     cfe/trunk/include/clang/Basic/Attr.td
> >>     cfe/trunk/lib/Sema/SemaDeclAttr.cpp
> >>     cfe/trunk/test/Sema/nonnull.c
> >>
> >
> > [snip]
> >
> >> Modified: cfe/trunk/test/Sema/nonnull.c
> >> URL:
> >> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/nonnull.c?rev=199663&r1=199662&r2=199663&view=diff
> >> ==============================================================================
> >> --- cfe/trunk/test/Sema/nonnull.c (original)
> >> +++ cfe/trunk/test/Sema/nonnull.c Mon Jan 20 08:19:44 2014
> >> @@ -32,10 +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
> >> +void test_void_returns_nonnull(void)
> >> __attribute__((returns_nonnull)); // expected-warning
> >> {{'returns_nonnull' attribute only applies to return values that
> >> are
> >> pointers}}
> >> +int test_int_returns_nonnull(void)
> >> __attribute__((returns_nonnull));
> >> // expected-warning {{'returns_nonnull' attribute only applies to
> >> return values that are pointers}}
> >> +void *test_ptr_returns_nonnull(void)
> >> __attribute__((returns_nonnull)); // no-warning
> >>
> >>  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}}
> >> -
> >> +void *test_no_fn_proto() __attribute__((returns_nonnull));  //
> >> expected-warning {{'returns_nonnull' attribute only applies to
> >> functions and methods}}
> >
> > This last check seems wrong to me. There is now a difference
> > between this:
> >
> > void *test_ptr_returns_nonnull(void)
> > __attribute__((returns_nonnull));
> >
> > which we handle and this:
> >
> > void *test_ptr_returns_nonnull() __attribute__((returns_nonnull));
> >
> > for which we ignore the attribute. However:
> >
> >  - Most people don't put void in the parameter lists, and
> 
> This is a C test file, where () is different than (void). The former
> has no prototype and is almost equivalent to it being written (...),
> while the latter has a prototype accepting no arguments. In C++ mode,
> there is no distinction.

I understand the distinction, but it is not relevant to an attribute on the return value.

> 
> >  - gcc 4.9.0 does not distinguish between the two forms w.r.t. the
> >  application of this attribute.
> >
> > Can you please fix this?
> 
> Ted was the original author of this functionality
> (http://lists.cs.uiuc.edu/pipermail/cfe-commits/Week-of-Mon-20140113/097562.html),
> this commit was a tidying of his original code but had no functional
> changes. Are you asking to have C code with no function prototype
> honor this attribute?

Yes, this is what gcc does. Also, you clearly did change the behavior in Ted's test case because you removed this:

-void *test_ptr_returns_nonnull() __attribute__((returns_nonnull)); // no-warning

and then added this:

+void *test_no_fn_proto() __attribute__((returns_nonnull));  // expected-warning {{'returns_nonnull' attribute only applies to functions and methods}}

Thanks again,
Hal

> If so, Ted, does that seem reasonable to you?
> 
> ~Aaron
> 

-- 
Hal Finkel
Assistant Computational Scientist
Leadership Computing Facility
Argonne National Laboratory