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