balazske added inline comments.
================ Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:83 + } + // The universal integral type to use in value range descriptions. ---------------- This enum is moved here from other location, and `negateKind` is added. ================ Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:124-125 // requirement would render the initialization of the Summary map infeasible. + // A new value constraint is created furthermore by the negate functionality + // of the constraint and returned as pointer. using ValueConstraintPtr = std::shared_ptr<ValueConstraint>; ---------------- Szelethus wrote: > Is this what you meant? Probably this is better: ``` // Mind that a pointer to a new value constraint can be created by negating an existing // constraint. ``` The important thing is that one reason for the shared pointer is the negate function that returns a pointer. ================ Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:218 + /// Check if a single argument falls into a specific range. + /// The "range" can be built from sub-ranges that are closed intervals. class RangeConstraint : public ValueConstraint { ---------------- Szelethus wrote: > If you want a killer doc, use examples. Not saying this is not good enough, > but it wouldn't hurt. The "range" is not the best term for this object in the documentation. A "range" is often used when a single interval is meant, but here it often means a set of ranges. I plan to improve this. ================ Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:271 + + private: + using RangeApplyFunction = ---------------- This new private section is the new added code. ================ Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:275 + + /// @brief Apply a function on all intervals in the range. + void applyOnWithinRange( ---------------- The "@brief" is probably not necessary and not used often, I plan to remove it. ================ Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:893 -std::string StdLibraryFunctionsChecker::NotNullConstraint::describe( - DescriptionKind DK, const CallEvent &Call, ProgramStateRef State, - const Summary &Summary) const { - SmallString<48> Result; - const auto Violation = ValueConstraint::DescriptionKind::Violation; - Result += "the "; - Result += getArgDesc(ArgN); - Result += " to '"; - Result += getFunctionName(Call); - Result += DK == Violation ? "' should not be NULL" : "' is not NULL"; - return Result.c_str(); +void StdLibraryFunctionsChecker::RangeConstraint::applyOnWithinRange( + BasicValueFactory &BVF, QualType ArgT, const RangeApplyFunction &F) const { ---------------- This code is got from `RangeConstraint::applyAsOutOfRange`. This becomes "within" because in the apply as out of range case all ranges are cut out, that is really a loop over ("within") the ranges. ================ Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:910 +void StdLibraryFunctionsChecker::RangeConstraint::applyOnOutOfRange( + BasicValueFactory &BVF, QualType ArgT, const RangeApplyFunction &F) const { + if (Ranges.empty()) ---------------- This code is got from `RangeConstraint::applyWithinRange`. That was implemented by removal of all out-of-range intervals. The code here is a general version that calls a lambda instead of the actual assume calls, so it becomes reusable for other purposes when a loop over all (out-of) intervals is used. ================ Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:1060 + const Summary &Summary, + CheckerContext &C) const { + SValBuilder &SvalBuilder = C.getSValBuilder(); ---------------- This function is just moved here from the previous (inline) location. ================ Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:1143 + if (ExplodedNode *N = C.generateErrorNode(State, NewNode)) + reportBug(Call, N, Constraint.get(), NegatedConstraint.get(), Summary, C); break; ---------------- The new argument in `reportBug` is not used but is used in the next patch. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143751/new/ https://reviews.llvm.org/D143751 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits