vsavchenko added a comment. Looks like a solid start!
================ Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:301 + + bool addTransition = false; + ProgramStateRef State = C.getState(); ---------------- nit: variable names should be capitalized ================ Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:321-323 + State = + State->assume(llvm::dyn_cast<DefinedOrUnknownSVal>(RetVal), true); + addTransition = true; ---------------- Another idea here. Instead of repeating: ``` State = State->assume(llvm::dyn_cast<DefinedOrUnknownSVal>(RetVal), VALUE); addTransition = true; ``` and having boolean `addTransition`, you can have `Optional<bool> CompResult` and do: ``` CompResult = VALUE; ``` And do the actual assumption at the bottom section when `CompResult.hasValue()` ================ Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:334 + default: + assert(false && "shouldn't reach here"); + } ---------------- nit: `llvm_unreachable` instead of `assert(false)` ================ Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:345-364 + if (FirstPtr && !SecondPtr && + State->assume(llvm::dyn_cast<DefinedOrUnknownSVal>(*FirstPtr), + false)) { + // FirstPtr is null, SecondPtr is unknown + if (OOK == OO_LessEqual) { + State = + State->assume(llvm::dyn_cast<DefinedOrUnknownSVal>(RetVal), true); ---------------- These are also symmetrical. ================ Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:373-391 + switch (OOK) { + case OO_Equal: + case OO_GreaterEqual: + case OO_LessEqual: + State = State->assume(llvm::dyn_cast<DefinedOrUnknownSVal>(RetVal), + true); + addTransition = true; ---------------- This switch exactly repeats the previous switch. ================ Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:394-436 + if (FirstNull && !SecondNull) { + switch (OOK) { + case OO_Less: + case OO_LessEqual: + State = State->assume(llvm::dyn_cast<DefinedOrUnknownSVal>(RetVal), + true); + addTransition = true; ---------------- These are symmetrical, is there a way to implement it without this boilerplate? 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