[PATCH] D82585: [analyzer][NFC] Move the data structures from CheckerRegistry to the Core library

Endre Fülöp via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Jun 30 03:14:12 PDT 2020


gamesh411 added a comment.

Good job, massive props to you for such an overhaul.



================
Comment at: clang/include/clang/StaticAnalyzer/Core/CheckerManager.h:155
   /// Constructs a CheckerManager without requiring an AST. No checker
-  /// registration will take place. Only useful for retrieving the
-  /// CheckerRegistry and print for help flags where the AST is unavalaible.
+  /// registration will take placer. Only useful when one needs to print the
+  /// help flags through CheckerRegistryData, and the AST is unavalaible.
----------------
placer -> place


================
Comment at: clang/include/clang/StaticAnalyzer/Core/CheckerRegistryData.h:16
+// this information, such as enforcing rules on checker dependency bug emission,
+// ensuring all checker options were querried, etc.
+//
----------------
querried -> queried


================
Comment at: clang/include/clang/StaticAnalyzer/Core/CheckerRegistryData.h:83
+
+using CmdLineOptionList = llvm::SmallVector<CmdLineOption, 0>;
+
----------------
Could you please explain why use zero for the small-vector element count? My first thought would be that literally anything other than 0 would be more beneficial because of non-dynamic storage, even if we have use a totally ad-hoc value here (like 8 or 42 or whatever). Why not std::vector if we do not want static storage? Maybe there is something that I'm just not aware of :D


================
Comment at: clang/lib/StaticAnalyzer/Core/CheckerRegistryData.cpp:179
+
+  // Its usually ill-advised to use multimap, but clang will terminate after
+  // this function.
----------------
Its -> It's


================
Comment at: clang/lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp:472
 
-  using CmdLineOption = CheckerRegistry::CmdLineOption;
+  using CmdLineOption = CmdLineOption;
 
----------------
A bit confused here. It is not immediately clear for me from which namespace are we importing CmdLineOption. Is it possible to use a fully qualified type name on the RHS of the using statement?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82585





More information about the cfe-commits mailing list