[PATCH] D19057: [analyzer] Let TK_PreserveContents span across the whole base region.

[Anna Zaks via cfe-commits]

zaks.anna accepted this revision.
zaks.anna added a comment.
This revision is now accepted and ready to land.

LGTM!

One thing to be aware here is that a const pointer could be deleted, so we should be able to delete a parent object without a warning. (I think that should work with this patch since you don't change the trait, but you might want to add a test.)

> What i don't like about the approach this patch implements, is that it makes the core rely on an 

>  implementation detail of RegionStoreManager ("only base regions are relevant" is such implementation 

>  detail). Instead, i also tried to add a few extra virtual methods into the StoreManager to avoid this 

>  problem, but it made the patch much heavier. I can post that, unless anybody else thinks that it's a 

>  natural thing (rather than implementation detail) to propagate this trait to base regions.


I'd commit this patch. If you want, feel free to follow up with another to remove the leaking of the implementation detail. I do not fully understand what the alternate solution is... I do not think it would make sense to automatically apply the TK_PreserveContents trait to the base region when one calls ETraits.setTrait(); not sure if you are suggesting that.


================
Comment at: lib/StaticAnalyzer/Core/CallEvent.cpp:182
@@ -181,3 +181,3 @@
                         RegionAndSymbolInvalidationTraits::TK_PreserveContents);
         // TODO: Factor this out + handle the lower level const pointers.
 
----------------
Not sure, but this could mean to deal with cases where you pass a non const pointer to a class that has const fields..


http://reviews.llvm.org/D19057