[PATCH] [Analyzer] Individual options for checkers #2

[Anna Zaks]

Could you include more context with the patch? See: http://llvm.org/docs/Phabricator.html

Looks like we don't have a public API for options with string values.

Thanks!
Anna.


================
Comment at: include/clang/StaticAnalyzer/Core/AnalyzerOptions.h:275
@@ +274,3 @@
+  /// ones.
+  ///// @retval CheckerOptionValue  An option for a checker if it was specified
+  /// @retval GroupOptionValue  An option for group if it was specified and no
----------------
Extra "//".
Nit: Please, make sure all comments have proper punctuation. (A period is missing here and in a few other places.)

================
Comment at: include/clang/StaticAnalyzer/Core/AnalyzerOptions.h:288
@@ -257,3 +287,3 @@
   ///
   /// Accepts the strings "true" and "false".
   /// If an option value is not provided, returns the given \p DefaultVal.
----------------
This is a bit confusing.

================
Comment at: include/clang/StaticAnalyzer/Core/AnalyzerOptions.h:311
@@ +310,3 @@
+  /// @param [in] C Optional parameter that may be used to retrieve
+  /// checker-related option for a given checker
+  /// @param [in] SearchInParents If set to true and the searched option was not
----------------
This might be more clear: "The optional checker parameter that can be used to restrict the search to the options of this particular checker."

================
Comment at: lib/Frontend/CompilerInvocation.cpp:140
@@ -134,1 +139,3 @@
+}
+
 static bool ParseAnalyzerArgs(AnalyzerOptions &Opts, ArgList &Args,
----------------
Shouldn't we use an existing clang check to check if substrings are identifiers?


================
Comment at: lib/StaticAnalyzer/Core/AnalyzerOptions.cpp:110
@@ +109,3 @@
+  // Search for a category option if option for checker is not specified and
+  // inheritance is enabled.
+  ConfigTable::const_iterator E = Config.end();
----------------
The comment needs to move and be rephrased (there is no inheritance and checkers are organized in packages, not categories).


================
Comment at: unittests/Analysis/AnalyzerOptionsTest.cpp:47
@@ +46,2 @@
+} // end namespace ast_matchers
+} // end namespace clang
----------------
I think we need more tests. Ex: overridden options, options set only in a package.

================
Comment at: unittests/CMakeLists.txt:16
@@ -15,2 +15,3 @@
 if(CLANG_ENABLE_STATIC_ANALYZER)
+  add_subdirectory(Analysis)
   add_subdirectory(Frontend)
----------------
"Analysis" is too vague if we only plan to use this for the clang static analyzer.

http://reviews.llvm.org/D7905

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/