On Apr 1, 2013, at 6:44 PM, Jordan Rose <[email protected]> wrote:

> 
> On Apr 1, 2013, at 18:28 , Anna Zaks <[email protected]> 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

_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to