Re: [PATCH] D18412: [clang-tidy] Add support for different char-types for the readability-redundant-string-cstr checker.

[Etienne Bergeron via cfe-commits]

etienneb updated this revision to Diff 51559.
etienneb added a comment.

remove dead-code


http://reviews.llvm.org/D18412

Files:
  clang-tidy/readability/RedundantStringCStrCheck.cpp
  test/clang-tidy/readability-redundant-string-cstr.cpp


Index: test/clang-tidy/readability-redundant-string-cstr.cpp
===================================================================
--- test/clang-tidy/readability-redundant-string-cstr.cpp
+++ test/clang-tidy/readability-redundant-string-cstr.cpp
@@ -12,14 +12,20 @@
   const C *c_str() const;
 };
 typedef basic_string<char, std::char_traits<char>, std::allocator<char>> string;
+typedef basic_string<wchar_t, std::char_traits<wchar_t>, std::allocator<wchar_t>> wstring;
+typedef basic_string<char16_t, std::char_traits<char16_t>, std::allocator<char16_t>> u16string;
+typedef basic_string<char32_t, std::char_traits<char32_t>, std::allocator<char32_t>> u32string;
 }
+
 namespace llvm {
 struct StringRef {
   StringRef(const char *p);
   StringRef(const std::string &);
 };
 }
 
+// Tests for std::string.
+
 void f1(const std::string &s) {
   f1(s.c_str());
   // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: redundant call to `c_str()` [readability-redundant-string-cstr]
@@ -39,3 +45,47 @@
   // CHECK-FIXES: {{^  }}std::string s;{{$}}
   // CHECK-FIXES-NEXT: {{^  }}f3(s);{{$}}
 }
+void f4(const std::string &s) {
+  const std::string* ptr = &s;
+  f1(ptr->c_str());
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: redundant call to `c_str()` [readability-redundant-string-cstr]
+  // CHECK-FIXES: {{^  }}f1(*ptr);{{$}}
+}
+
+// Tests for std::wstring.
+
+void g1(const std::wstring &s) {
+  g1(s.c_str());
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: redundant call to `c_str()` [readability-redundant-string-cstr]
+  // CHECK-FIXES: {{^  }}g1(s);{{$}}
+}
+
+// Tests for std::u16string.
+
+void h1(const std::u16string &s) {
+  h1(s.c_str());
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: redundant call to `c_str()` [readability-redundant-string-cstr]
+  // CHECK-FIXES: {{^  }}h1(s);{{$}}
+}
+
+// Tests for std::u32string.
+
+void k1(const std::u32string &s) {
+  k1(s.c_str());
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: redundant call to `c_str()` [readability-redundant-string-cstr]
+  // CHECK-FIXES: {{^  }}k1(s);{{$}}
+}
+
+// Tests on similar classes that aren't good candidates for this checker.
+
+struct NotAString {
+  NotAString();
+  NotAString(const NotAString&);
+  const char *c_str() const;
+};
+
+void dummy(const char*) {}
+
+void invalid(const NotAString &s) {
+  dummy(s.c_str());
+}
\ No newline at end of file
Index: clang-tidy/readability/RedundantStringCStrCheck.cpp
===================================================================
--- clang-tidy/readability/RedundantStringCStrCheck.cpp
+++ clang-tidy/readability/RedundantStringCStrCheck.cpp
@@ -65,14 +65,6 @@
   return (llvm::Twine("*") + Text).str();
 }
 
-const char StringConstructor[] =
-    "::std::basic_string<char, std::char_traits<char>, std::allocator<char> >"
-    "::basic_string";
-
-const char StringCStrMethod[] =
-    "::std::basic_string<char, std::char_traits<char>, std::allocator<char> >"
-    "::c_str";
-
 } // end namespace
 
 namespace tidy {
@@ -85,23 +77,31 @@
   if (!getLangOpts().CPlusPlus)
     return;
 
+  // Match expressions of type 'string' or 'string*'.
+  const auto StringDecl =
+      cxxRecordDecl(hasName("::std::basic_string"));
+  const auto StringExpr =
+      expr(anyOf(hasType(StringDecl),
+                 hasType(qualType(pointsTo(StringDecl)))));
+
   // Match string constructor.
   const auto StringConstructorExpr = expr(anyOf(
       cxxConstructExpr(
           argumentCountIs(1),
-          hasDeclaration(cxxMethodDecl(hasName(StringConstructor)))),
+          hasDeclaration(cxxMethodDecl(hasName("basic_string")))),
       cxxConstructExpr(
           argumentCountIs(2),
-          hasDeclaration(cxxMethodDecl(hasName(StringConstructor))),
+          hasDeclaration(cxxMethodDecl(hasName("basic_string"))),
           // If present, the second argument is the alloc object which must not
           // be present explicitly.
           hasArgument(1, cxxDefaultArgExpr()))));
 
   // Match a call to the string 'c_str()' method.
-  const auto StringCStrCallExpr = cxxMemberCallExpr(
-                            callee(memberExpr().bind("member")),
-                            callee(cxxMethodDecl(hasName(StringCStrMethod))),
-                            on(expr().bind("arg"))).bind("call");
+  const auto StringCStrCallExpr =
+      cxxMemberCallExpr(on(StringExpr.bind("arg")),
+                        callee(memberExpr().bind("member")),
+                        callee(cxxMethodDecl(matchesName(StringCStrMethod))))
+          .bind("call");
 
   Finder->addMatcher(
       cxxConstructExpr(StringConstructorExpr,

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits