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
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
