steakhal added inline comments.
================ Comment at: clang/lib/StaticAnalyzer/Checkers/CStringChecker/CStringLength.h:43 + ProgramStateRef &State, const Expr *Ex, + SVal Buf, bool Hypothetical = false); + ---------------- steakhal wrote: > 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. On second thought, It probably worth having a cleaner API to a slight inconvenience. If he feels like, still can wrap them. I will investigate it tomorrow. 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
