vsavchenko added inline comments.
================ Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:97 +// If a region is removed all of the subregions needs to be removed too. +static ProgramStateRef removeTrackedRegions(ProgramStateRef State, ---------------- nit: *need to be removed ================ Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:98 +// If a region is removed all of the subregions needs to be removed too. +static ProgramStateRef removeTrackedRegions(ProgramStateRef State, + const MemRegion *Region) { ---------------- Maybe `Subregions` would fit better in this name then? ================ Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:186 + for (const auto *Region : Regions) + State = removeTrackedRegions(State, Region->getBaseRegion()); + return State; ---------------- It is not critical, but potentially we can allocate/deallocate a whole bunch of states here. We can do the same sort of operation with the map itself (`State->get<TrackedRegionMap>()`), which still have similar problem but to a lesser degree. Additionally, this `get<MAP_TYPE>` method is not compile-time, it searches for the corresponding map in runtime (also in a map), so you repeat that as many times as you have `Regions`. And super-duper over-optimization note on my part: making the loop over `Regions` to be the inner-most is more cache-friendly. It is not really critical here, but it is overall good to have an eye for things like that. ================ Comment at: clang/test/Analysis/smart-ptr.cpp:190 +/* +// TODO: Enable this test after '=' operator overloading modeling. +void derefAfterAssignment() { ---------------- Usually we simply add the test with expectations matching current state of things, but add a TODO over those particular lines. This way when you fix the issue the test will start failing and you won't forget to uncomment it. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83836/new/ https://reviews.llvm.org/D83836 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits