aaron.ballman added inline comments.
Herald added a subscriber: jdoerfert.
================
Comment at: include/clang/StaticAnalyzer/Checkers/CheckerBase.td:13-14
+/// Describes a checker or package option type. This is important for
validating
+/// user supplied inputs.
+class CmdLineOptionTypeEnum<bits<2> val> {
----------------
Might want to add a comment mentioning that additional enumerators require
changes to the tablegen code.
================
Comment at: include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h:166
+
+ PackageInfo(StringRef FullName) : FullName(FullName) {}
+ };
----------------
Make this `explicit`?
================
Comment at: include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h:248-250
+ void printCheckerWithDescList(raw_ostream &out,
+ size_t maxNameChars = 30) const;
+ void printEnabledCheckerList(raw_ostream &out) const;
----------------
If we're changing these, can you fix up the parameter names for our coding
conventions?
================
Comment at: lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp:295
+
+ SameFullName(StringRef RequestedName) : RequestedName(RequestedName) {}
+
----------------
`explicit`?
================
Comment at: lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp:306
+
+void CheckerRegistry::addDependency(StringRef fullName, StringRef dependency) {
+ auto CheckerIt = llvm::find_if(Checkers, SameCheckerName(fullName));
----------------
Naming conventions.
================
Comment at: lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp:349
+
+ PackageIt->CmdLineOptions.push_back(
+ {OptionType, OptionName, DefaultValStr, Description});
----------------
`emplace_back()` instead?
================
Comment at: utils/TableGen/ClangSACheckersEmitter.cpp:95
+ if (BitsInit *BI = R.getValueAsBitsInit("Type")) {
+ switch(getValueFromBitsInit(BI, R)) {
+ case 0:
----------------
There should be come comments here marrying this up with the .td file.
Also, the formatting here looks somewhat off (the case statement seem to be
indented too far).
================
Comment at: utils/TableGen/ClangSACheckersEmitter.cpp:171
+ "#ifdef GET_PACKAGE_OPTIONS\n";
+ for (const Record *package : packages) {
+
----------------
Naming conventions.
================
Comment at: utils/TableGen/ClangSACheckersEmitter.cpp:253
+ "#ifdef GET_CHECKER_OPTIONS\n";
+ for (const Record *checker : checkers) {
+
----------------
Naming conventions.
================
Comment at: utils/TableGen/ClangSACheckersEmitter.cpp:258
+
+
+ std::vector<Record *> CheckerOptions = checker
----------------
Can remove some of the extra vertical whitespace.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D57855/new/
https://reviews.llvm.org/D57855
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
