martong added a comment. I like the approach of this patch and I think this is somewhat aligned with @NoQ's ideas about
> a list of explicitly-live compound values and > "weak region roots" that aren't necessarily live themselves but anything > derived from them ... is live Coupled with the new tests for regression cases in D134941 <https://reviews.llvm.org/D134941>, I think this is really good. ================ Comment at: clang/include/clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h:586 + RegionSetTy LiveRegionRoots; + RegionSetTy LazilyCopiedRegionRoots; ---------------- Could you please incorporate the definition of //lazily copied locations (regions)// from the summary to here as a comment? ================ Comment at: clang/include/clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h:653 + bool isLazilyCopiedRegion(const MemRegion *region) const; + bool isReadableRegion(const MemRegion *region); + ---------------- Could you please incorporate the definition of //readable locations (regions)// from the summary to here as a comment? ================ Comment at: clang/lib/StaticAnalyzer/Core/RegionStore.cpp:2841-2842 V.getAs<nonloc::LazyCompoundVal>()) { + // TODO: Make regions referred to by `lazyCompoundVals` that are bound to + // subregions of the `LCS.getRegion()` also lazily copied. + if (const MemRegion *R = LCS->getRegion()) ---------------- Just a nit, I wonder if you might have a test case for this (which should fail for now). ================ Comment at: clang/lib/StaticAnalyzer/Core/SymbolManager.cpp:461 +bool SymbolReaper::isLazilyCopiedRegion(const MemRegion *MR) const { + // TODO: See comment in isLiveRegion. + return LazilyCopiedRegionRoots.count(MR->getBaseRegion()); ---------------- Just out of curiosity, do you have plans to tackle this todo sometime? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134947/new/ https://reviews.llvm.org/D134947 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
