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