[PATCH] D84316: [analyzer][NFC] Split CStringChecker to modeling and reporting

[Balázs Benics via Phabricator via cfe-commits]

steakhal added a comment.

Thanks for checking this @balazske.



================
Comment at: clang/lib/StaticAnalyzer/Checkers/CStringChecker/CStringLength.h:43
+                                     ProgramStateRef &State, const Expr *Ex,
+                                     SVal Buf, bool Hypothetical = false);
+
----------------
balazske wrote:
> I do not like that the //get// and //set// (CStringLength) functions are not symmetrical. I (and other developers) would think that the get function returns a stored value and the set function sets it. The `getCStringLength` is more a `computeCStringLength` and additionally may manipulate the `State` too. In this form it is usable mostly only for CStringChecker. (A separate function to get the value stored in the length map should exist instead of this `Hypothetical` thing.)
> [...] get function returns a stored value and the set function sets it.
Certainly a burden to understand. It would be more appealing, but more useful?
The user would have to check and create if necessary regardless. So fusing these two functions is more like a feature.
What use case do you think of using only the query function? In other words, how can you guarantee that you will find a length for a symbol?

> In this form it is usable mostly only for CStringChecker. (A separate function to get the value stored in the length map should exist instead of this Hypothetical thing.)
You are right. However, I want to focus on splitting parts without modifying the already existing API reducing the risk of breaking things.
You should expect such a change in an upcoming patch.


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

https://reviews.llvm.org/D84316