NoQ accepted this revision. NoQ added a comment. In D57230#1373721 <https://reviews.llvm.org/D57230#1373721>, @xazax.hun wrote:
> Do you think I should try to reduce additional files? Aha, ok, it reduced an interesting positive into a non-interesting positive. So i guess my method only works when you're catching changes that are more unexpected than this one :) Ok, nvm then, thank you for trying this out! I didn't have time to evaluate this change yet, but that definitely shouldn't block you from committing. In D57230#1373721 <https://reviews.llvm.org/D57230#1373721>, @xazax.hun wrote: > I think this the core idea is quite straightforward but this example is a bit > convoluted due to the recursion. Hint: You can almost always "unroll" recursions or loops in reduced tests by looking at the Exploded Graph and figuring out how many times were they actually executed before the bug was found and then copy-paste-ing the code that many times. ================ Comment at: lib/StaticAnalyzer/Core/CallEvent.cpp:320-321 + ETraits.setTrait( + UseBaseRegion ? MR->getBaseRegion() : MR, + RegionAndSymbolInvalidationTraits::TK_PreserveContents); + } ---------------- xazax.hun wrote: > NoQ wrote: > > I suspect that the trait for non-base `MR` would never be read. The only > > place where this trait is accessed is in RegionStore.cpp where it asks > > whether the trait is applied to a cluster base, which is always a base > > region. > I see some test failures when I always used the base region. I suspect the > reason is that `InvalidateRegionsWorker::AddToWorkList` will add the region > itself instead of the base region when `TK_DoNotInvalidateSuperRegion` is > set. So if we only set the `TK_PreserveContents` trait for the base region > `InvalidateRegionsWorker::VisitCluster` will not see the > `TK_PreserveContents` trait. > > In fact, the naming of regions in the those functions are very confusing. > Even though the formal paramter is called `baseR`, my suspicion is that, we > might visit non-base regions (due to the `TK_DoNotInvalidateSuperRegion` > trait). Aha, yeah, you're right! The word "Cluster" is also confusing because it is usually used for `ClusterBindings` :) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57230/new/ https://reviews.llvm.org/D57230 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
