[PATCH] [clang-tidy] Add support for boolean check options.

Manuel Klimek klimek at google.com
Mon Oct 6 01:43:30 PDT 2014


================
Comment at: clang-tidy/ClangTidy.h:50
@@ -49,3 +49,3 @@
   /// \p Default.
-  std::string get(StringRef LocalName, std::string Default) const;
+  std::string get(StringRef LocalName, const char *Default) const;
 
----------------
If there's a reason not to use StringRef instead of const char*, it should be in ALL CAPS, RED AND BOLD in the comment above ;)

================
Comment at: clang-tidy/ClangTidy.h:68-80
@@ -67,1 +67,15 @@
 
+  /// \brief Read a named option from the \c Context and parse it as bool.
+  ///
+  /// Reads the option with the check-local name \p LocalName from the
+  /// \c CheckOptions. If the corresponding key is not present, returns
+  /// \p Default.
+  bool get(StringRef LocalName, bool Default) const {
+    std::string Value = get(LocalName, "");
+    if (Value == "true")
+      return true;
+    if (Value == "false")
+      return false;
+    return Default;
+  }
+
----------------
Why do we want unknown values to give Default? If we do, we should put it into the comments; can we rather make them given an error while parsing, and then do:
return get(LocalName, Default ? "true" : "false") == "true";

================
Comment at: clang-tidy/ClangTidy.h:94-99
@@ +93,8 @@
+  /// an integral value \p Value to \p Options.
+  template <typename T>
+  typename std::enable_if<std::is_integral<T>::value, void>::type
+  store(ClangTidyOptions::OptionMap &Options, StringRef LocalName,
+        T Value) const {
+    store(Options, LocalName, static_cast<int64_t>(Value));
+  }
+
----------------
Will is_integral trigger for 128 bit integral values, and then narrow?

http://reviews.llvm.org/D5602






More information about the cfe-commits mailing list