From 4c3b55cfce02a263ad8ee2b71b21472f14712847 Mon Sep 17 00:00:00 2001 From: Etienne Bergeron Date: Tue, 22 Mar 2016 18:00:13 +0000 Subject: [PATCH] [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 'const std::string':'const class std::basic_string, class std::allocator >' 'void (const char *) __attribute__((thiscall))' `-CXXMemberCallExpr 0x3f1a008 'const char *' `-MemberExpr 0x3f19fe0 '' .c_str 0x3cc22f8 `-DeclRefExpr 0x3f19fc8 'const std::string':'const class std::basic_string, class std::allocator >' lvalue ParmVar 0x3f19c80 's' 'const std::string &' ``` Reviewers: aaron.ballman, alexfh Subscribers: aemerson Differential Revision: http://reviews.llvm.org/D18285 llvm-svn: 264075 --- .../readability/RedundantStringCStrCheck.cpp | 40 ++++++++++---------- .../readability-redundant-string-cstr-msvc.cpp | 43 ++++++++++++++++++++++ 2 files changed, 64 insertions(+), 19 deletions(-) create mode 100644 clang-tools-extra/test/clang-tidy/readability-redundant-string-cstr-msvc.cpp diff --git a/clang-tools-extra/clang-tidy/readability/RedundantStringCStrCheck.cpp b/clang-tools-extra/clang-tidy/readability/RedundantStringCStrCheck.cpp index c58c1dc..64a7b61 100644 --- a/clang-tools-extra/clang-tidy/readability/RedundantStringCStrCheck.cpp +++ b/clang-tools-extra/clang-tidy/readability/RedundantStringCStrCheck.cpp @@ -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); } diff --git a/clang-tools-extra/test/clang-tidy/readability-redundant-string-cstr-msvc.cpp b/clang-tools-extra/test/clang-tidy/readability-redundant-string-cstr-msvc.cpp new file mode 100644 index 0000000..1c31118 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/readability-redundant-string-cstr-msvc.cpp @@ -0,0 +1,43 @@ +// RUN: %check_clang_tidy %s readability-redundant-string-cstr %t + +namespace std { +template +class allocator {}; +template +class char_traits {}; +template +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, std::allocator> 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);{{$}} +} -- 2.7.4