Szelethus added a comment. A high level comment before getting into (even more) nitty gritty stuff. But shut me down if I misunderstood whats happening.
================ 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>; ---------------- balazske wrote: > 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. Can be, or //will// be? But otherwise, sure. ================ Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:271 + + private: + using RangeApplyFunction = ---------------- balazske wrote: > This new private section is the new added code. While generalizing code is great, whenever its done by introducing function arguments / lambdas, the code becomes harder to understand. This is fine, as long as this complexity is justified, but I really needed to see what happened in the followup patch to see whats the gain behind this. The gain is that you can capture a stream and construct a helpful message as the range is applied. Question: couldn't you just expose a lambda for the specific purpose for string smithing, and only that? Seems like all lambdas contain kind of the same thing: a call to `ConstraintManager::assumeInclusiveRange`. Maybe this design (for the above reasons) is worth documenting. 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