etienneb created this revision.
etienneb added a subscriber: cfe-commits.
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.
http://reviews.llvm.org/D18475
Files:
clang-tidy/readability/RedundantStringCStrCheck.cpp
test/clang-tidy/readability-redundant-string-cstr.cpp
Index: clang-tidy/readability/RedundantStringCStrCheck.cpp
===================================================================
--- clang-tidy/readability/RedundantStringCStrCheck.cpp
+++ clang-tidy/readability/RedundantStringCStrCheck.cpp
@@ -103,12 +103,79 @@
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(anyOf(hasName("append"),
+ hasName("assign"),
+ hasName("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(anyOf(hasName("find"),
+ hasName("find_first_not_of"),
+ hasName("find_first_of"),
+ hasName("find_last_not_of"),
+ hasName("find_last_of"),
+ hasName("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
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
+typedef unsigned __INT16_TYPE__ char16;
+typedef unsigned __INT32_TYPE__ char32;
+
namespace std {
template <typename T>
class allocator {};
@@ -7,16 +10,50 @@
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_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<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,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) {
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
