[PATCH] D73897: [analyzer] StdLibraryFunctionsChecker refactor: remove macros

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.

Thanks for the review guys!



================
Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:537-551
+  // The format is as follows:
   //{ "function name",
-  //  { spec:
+  //  { variant0:
   //    { argument types list, ... },
-  //    return type, purity, { range set list:
+  //    return type, purity, { specification list:
   //      { range list:
   //        { argument index, within or out of, {{from, to}, ...} },
----------------
NoQ wrote:
> martong wrote:
> > NoQ wrote:
> > > I suspect that this comment would need a lot more updates.
> > Could you please elaborate? Do you mean to add comments e.g. to `ArgumentCondition` and the rest below? Or to rewrite the above comment?
> Actually let's ditch it entirely. It was worth it when it was all macros, so that it was apparent how macros expanded, but now it's pretty self-explanatory all the way.
> 
> Otherwise i was thinking about making this a pattern that the user can copy-paste and fill in. Like, maybe, include all the constructors explicitly (`Summary`, `ArgTypes`, etc.).
Ok, I ditched it.


================
Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:598
+          Summaries{
+              Summary(ArgTypes{IntTy}, RetType(IntTy), EvalCallAsPure)
+                  // Boils down to isupper() or islower() or isdigit().
----------------
NoQ wrote:
> Just curious, can `RetType` also use curly braces?
Yes. I've changed it to use the curly braces with `RetType` too, now the format is more consistent with `ArgTypes`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73897