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

Artem Dergachev via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Feb 3 09:47:28 PST 2020


NoQ added a comment.

This looks fantastic, i didn't think of this originally. All hail type safety!
Nits below.



================
Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:182
+
+    FunctionSummaryTy &Specification(ValueRangeSet VRS) {
+      Ranges.push_back(VRS);
----------------
Let's keep calling them "cases" because iirc this was a matter of discussion (https://reviews.llvm.org/D20811?id=71510#inline-211441).


================
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}, ...} },
----------------
I suspect that this comment would need a lot more updates.


================
Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:553
   //}
+  using Summary = FunctionSummaryTy;
+  auto ArgumentCondition = [](ArgNoTy ArgNo, ValueRangeKindTy Kind,
----------------
There seems to be a lot of inconsistency in the use of `T`, `Ty`, and lack-of-suffix.

I'd prefer to have one of them reserved for `QualType`s (eg., `IntTy`).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73897





More information about the cfe-commits mailing list