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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
