[cfe-dev] [PATCH] Clang Static Analyzer support for temporary destructors

[Alex McCarthy]

Maybe I misinterpreted your previous feedback: I also don't the "check one
ahead" approach in the patch.

Here's another version that uses a recursive solution like the original
patch, which I think handles this much more elegantly. It's an early
recurse and return so there shouldn't be much wasted extra work. What do
you think?

-Alex


On Tue, Apr 1, 2014 at 9:46 AM, Jordan Rose <jordan_rose at apple.com> wrote:

> I've committed all the cleanup patches, but I'm not sure the destructor
> patch is correct. What if there were two temporaries within the assignment?
> Wouldn't we see CFGImplicitDtors for both of them? (Maybe a loop can solve
> this problem, although it's kind of unfortunate.)
>
> Jordan
>
>
> On Apr 1, 2014, at 6:50 , Alex McCarthy <alexmc at google.com> wrote:
>
> And two more patches: the first is the refactor to split out
> a getRegionForConstructedObject from ExprEngineCXX.cpp: this can be applied
> to head.
>
> The second fixes spurious garbage value warnings when using temporary
> destructor analysis. It has to be applied after both the first refactoring
> attached to this mail
> (0001-Refactor-out-a-getRegionForConstructedObject-helper-.patch) and the
> previous patch to add new tests
> (0001-Prep-work-for-fixing-C-analysis-handling-of-temporar.patch).
>
> -Alex
>
>
> On Mon, Mar 31, 2014 at 8:29 PM, Alex McCarthy <alexmc at google.com> wrote:
>
>> And here's a second patch, to be applied after the first, that fixes the
>> crash while processing c++11 initializer lists. Thanks for explaining the
>> intent of makeZeroElementRegion: I'm not sure why the FIXME I removed was
>> there in the first place, but this seems like a much better fix than the
>> one I had before.
>>
>> -Alex
>>
>>
>> On Mon, Mar 31, 2014 at 8:03 PM, Alex McCarthy <alexmc at google.com> wrote:
>>
>>> Here's a new version of the simple patch. nullptr tests are gone, and
>>> I've uncommented tests that generate incorrect warnings. As discussed, I've
>>> left a few tests commented out because they trigger crashes or asserts: I
>>> think having the tests there but commented out serves as a cheap/poor form
>>> of documentation of the current state of these bugs in clang, and it should
>>> make upcoming patches that contain fixes smaller and easier to read. Let me
>>> know if you'd like me to remove those too.
>>>
>>> Since I don't have clang/llvm commit access, are you able to commit this
>>> on my behalf once this looks good to oyu?
>>>
>>> Thanks!
>>>
>>> -Alex
>>>
>>>
>>> On Mon, Mar 31, 2014 at 3:47 PM, Jordan Rose <jordan_rose at apple.com>wrote:
>>>
>>>>
>>>> On Mar 31, 2014, at 10:40, Alex McCarthy <alexmc at google.com> wrote:
>>>>
>>>> > Thanks for your advice, Jordan.
>>>> >
>>>> > I've split the NORETURN printing changes to CFG.cpp and the new test
>>>> additions into a separate patch (attached). Hopefully this should be a safe
>>>> no-op from a behavior perspective: debug output changes slightly, and the
>>>> new test cases more accurately reflects current clang behavior. Does this
>>>> look safe to commit?
>>>>
>>>> Rather than commenting out tests, I'd prefer you put them in with the
>>>> "wrong" expected-warnings, and then put a FIXME next to the warnings.
>>>> Commented-out tests become dead tests all too easily.
>>>>
>>>> I would drop the tests from nullptr.cpp. That file's for testing
>>>> nullptr and nullptr_t specifically, not null pointers in general, and the
>>>> new tests don't actually test any new behavior if we already believe that
>>>> nullptr properly converts to a null pointer value.
>>>>
>>>> Jordan
>>>>
>>>>
>>>
>>
> <0001-Refactor-out-a-getRegionForConstructedObject-helper-.patch>
> <0002-Fix-spurious-Undefined-or-garbage-value-returned-to-.patch>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20140401/530dd023/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-Fix-spurious-Undefined-or-garbage-value-returned-to-.patch
Type: application/octet-stream
Size: 2414 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20140401/530dd023/attachment.obj>