[PATCH] D104616: [analyzer] Model comparision methods of std::unique_ptr

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added inline comments.


================
Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:431
+      State, BOK, FirstPtrVal, SecondPtrVal, Call.getResultType());
+  State = State->BindExpr(Call.getOriginExpr(), C.getLocationContext(), RetVal);
+
----------------
Now that we've made a state split, it makes sense to bind a concrete true value or a concrete false value (depending on the state) instead of a symbolic value. This produces simpler symbolic equations for our constraint solver to handle.

I.e., something like
```lang=c++
std::tie(TrueState, FalseState) = State->assume(...);
if (TrueState)
  C.addTransition(TrueState->BindExpr(E, LCtx, SVB.makeTruthVal(true));
if (FalseState)
  C.addTransition(FalseState->BindExpr(E, LCtx, SVB.makeTruthVal(false));
```


================
Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:435
+    ProgramStateRef TrueState, FalseState;
+    std::tie(TrueState, FalseState) =
+        State->assume(*RetVal.getAs<DefinedOrUnknownSVal>());
----------------
xazax.hun wrote:
> `assume` might not return a state (when the analyzer is smart enough to figure out one path is infeasible). While your code would probably work as is, as far as I remember we usually try to not call addTransition with `null` states. 
Yeah I have a lot of anxiety about passing nulls there as well, have to look up the source code of these functions every time :/


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104616