Author: etienneb
Date: Tue Mar 22 13:00:13 2016
New Revision: 264075
URL: http://llvm.org/viewvc/llvm-project?rev=264075&view=rev
Log:
[clang-tidy] Fix redundant-string-cstr check with msvc 14 headers.
Summary:
The string constructors are not defined using optional parameters and are not
recognize by the checker.
The constructor defined in the MSVC header is defined with 1 parameter.
Therefore, patterns are not recognized by the checker.
The current patch add support to accept constructor with only one parameter.
Repro on a Visual Studio 14 installation with the following code:
```
void f1(const std::string &s) {
f1(s.c_str());
}
```
In the xstring.h header, the constructors are defined this way:
```
basic_string(const _Myt& _Right) [...]
basic_string(const _Myt& _Right, const _Alloc& _Al) [...]
```
The CXXConstructExpr to recognize only contains 1 parameter.
```
CXXConstructExpr 0x3f1a070 <C:\src\llvm\examples\test.cc:6:6, col:14> 'const
std::string':'const class std::basic_string<char, struct
std::char_traits<char>, class
std::allocator<char> >' 'void (const char *) __attribute__((thiscall))'
`-CXXMemberCallExpr 0x3f1a008 <col:6, col:14> 'const char *'
`-MemberExpr 0x3f19fe0 <col:6, col:8> '<bound member function type>' .c_str
0x3cc22f8
`-DeclRefExpr 0x3f19fc8 <col:6> 'const std::string':'const class
std::basic_string<char, struct std::char_traits<char>, class
std::allocator<char> >' lvalue ParmVar 0x3f19c80 's' 'const std::string &'
```
Reviewers: aaron.ballman, alexfh
Subscribers: aemerson
Differential Revision: http://reviews.llvm.org/D18285
Added:
clang-tools-extra/trunk/test/clang-tidy/readability-redundant-string-cstr-msvc.cpp
Modified:
clang-tools-extra/trunk/clang-tidy/readability/RedundantStringCStrCheck.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=264075&r1=264074&r2=264075&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/readability/RedundantStringCStrCheck.cpp
(original)
+++ clang-tools-extra/trunk/clang-tidy/readability/RedundantStringCStrCheck.cpp
Tue Mar 22 13:00:13 2016
@@ -85,23 +85,29 @@ void RedundantStringCStrCheck::registerM
if (!getLangOpts().CPlusPlus)
return;
- Finder->addMatcher(
+ // Match string constructor.
+ const auto StringConstructorExpr = expr(anyOf(
+ cxxConstructExpr(
+ argumentCountIs(1),
+ hasDeclaration(cxxMethodDecl(hasName(StringConstructor)))),
cxxConstructExpr(
- hasDeclaration(cxxMethodDecl(hasName(StringConstructor))),
argumentCountIs(2),
- // The first argument must have the form x.c_str() or p->c_str()
- // where the method is string::c_str(). We can use the copy
- // constructor of string instead (or the compiler might share
- // the string object).
- hasArgument(0, cxxMemberCallExpr(
- callee(memberExpr().bind("member")),
- callee(cxxMethodDecl(hasName(StringCStrMethod))),
- on(expr().bind("arg")))
- .bind("call")),
- // The second argument is the alloc object which must not be
- // present explicitly.
- hasArgument(1, cxxDefaultArgExpr())),
+ hasDeclaration(cxxMethodDecl(hasName(StringConstructor))),
+ // 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");
+
+ Finder->addMatcher(
+ cxxConstructExpr(StringConstructorExpr,
+ hasArgument(0, StringCStrCallExpr)),
this);
+
Finder->addMatcher(
cxxConstructExpr(
// Implicit constructors of these classes are overloaded
@@ -117,11 +123,7 @@ void RedundantStringCStrCheck::registerM
// a constructor from string which is more efficient (avoids
// strlen), so we can construct StringRef from the string
// directly.
- hasArgument(0, cxxMemberCallExpr(
- callee(memberExpr().bind("member")),
- callee(cxxMethodDecl(hasName(StringCStrMethod))),
- on(expr().bind("arg")))
- .bind("call"))),
+ hasArgument(0, StringCStrCallExpr)),
this);
}
Added:
clang-tools-extra/trunk/test/clang-tidy/readability-redundant-string-cstr-msvc.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/readability-redundant-string-cstr-msvc.cpp?rev=264075&view=auto
==============================================================================
---
clang-tools-extra/trunk/test/clang-tidy/readability-redundant-string-cstr-msvc.cpp
(added)
+++
clang-tools-extra/trunk/test/clang-tidy/readability-redundant-string-cstr-msvc.cpp
Tue Mar 22 13:00:13 2016
@@ -0,0 +1,43 @@
+// RUN: %check_clang_tidy %s readability-redundant-string-cstr %t
+
+namespace std {
+template <typename T>
+class allocator {};
+template <typename T>
+class char_traits {};
+template <typename C, typename T, typename A>
+struct basic_string {
+ basic_string();
+ // MSVC headers define two constructors instead of using optional arguments.
+ basic_string(const C *p);
+ basic_string(const C *p, const A &a);
+ const C *c_str() const;
+};
+typedef basic_string<char, std::char_traits<char>, std::allocator<char>>
string;
+}
+namespace llvm {
+struct StringRef {
+ StringRef(const char *p);
+ StringRef(const 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]
+ // CHECK-FIXES: {{^ }}f1(s);{{$}}
+}
+void f2(const llvm::StringRef r) {
+ std::string s;
+ f2(s.c_str());
+ // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: redundant call {{.*}}
+ // CHECK-FIXES: {{^ }}std::string s;{{$}}
+ // CHECK-FIXES-NEXT: {{^ }}f2(s);{{$}}
+}
+void f3(const llvm::StringRef &r) {
+ std::string s;
+ f3(s.c_str());
+ // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: redundant call {{.*}}
+ // CHECK-FIXES: {{^ }}std::string s;{{$}}
+ // CHECK-FIXES-NEXT: {{^ }}f3(s);{{$}}
+}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
