r178518 - [analyzer] Teach invalidateRegions that regions within LazyCompoundVal need to be invalidated

Anna Zaks ganna at apple.com
Tue Apr 2 10:11:37 PDT 2013


On Apr 1, 2013, at 6:44 PM, Jordan Rose <jordan_rose at apple.com> wrote:

> 
> On Apr 1, 2013, at 18:28 , Anna Zaks <ganna at apple.com> wrote:
> 
>> +        // Note: the last argumet is false here because these are
>> +        // non-top-level regions.
> 
> Typo: "agument".
> 
> 
> This is fine for now, but I hope we can kill the three sets of regions...they're already a bit hard to follow.
> 
> - top-level non-const regions (passed to checkRegionChanges and checkPointerEscape)
> - all non-const regions (passed to checkRegionChanges and checkPointerEscape)
> - top-level const regions (passed to checkPointerEscape's const variant)
> 
> - all const regions (we currently don't model this but we might some day)
> 
> I guess we save a little bit of effort by caching the top-level regions in a list instead of extracting them from the CallEvent again,

I did implement the other approach first - where we construct the top level sets in users when they need them instead of passing them around. The code looked much more complex and less maintainable. So I've reimplemented this to let RegionStore populate those regions. I like it much better. Here are the additional benefits to the one mentioned above:
 - This way the abstraction is right - the users rely on the info from RegionStore invalidation authority, instead of recalculating the top-level sets themselves.
 - Top level regions are not only call arguments, but should also include "extra invalidated values" (self and this), so their computation is complex.
 - If we ever add a new top level region, we don't need to change the logic in multiple places.

> but I'm not sure it's worth the additional complexity in either notifyCheckersOfPointerEscape or checkRegionChanges.

? With the current approach we added complexity only to RegionStore, instead of adding it to the users. notifyCheckersOfPointerEscape has to differentiate between the regions. Hopefully, checkRegionChanges can be completely removed after we remove the calls to it from RetainCountChecker; and migrate the checker to use pointer escape callback. After that is done, none of the checkers will have to reason about the regions. 


> 
> Jordan

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130402/338ed61a/attachment.html>


More information about the cfe-commits mailing list