[PATCH] D18136: boost-use-to-string check

[Richard via cfe-commits]

LegalizeAdulthood added a subscriber: LegalizeAdulthood.

================
Comment at: clang-tidy/boost/BoostTidyModule.cpp:21
@@ -19,3 +20,3 @@
 
 class ModernizeModule : public ClangTidyModule {
 public:
----------------
This should be `BoostModule`, not `ModernizeModule`.

================
Comment at: clang-tidy/boost/UseToStringCheck.cpp:24
@@ +23,3 @@
+  // returning type of className.
+  auto getMatcher = [](std::string className) {
+    return callExpr(
----------------
`getMatcher` doesn't reveal the intent here; the name should reveal what is matched without me having to reverse engineer that information from it's defintiion.

================
Comment at: clang-tidy/boost/UseToStringCheck.cpp:33
@@ +32,3 @@
+  Finder->addMatcher(
+      getMatcher("class std::__cxx11::basic_string<char>").bind("to_string"),
+      this);
----------------
Why do we need to match something that is implementation specific?  This seems really fragile.

================
Comment at: clang-tidy/boost/UseToStringCheck.cpp:40
@@ +39,3 @@
+  Finder->addMatcher(
+      getMatcher("class std::__cxx11::basic_string<char>").bind("to_wstring"),
+      this);
----------------
Ditto

================
Comment at: clang-tidy/boost/UseToStringCheck.cpp:55-58
@@ +54,6 @@
+
+static const std::string toStringMSG =
+    "use std::to_string instead of boost::lexical_cast<std::string>";
+static const std::string toWStringMSG =
+    "use std::to_wstring instead of boost::lexical_cast<std::wstring>";
+
----------------
Why do we need variables for these strings?  They are used only once and the introduced variable name doesn't reveal more intent.

================
Comment at: docs/clang-tidy/checks/list.rst:90
@@ -88,2 +89,3 @@
    modernize-use-override
+   modernize-use-using
    performance-faster-string-find
----------------
Why was this added?  It seems unrelated to this changeset.

================
Comment at: test/clang-tidy/boost-use-to-string.cpp:4-10
@@ +3,9 @@
+
+namespace std {
+
+template <typename T> class basic_string {};
+
+using string = basic_string<char>;
+using wstring = basic_string<wchar_t>;
+}
+
----------------
Isn't there an existing stub header file providing std::{w,}string?

================
Comment at: test/clang-tidy/boost-use-to-string.cpp:76-81
@@ +75,8 @@
+// CHECK-FIXES: fun(std::to_string(f));
+  fun(boost::lexical_cast<std::string>(g));
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use std::to_string instead of boost::lexical_cast<std::string> [boost-use-to-string]
+// CHECK-FIXES: fun(std::to_string(g));
+  fun(boost::lexical_cast<std::string>(h));
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use std::to_string instead of boost::lexical_cast<std::string> [boost-use-to-string]
+// CHECK-FIXES: fun(std::to_string(h));
+  fun(boost::lexical_cast<std::string>(i));
----------------
Do we know that the semantics are the same for floating-point types?


http://reviews.llvm.org/D18136