vrnithinkumar marked 2 inline comments as done.
vrnithinkumar 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,
----------------
vsavchenko wrote:
> nit: *need to be removed
fixed
================
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) {
----------------
vsavchenko wrote:
> Maybe `Subregions` would fit better in this name then?
Changed to Subregions
================
Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:186
+ for (const auto *Region : Regions)
+ State = removeTrackedRegions(State, Region->getBaseRegion());
+ return State;
----------------
vsavchenko wrote:
> 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.
Updated `removeTrackedSubregions` for passing `TrackedRegionMap`
================
Comment at: clang/test/Analysis/Inputs/system-header-simulator-cxx.h:962
+ operator bool() const;
+ unique_ptr<T> &operator=(unique_ptr<T> &&p);
+};
----------------
xazax.hun wrote:
> vrnithinkumar wrote:
> > added this to support use case like `Q = std::move(P)`
> This operation should be `noexcept`:
> https://en.cppreference.com/w/cpp/memory/unique_ptr/operator%3D
>
> While it makes little difference for the analyzer at this point we should try
> to be as close to the standard as possible. If you have some time feel free
> to add `noexcept` to other methods that miss it :)
Added `noexcept` for all applicable methods
================
Comment at: clang/test/Analysis/smart-ptr.cpp:190
+/*
+// TODO: Enable this test after '=' operator overloading modeling.
+void derefAfterAssignment() {
----------------
vsavchenko wrote:
> 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.
Enabled the test and added the todo.
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
