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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
