[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 llvm-svn: 264075
This commit is contained in:
parent
190fadcdb2
commit
4c3b55cfce
|
@ -85,23 +85,29 @@ void RedundantStringCStrCheck::registerMatchers(
|
|||
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::registerMatchers(
|
|||
// 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);
|
||||
}
|
||||
|
||||
|
|
|
@ -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);{{$}}
|
||||
}
|
Loading…
Reference in New Issue