[clang-tidy] Support std::string_view in readability-redundant-string-cstr

[tberghammer]

Summary

Previously we were matching constructor calls for std::string and
llvm::StringRef and this change extends this set with including
std::string_view as well.

[carlosgalvezp]

Please document the changes in the Release Notes.

[tberghammer]

Add release notes

[tberghammer]

Fix formatting type in release notes

[carlosgalvezp]

LGTM! Perhaps give the other reviewers a couple days to take a look in case I've missed anything.

[njames93]

clang-tools-extra/test/clang-tidy/checkers/readability/redundant-string-cstr.cpp

56

Can you add typedefs and tests for wstring_view and others

[tberghammer]

Add wstring_view test

[carlosgalvezp]

LGTM, perhaps give a couple days to @njames93 to give a final look?

[njames93]

LGTM, thank you for the patch

[carlosgalvezp]

@tberghammer Do you need help to land the patch?

[tberghammer]

Rebase

[tberghammer]

Sorry, I was away during the holidays but just pushed the change.

clang-tools-extra/clang-tidy/readability/RedundantStringCStrCheck.cpp

 //===- RedundantStringCStrCheck.cpp - Check for redundant c_str calls -----===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
const auto StringConstructorExpr = expr(anyOf(
                        hasDeclaration(cxxMethodDecl(hasName("basic_string")))),
       cxxConstructExpr(
           argumentCountIs(2),
           hasDeclaration(cxxMethodDecl(hasName("basic_string"))),
           // If present, the second argument is the alloc object which must not
           // be present explicitly.
           hasArgument(1, cxxDefaultArgExpr()))));
 
+  // Match string constructor.
+  const auto StringViewConstructorExpr = cxxConstructExpr(
+      argumentCountIs(1),
+      hasDeclaration(cxxMethodDecl(hasName("basic_string_view"))));
+
   // Match a call to the string 'c_str()' method.
   const auto StringCStrCallExpr =
       cxxMemberCallExpr(on(StringExpr.bind("arg")),
                         callee(memberExpr().bind("member")),
                         callee(cxxMethodDecl(hasAnyName("c_str", "data"))))
           .bind("call");
   const auto HasRValueTempParent =
       hasParent(materializeTemporaryExpr(unless(isBoundToLValue())));
   // Detect redundant 'c_str()' calls through a string constructor.
   // If CxxConstructExpr is the part of some CallExpr we need to
   // check that matched ParamDecl of the ancestor CallExpr is not rvalue.
   Finder->addMatcher(
       traverse(
           TK_AsIs,
           cxxConstructExpr(
-              StringConstructorExpr, hasArgument(0, StringCStrCallExpr),
+              anyOf(StringConstructorExpr, StringViewConstructorExpr),
+              hasArgument(0, StringCStrCallExpr),
               unless(anyOf(HasRValueTempParent, hasParent(cxxBindTemporaryExpr(
                                                     HasRValueTempParent)))))),
       this);
 
   // Detect: 's == str.c_str()'  ->  's == str'
   Finder->addMatcher(
       cxxOperatorCallExpr(
           hasAnyOverloadedOperatorName("<", ">", ">=", "<=", "!=", "==", "+"),

clang-tools-extra/docs/ReleaseNotes.rst

- Change the default behavior of :doc:`readability-const-return-type
   warn about `const` value parameters of declarations inside macros.
 
 - Improved :doc:`misc-redundant-expression <clang-tidy/checks/misc/redundant-expression>`
   check.
 
   The check now skips concept definitions since redundant expressions still make sense
   inside them.
 
+- Support removing ``c_str`` calls from std::string_view constructor calls in
+  :doc: `readability-redundant-string-cstr <clang-tidy/checks/readability/redundant-string-cstr>`
+  check.
+
 Removed checks
 ^^^^^^^^^^^^^^
 
 Improvements to include-fixer
 -----------------------------
 
 The improvements are...
 

clang-tools-extra/test/clang-tidy/checkers/readability/redundant-string-cstr.cpp

 // RUN: %check_clang_tidy %s readability-redundant-string-cstr %t
 
 typedef unsigned __INT16_TYPE__ char16;
 typedef unsigned __INT32_TYPE__ char32;
 typedef __SIZE_TYPE__ size;
 
 namespace std {
 template <typename T>
struct basic_string {
   _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, std::char_traits<char16>, std::allocator<char16>> u16string;
 typedef basic_string<char32, std::char_traits<char32>, std::allocator<char32>> u32string;
+
+template <typename C, typename T>
+struct basic_string_view {
+  basic_string_view(const C* s);
+};
+typedef basic_string_view<char, std::char_traits<char>> string_view;
 }
 
 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*);
void f6(const std::string &s) {
 
   tmp = s + s;
 
   if (tmp.find(s.c_str(), 2, 4) == 0) return;
 
   tmp.insert(1, s);
   tmp.insert(1, s.c_str(), 2);
 }
+void f7(std::string_view sv) {
+  std::string s;
+  f7(s.c_str());
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: redundant call to 'c_str' [readability-redundant-string-cstr]
+  // CHECK-FIXES: {{^  }}f7(s);{{$}}
+  f7(s.data());
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: redundant call to 'data' [readability-redundant-string-cstr]
+  // CHECK-FIXES: {{^  }}f7(s);{{$}}
+}
 
 // Tests for std::wstring.
 
 void g1(const std::wstring &s) {
   g1(s.c_str());
   // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: redundant call to 'c_str' [readability-redundant-string-cstr]
   // CHECK-FIXES: {{^  }}g1(s);{{$}}
 }