[PATCH] D86027: [analyzer] Add bool operator modeling for unque_ptr

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun added inline comments.


================
Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:147
 
-    if (!move::isMovedFrom(State, ThisR)) {
-      // TODO: Model this case as well. At least, avoid invalidation of
-      // globals.
-      return false;
+    if (ModelSmartPtrDereference) {
+      handleBoolOperation(Call, C);
----------------
vrnithinkumar wrote:
> This seems little messy here.
> I guess once we model the `std::move` for smart ptr it will be less messy 
I agree. `isNullAfterMoveMethod` is especially confusing as it does not do what the name of the function says. It checks if the `Call` is a boolean conversion operator. I'd suggest renaming that method to make this code a bit more sensible. 

Moreover, when `ModelSmartPtrDereference` is true, we no longer model moves below right? I think a comment that this logic is duplicated `handleBoolOperation` might make the code cleaner. 

But yeah, this needs to be cleaned up, hopefully really soon.


================
Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:389
+    }
+    C.addTransition(State);
+  } else if (move::isMovedFrom(State, ThisRegion)) {
----------------
This looks fine for now, but we often prefer adding a return after each case to avoid executing multiple `addTransition`s accidentally after refactoring.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86027