[PATCH] D104300: [analyzer] Handle std::swap for std::unique_ptr

[Valeriy Savchenko via Phabricator via cfe-commits]

vsavchenko added a comment.

I agree with @NoQ that notes are pretty much straightforward and we shouldn't abandon them altogether.  Refinements about what is null or non-null pointer are purely cosmetic and we definitely can tweak this messaging.



================
Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:196-202
+    const MemRegion *FirstArgThisRegion = Call.getArgSVal(0).getAsRegion();
+    if (!FirstArgThisRegion)
+      return false;
+    const MemRegion *SecondArgThisRegion = Call.getArgSVal(1).getAsRegion();
+    if (!SecondArgThisRegion)
+      return false;
+
----------------
I guess `handleSwap` can take `SVal`s instead of `MemRegion` and we can mostly cut on this boilerplate as well.
```
   return handleSwap(State, Call.getArgSVal(0), Call.getArgSVal(1), C);
```
and
```
  return handleSwap(State, IC->getCXXThisVal(), Call.getArgSVal(0), C);
```


================
Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:458-467
+    if (BR.isInteresting(FirstThisRegion) &&
+        !BR.isInteresting(SecondThisRegion)) {
+      BR.markInteresting(SecondThisRegion);
+      BR.markNotInteresting(FirstThisRegion);
+    }
+    if (BR.isInteresting(SecondThisRegion) &&
+        !BR.isInteresting(FirstThisRegion)) {
----------------
nit: these two pieces of code are very much the same


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104300