[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:443-446
+  auto RetVal = C.getSValBuilder().evalBinOp(
+      State, BOK, FirstPtrVal, SecondPtrVal, Call.getResultType());
+  State = State->BindExpr(Call.getOriginExpr(), C.getLocationContext(), RetVal);
+  C.addTransition(State);
----------------
RedDocMD wrote:
> NoQ wrote:
> > Because these operators are pure boolean functions, a state split would be justified. It's pointless to call the operator unless both return values are feasible. I think you should do an `assume()` and transition into both states.
> > 
> > It might also make sense to consult `-analyzer-config eagerly-assume=` before doing it but it sounds like this option will stays true forever and we might as well remove it.
> > 
> > The spaceship operator is obviously an exceptional case. Invocation of the spaceship operator isn't a good indication that all three return values are feasible, might be only two.
> I think this comment has got displaced from its original location from subsequent updates. Could you please clarify?
You can always go back in time by clicking the `|<<` button in the top-left corner of the comment ;)

I'm trying to say that you should introduce a state split while evaluating the operators, not wait for the control flow operators to do that for you, exactly like the static analyzer currently does for the raw comparison operators.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104616