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

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to