[PATCH] D57855: [analyzer] Reimplement checker options
Kristóf Umann via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Tue Mar 12 02:24:16 PDT 2019
Szelethus added inline comments.
Comment at: include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h:156
+ return FullName == Rhs.FullName;
> Are we able now to distinguish checkers of the same short names but in different packages? In earlier versions the short name had to be different to avoid conflicts.
Full names contain the packages as well, like `"alpha.cplusplus.IteratorModeling"`. We don't really care about short names (which refers to the checker's name, right?) in `CheckerRegistry`.
Comment at: include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h:282
+ /// \c Checkers field. For convenience purposes.
+ llvm::StringMap<size_t> PackageSizes;
> Cannot we store the size of the `Package` in the `Package` itself?
Yup, this is a mess, but I only renamed this field. I sat on this for a while, thinking about how `Packages` and `PackageSizes` could be merged, and figured that it just isn't worth the development time to refactor, and would just further bloat this patch.
Comment at: lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp:50
> Could we maybe use `std::less<CheckerOrPackageInfo>`?
Nope, `CheckerOrPackageInfo` doesn't implement `operator<`.
CHANGES SINCE LAST ACTION
More information about the cfe-commits