----- Original Message ----- > From: "Aaron Ballman" <[email protected]> > To: "Hal Finkel" <[email protected]> > Cc: "llvm cfe" <[email protected]>, "Ted Kremenek" <[email protected]> > Sent: Friday, July 11, 2014 11:42:10 AM > Subject: Re: r199663 - Making some minor improvements to r199626. > > On Fri, Jul 11, 2014 at 12:29 PM, Aaron Ballman > <[email protected]> wrote: > > On Fri, Jul 11, 2014 at 12:13 PM, Hal Finkel <[email protected]> > > wrote: > >> ----- Original Message ----- > >>> From: "Aaron Ballman" <[email protected]> > >>> To: "Hal Finkel" <[email protected]>, "Ted Kremenek" > >>> <[email protected]> > >>> Cc: "llvm cfe" <[email protected]> > >>> 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 <[email protected]> > >>> wrote: > >>> > ----- Original Message ----- > >>> >> From: "Aaron Ballman" <[email protected]> > >>> >> To: [email protected] > >>> >> 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. > > > > Agreed. > > > >> > >>> > >>> > - 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}} > > > > Derp! I misspoke, sorry about that. > > > > Ted's original declaration of the attribute set a subject > > requirement > > of "HasFunctionProto", which is I was asking for Ted's opinion in > > case > > there was logic there I wasn't following. That being said, I don't > > see > > why this would require a prototype (and gcc compat makes sense > > too). > > I'll look into changing this, unless Ted comes back with something. > > "Looking into" turns out to yield r212827. I am pretty sure this is > explained by some likely copy/paste problems. The > getFunctionOrMethodResultType was relying on a FunctionProtoType > being > available for the function declaration, but one isn't required to get > the return type. This in turn meant that the returns_nonnull > attribute > declaration required HasFunctionProto. Fixing these two up resolves > the issue.
Great, thanks! I really appreciate the prompt fix. -Hal > > ~Aaron > -- Hal Finkel Assistant Computational Scientist Leadership Computing Facility Argonne National Laboratory _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
