[clang-tools-extra] r266463 - [clang-tidy] Add more detection rules for redundant c_str calls.
Etienne Bergeron via cfe-commits
cfe-commits at lists.llvm.org
Fri Apr 15 11:12:06 PDT 2016
Author: etienneb
Date: Fri Apr 15 13:12:06 2016
New Revision: 266463
URL: http://llvm.org/viewvc/llvm-project?rev=266463&view=rev
Log:
[clang-tidy] Add more detection rules for redundant c_str calls.
Summary:
The string class contains methods which support receiving either a string literal or a string object.
For example, calls to append can receive either a char* or a string.
```
string& append (const string& str);
string& append (const char* s);
```
Which make these cases equivalent, and the .c_str() useless:
```
std::string s = "123";
str.append(s);
str.append(s.c_str());
```
In these cases, removing .c_str() doesn't provide any size or speed improvement.
It's only a readability issue.
If the string contains embedded NUL characters, the string literal and the string
object won't produce the same semantic.
Reviewers: alexfh, sbenza
Subscribers: LegalizeAdulthood, aaron.ballman, chapuni, Eugene.Zelenko, cfe-commits
Differential Revision: http://reviews.llvm.org/D18475
Modified:
clang-tools-extra/trunk/clang-tidy/readability/RedundantStringCStrCheck.cpp
clang-tools-extra/trunk/test/clang-tidy/readability-redundant-string-cstr.cpp
Modified: clang-tools-extra/trunk/clang-tidy/readability/RedundantStringCStrCheck.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/readability/RedundantStringCStrCheck.cpp?rev=266463&r1=266462&r2=266463&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/readability/RedundantStringCStrCheck.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/readability/RedundantStringCStrCheck.cpp Fri Apr 15 13:12:06 2016
@@ -103,11 +103,74 @@ void RedundantStringCStrCheck::registerM
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
@@ -115,8 +178,8 @@ void RedundantStringCStrCheck::registerM
// 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
Modified: clang-tools-extra/trunk/test/clang-tidy/readability-redundant-string-cstr.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/readability-redundant-string-cstr.cpp?rev=266463&r1=266462&r2=266463&view=diff
==============================================================================
--- clang-tools-extra/trunk/test/clang-tidy/readability-redundant-string-cstr.cpp (original)
+++ clang-tools-extra/trunk/test/clang-tidy/readability-redundant-string-cstr.cpp Fri Apr 15 13:12:06 2016
@@ -1,4 +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;
+typedef __SIZE_TYPE__ size;
namespace std {
template <typename T>
@@ -7,16 +11,50 @@ template <typename T>
class char_traits {};
template <typename C, typename T, typename A>
struct basic_string {
+ typedef basic_string<C, T, A> _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 n);
+ _Type& assign(const C *s);
+ _Type& assign(const C *s, size n);
+
+ int compare(const _Type&) const;
+ int compare(const C* s) const;
+ int compare(size pos, size len, const _Type&) const;
+ int compare(size pos, size len, const C* s) const;
+
+ size find(const _Type& str, size pos = 0) const;
+ size find(const C* s, size pos = 0) const;
+ size find(const C* s, size pos, size n) const;
+
+ _Type& insert(size pos, const _Type& str);
+ _Type& insert(size pos, const C* s);
+ _Type& insert(size pos, const C* s, size n);
+
+ _Type& operator+=(const _Type& str);
+ _Type& operator+=(const C* s);
+ _Type& operator=(const _Type& str);
+ _Type& operator=(const C* s);
};
+
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;
+typedef basic_string<char16, std::char_traits<char16>, std::allocator<char16>> u16string;
+typedef basic_string<char32, std::char_traits<char32>, std::allocator<char32>> 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,6 +89,80 @@ void f4(const std::string &s) {
// 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.
More information about the cfe-commits
mailing list