[PATCH] D83877: [Analyzer] Changed in SmartPtrModeling to handle Swap

[Valeriy Savchenko via Phabricator via cfe-commits]

vsavchenko added inline comments.


================
Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:198
 
 void SmartPtrModeling::handleSwap(const CallEvent &Call,
                                   CheckerContext &C) const {
----------------
I think it would be good to add some comments in the body of the function to be more explicit about the situation you are handling.


================
Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:224-228
+  if (ThisRegionInnerPointerVal) {
+    State = State->set<TrackedRegionMap>(ArgRegion, *ThisRegionInnerPointerVal);
+  } else {
+    State = State->remove<TrackedRegionMap>(ArgRegion);
+  }
----------------
These two if's are clearly duplicates and it seems like a good candidate for a separate function.


================
Comment at: clang/test/Analysis/smart-ptr.cpp:124
+
+void derefOnStdSwappedNullPtr() {
+  std::unique_ptr<A> P(new A());
----------------
xazax.hun wrote:
> This test case is very similar to the first one. Maybe you could move them closer. And you could make both pointers invalid to make them more distinct.
+1


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D83877/new/

https://reviews.llvm.org/D83877