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. ~Aaron _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
