[PATCH] D57855: [analyzer] Reimplement checker options

[Kristóf Umann via Phabricator via cfe-commits]

Szelethus added inline comments.


================
Comment at: include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h:156
+      return FullName == Rhs.FullName;
+    }
+
----------------
baloghadamsoftware wrote:
> 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;
 
----------------
baloghadamsoftware wrote:
> 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
+  }
+};
+
----------------
baloghadamsoftware wrote:
> Could we maybe use `std::less<CheckerOrPackageInfo>`?
Nope, `CheckerOrPackageInfo` doesn't implement `operator<`.


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

https://reviews.llvm.org/D57855