baloghadamsoftware added inline comments.
================ Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:814 +Optional<ProgramStateRef> RangeConstraintManager::tryAssumeSymSymOp( + ProgramStateRef State, BinaryOperator::Opcode Op, SymbolRef LHSSym, ---------------- NoQ wrote: > I believe you don't need to return an optional here. The method simply > acknowledges any assumptions it could make in the existing state and returns > the updated state. Therefore, if it wasn't able to record any assumptions, it > returns the existing state. Because the only reasonable behavior the caller > could implement when the current implementation returns `None` is to roll > back to the existing state anyway. Actually, `tryAssumeSymSymOp()` does not assume anything and therefore it does not return a new state. What it actually returns is a ternary logical value: the assumption is either valid, invalid or we cannot reason about it. Maybe Optional<bool> would be better here and we should also chose a less misleading name, because it does not "try to assume" anything, but tries to reason about the existing assumption. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77792/new/ https://reviews.llvm.org/D77792 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
