Index: clang-tidy/readability/RedundantStringCStrCheck.cpp =================================================================== --- clang-tidy/readability/RedundantStringCStrCheck.cpp +++ clang-tidy/readability/RedundantStringCStrCheck.cpp @@ -103,20 +103,83 @@ callee(cxxMethodDecl(hasName("c_str")))) .bind("call"); + // Detect redundant 'c_str()' calls through a string constructor. Finder->addMatcher( cxxConstructExpr(StringConstructorExpr, hasArgument(0, StringCStrCallExpr)), this); + // Detect: 's == str.c_str()' -> 's == str' Finder->addMatcher( + cxxOperatorCallExpr( + anyOf(hasOverloadedOperatorName("<"), + hasOverloadedOperatorName(">"), + hasOverloadedOperatorName(">="), + hasOverloadedOperatorName("<="), + hasOverloadedOperatorName("!="), + hasOverloadedOperatorName("=="), + hasOverloadedOperatorName("+")), + anyOf(allOf(hasArgument(0, StringExpr), + hasArgument(1, StringCStrCallExpr)), + allOf(hasArgument(0, StringCStrCallExpr), + hasArgument(1, StringExpr)))), + this); + + // Detect: 'dst += str.c_str()' -> 'dst += str' + // Detect: 's == str.c_str()' -> 's == str' + Finder->addMatcher( + cxxOperatorCallExpr( + anyOf(hasOverloadedOperatorName("="), + hasOverloadedOperatorName("+=")), + hasArgument(0, StringExpr), + hasArgument(1, StringCStrCallExpr)), + this); + + // Detect: 'dst.append(str.c_str())' -> 'dst.append(str)' + Finder->addMatcher( + cxxMemberCallExpr(on(StringExpr), + callee(decl(cxxMethodDecl( + hasAnyName("append", "assign", "compare")))), + argumentCountIs(1), + hasArgument(0, StringCStrCallExpr)), + this); + + // Detect: 'dst.compare(p, n, str.c_str())' -> 'dst.compare(p, n, str)' + Finder->addMatcher( + cxxMemberCallExpr(on(StringExpr), + callee(decl(cxxMethodDecl(hasName("compare")))), + argumentCountIs(3), + hasArgument(2, StringCStrCallExpr)), + this); + + // Detect: 'dst.find(str.c_str())' -> 'dst.find(str)' + Finder->addMatcher( + cxxMemberCallExpr(on(StringExpr), + callee(decl(cxxMethodDecl( + hasAnyName("find", "find_first_not_of", "find_first_of", + "find_last_not_of", "find_last_of", "rfind")))), + anyOf(argumentCountIs(1), argumentCountIs(2)), + hasArgument(0, StringCStrCallExpr)), + this); + + // Detect: 'dst.insert(pos, str.c_str())' -> 'dst.insert(pos, str)' + Finder->addMatcher( + cxxMemberCallExpr(on(StringExpr), + callee(decl(cxxMethodDecl(hasName("insert")))), + argumentCountIs(2), + hasArgument(1, StringCStrCallExpr)), + this); + + // Detect redundant 'c_str()' calls through a StringRef constructor. + Finder->addMatcher( cxxConstructExpr( // Implicit constructors of these classes are overloaded // wrt. string types and they internally make a StringRef // referring to the argument. Passing a string directly to // them is preferred to passing a char pointer. hasDeclaration( - cxxMethodDecl(anyOf(hasName("::llvm::StringRef::StringRef"), - hasName("::llvm::Twine::Twine")))), + cxxMethodDecl(hasAnyName("::llvm::StringRef::StringRef", + "::llvm::Twine::Twine"))), argumentCountIs(1), // The only argument must have the form x.c_str() or p->c_str() // where the method is string::c_str(). StringRef also has 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 @@ -1,5 +1,8 @@ -// RUN: %check_clang_tidy %s readability-redundant-string-cstr %t -- -- -target x86_64-unknown -std=c++11 +// RUN: %check_clang_tidy %s readability-redundant-string-cstr %t -- -- -std=c++11 +typedef unsigned __INT16_TYPE__ char16; +typedef unsigned __INT32_TYPE__ char32; + namespace std { template class allocator {}; @@ -7,16 +10,50 @@ class char_traits {}; template struct basic_string { + typedef basic_string _Type; basic_string(); basic_string(const C *p, const A &a = A()); + const C *c_str() const; + + _Type& append(const C *s); + _Type& append(const C *s, size_t n); + _Type& assign(const C *s); + _Type& assign(const C *s, size_t n); + + int compare(const _Type&) const; + int compare(const C* s) const; + int compare(size_t pos, size_t len, const _Type&) const; + int compare(size_t pos, size_t len, const C* s) const; + + size_t find(const _Type& str, size_t pos = 0) const; + size_t find(const C* s, size_t pos = 0) const; + size_t find(const C* s, size_t pos, size_t n) const; + + _Type& insert(size_t pos, const _Type& str); + _Type& insert(size_t pos, const C* s); + _Type& insert(size_t pos, const C* s, size_t n); + + _Type& operator+=(const _Type& str); + _Type& operator+=(const C* s); + _Type& operator=(const _Type& str); + _Type& operator=(const C* s); }; + typedef basic_string, std::allocator> string; typedef basic_string, std::allocator> wstring; -typedef basic_string, std::allocator> u16string; -typedef basic_string, std::allocator> u32string; +typedef basic_string, std::allocator> u16string; +typedef basic_string, std::allocator> u32string; } +std::string operator+(const std::string&, const std::string&); +std::string operator+(const std::string&, const char*); +std::string operator+(const char*, const std::string&); + +bool operator==(const std::string&, const std::string&); +bool operator==(const std::string&, const char*); +bool operator==(const char*, const std::string&); + namespace llvm { struct StringRef { StringRef(const char *p); @@ -51,7 +88,81 @@ // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: redundant call to `c_str()` [readability-redundant-string-cstr] // CHECK-FIXES: {{^ }}f1(*ptr);{{$}} } +void f5(const std::string &s) { + std::string tmp; + tmp.append(s.c_str()); + // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: redundant call {{.*}} + // CHECK-FIXES: {{^ }}tmp.append(s);{{$}} + tmp.assign(s.c_str()); + // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: redundant call {{.*}} + // CHECK-FIXES: {{^ }}tmp.assign(s);{{$}} + if (tmp.compare(s.c_str()) == 0) return; + // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: redundant call {{.*}} + // CHECK-FIXES: {{^ }}if (tmp.compare(s) == 0) return;{{$}} + + if (tmp.compare(1, 2, s.c_str()) == 0) return; + // CHECK-MESSAGES: :[[@LINE-1]]:25: warning: redundant call {{.*}} + // CHECK-FIXES: {{^ }}if (tmp.compare(1, 2, s) == 0) return;{{$}} + + if (tmp.find(s.c_str()) == 0) return; + // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: redundant call {{.*}} + // CHECK-FIXES: {{^ }}if (tmp.find(s) == 0) return;{{$}} + + if (tmp.find(s.c_str(), 2) == 0) return; + // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: redundant call {{.*}} + // CHECK-FIXES: {{^ }}if (tmp.find(s, 2) == 0) return;{{$}} + + if (tmp.find(s.c_str(), 2) == 0) return; + // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: redundant call {{.*}} + // CHECK-FIXES: {{^ }}if (tmp.find(s, 2) == 0) return;{{$}} + + tmp.insert(1, s.c_str()); + // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: redundant call {{.*}} + // CHECK-FIXES: {{^ }}tmp.insert(1, s);{{$}} + + tmp = s.c_str(); + // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: redundant call {{.*}} + // CHECK-FIXES: {{^ }}tmp = s;{{$}} + + tmp += s.c_str(); + // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: redundant call {{.*}} + // CHECK-FIXES: {{^ }}tmp += s;{{$}} + + if (tmp == s.c_str()) return; + // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: redundant call {{.*}} + // CHECK-FIXES: {{^ }}if (tmp == s) return;{{$}} + + tmp = s + s.c_str(); + // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: redundant call {{.*}} + // CHECK-FIXES: {{^ }}tmp = s + s;{{$}} + + tmp = s.c_str() + s; + // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: redundant call {{.*}} + // CHECK-FIXES: {{^ }}tmp = s + s;{{$}} +} +void f6(const std::string &s) { + std::string tmp; + tmp.append(s.c_str(), 2); + tmp.assign(s.c_str(), 2); + + if (tmp.compare(s) == 0) return; + if (tmp.compare(1, 2, s) == 0) return; + + tmp = s; + tmp += s; + + if (tmp == s) + return; + + tmp = s + s; + + if (tmp.find(s.c_str(), 2, 4) == 0) return; + + tmp.insert(1, s); + tmp.insert(1, s.c_str(), 2); +} + // Tests for std::wstring. void g1(const std::wstring &s) {