[PATCH] Combine the checks for returns_nonnull and for operator new returning null, in Sema::CheckReturnValExpr

[Jordan Rose]

This mostly looks good to me. One comment, though:

+          !RetValExp->isValueDependent() &&

The returns_nonnull check doesn't bother to check if it's value-dependent first. Is that necessary? Should that be folded in somewhere else?

Jordan


On Jan 23, 2014, at 2:24 , Artyom Skrobov <Artyom.Skrobov at arm.com> wrote:

> Jordan, good catch that this can be combined; attaching for a review.
> 
> 
> 
> -----Original Message-----
> From: Jordan Rose [mailto:jordan_rose at apple.com] 
> Sent: 22 January 2014 17:21
> To: Ted Kremenek
> Cc: llvm cfe; Richard Smith; Artyom Skrobov
> Subject: Re: r199790 - Add basic checking for returning null from
> functions/methods marked 'returns_nonnull'.
> 
> Should this be combined with the check that a throwing operator new never
> returns non-null? (r199452)
> 
> 
> On Jan 21, 2014, at 22:10 , Ted Kremenek <kremenek at apple.com> wrote:
> 
>> Author: kremenek
>> Date: Wed Jan 22 00:10:28 2014
>> New Revision: 199790
>> 
>> URL: http://llvm.org/viewvc/llvm-project?rev=199790&view=rev
>> Log:
>> Add basic checking for returning null from functions/methods marked
> 'returns_nonnull'.
>> 
>> This involved making CheckReturnStackAddr into a static function, which
>> is now called by a top-level return value checking routine called
>> CheckReturnValExpr.
>> 
>> Modified:
>>   cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
>>   cfe/trunk/include/clang/Sema/Sema.h
>>   cfe/trunk/lib/Sema/SemaChecking.cpp
>>   cfe/trunk/lib/Sema/SemaStmt.cpp
>>   cfe/trunk/test/Sema/nonnull.c
>>   cfe/trunk/test/SemaObjC/nonnull.m
-------------- next part --------------
A non-text attachment was scrubbed...
Name: combined-nonnull.patch
Type: application/octet-stream
Size: 3990 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140123/8dd76371/attachment.obj>
-------------- next part --------------