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

[Gabor Marton via Phabricator via cfe-commits]

martong added inline comments.


================
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
----------------
Szelethus wrote:
> If we put pure in quotes, we might as well go the extra mile and quickly explain what that means.
Ok, I removed the quotes and added the meaning of pure in parenthesis.


================
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
----------------
Szelethus wrote:
> 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.
Yes I agree, especially because I am planning to add a plethora of new functions in the future. I think that would be the appropriate time to implement the checker option.


================
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;
----------------
Szelethus wrote:
> But why? This isn't important for this patch, so feel free to leave as-is, but still.
I suppose this is because we cannot have a reference to the `ASTContext` in the constructor of the Checker. We can get that only via any of the checker callbacks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73897