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

Kristóf Umann via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Feb 7 04:16:31 PST 2020


Szelethus accepted this revision.
Szelethus added a comment.
This revision is now accepted and ready to land.

This is amazing. LGTM, granted that @NoQ is happy with the patch as well.



================
Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:33
 //
 // This checker uses eval::Call for modeling "pure" functions, for which
 // their `FunctionSummaryTy' is a precise model. This avoids unnecessary
----------------
If we put pure in quotes, we might as well go the extra mile and quickly explain what that means.


================
Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:42-49
 // The following standard C functions are currently supported:
 //
 //   fgetc      getline   isdigit   isupper
 //   fread      isalnum   isgraph   isxdigit
 //   fwrite     isalpha   islower   read
 //   getc       isascii   isprint   write
 //   getchar    isblank   ispunct
----------------
I would prefer to just have a checker option that could print out the currently modeled function rather than these lines of a recipe for outdated comments.


================
Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:214-215
 
   // The map of all functions supported by the checker. It is initialized
   // lazily, and it doesn't change after initialization.
+  mutable llvm::StringMap<Summaries> FunctionSummaryMap;
----------------
But why? This isn't important for this patch, so feel free to leave as-is, but still.


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