[PATCH] D143751: [clang][analyzer][NFC] Refactor code of StdLibraryFunctionsChecker.

[Balázs Kéri via Phabricator via cfe-commits]

balazske added inline comments.


================
Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:271
+
+  private:
+    using RangeApplyFunction =
----------------
Szelethus wrote:
> 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.
It looks not easy to change the design, the lambda is not only used for string construction and the way the state is applied (only check the state or change it too) is different. Probably it is possible but with more additional special purpose function parameters that add again complexity. Otherwise the current way looks not difficult, just a function that is called on all ranges or out-of-ranges, like an iteration. It can be possible to make a function that instead of calling lambda returns a list of ranges for the within-range or out-of-range case, and then make an iteration over these lists, but this is really only more code, and the returned lists are used only for these iterations.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D143751/new/

https://reviews.llvm.org/D143751