vlad.tsyrklevich added inline comments.
================ Comment at: lib/StaticAnalyzer/Core/RegionStore.cpp:502 + RegionBindingsRef B = getRegionBindings(S); + const MemRegion *MR = L.getRegion()->getBaseRegion(); + if (Optional<SVal> V = B.getDefaultBinding(MR)) ---------------- vlad.tsyrklevich wrote: > vlad.tsyrklevich wrote: > > a.sidorin wrote: > > > We get the LazyCompoundVal for some region but return the symbol for its > > > base. It means that at least method name is very confusing. > > I believe that default bindings are only on base regions, so if you pass a > > reference to `outer_struct.inner_struct` the default binding for that LCV > > will be over `outer_struct`. I'm basing this on other references to LCVs in > > Core/RegionStore.cpp but I could be wrong. Either way, I'd be happy to > > change the interface to have the caller pass the correct MemRegion here. > One change we could introduce to make getDefaultBinding() more explicit is to > have the caller pass LCV.getRegion()->getBaseRegion() instead of just passing > the LCV. However, this has the disadvantage of hardcoding an implementation > detail of RegionStoreManager into users of the StoreManager interface. I > think the correct way forward here might just be to add a comment that > mentions that currently RegionStoreManagers bindings are only over base > regions. What do you think? I spoke too soon, after digging a little deeper I realized that lookups using `RegionBindingsRef ::getDefaultBinding` converts to a base region check anyway in `BindingKey::Make`. Therefore it'd be better to just completely hide that implementation detail to the RegionBindings than hard coding it here unnecessarily. The updated revision corrects this. https://reviews.llvm.org/D28445 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
