[PATCH] D145730: [clang-tidy] readability-redundant-string-cstr for smart pointer #576705

[Mike Crowe via Phabricator via cfe-commits]

mikecrowe created this revision.
mikecrowe added a reviewer: njames93.
Herald added subscribers: PiotrZSL, carlosgalvezp, xazax.hun.
Herald added a project: All.
mikecrowe requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.

The readability-redundant-string-cstr check incorrectly replaces calls
to c_str() that involve an overloaded operator->.

Running `clang-tidy --checks="-*,readability-redundant-string-cstr"` on:

#include <string>
 #include <vector>

void it(std::vector<std::string>::const_iterator i)
 {

  std::string tmp;
  tmp = i->c_str();

}

yields:

/home/mac/git/llvm-project/build/../bug.cpp:7:9: warning: redundant call to 'c_str' [readability-redundant-string-cstr]

  tmp = i->c_str();
        ^~~~~~~~~~
        *i->

which means that the code is "fixed" to incorrectly say:

  tmp = *i->;

rather than the expected:

  tmp = *i;

This appears to be due to the overloaded `operator->` meaning that
RedundantStringCStrCheck.cpp#L53 ends up with Text containing "i->"
rather than just the expected "i".

Add test case for this and fix it in a somewhat nasty manner in the
absence of something better.

Fixes: https://github.com/llvm/llvm-project/issues/56705


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D145730

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


Index: clang-tools-extra/test/clang-tidy/checkers/readability/redundant-string-cstr.cpp
===================================================================
--- clang-tools-extra/test/clang-tidy/checkers/readability/redundant-string-cstr.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/readability/redundant-string-cstr.cpp
@@ -1,6 +1,11 @@
 // RUN: %check_clang_tidy %s readability-redundant-string-cstr %t -- -- -isystem %clang_tidy_headers
 #include <string>
 
+template <typename T>
+struct iterator {
+  T *operator->();
+};
+
 namespace llvm {
 struct StringRef {
   StringRef(const char *p);
@@ -202,6 +207,15 @@
   m1p2(s.c_str());  
 }
 
+// Test for iterator
+void it(iterator<std::string> i)
+{
+  std::string tmp;
+  tmp = i->c_str();
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: redundant call to 'c_str' [readability-redundant-string-cstr]
+  // CHECK-FIXES: {{^  }}tmp = *i;{{$}}
+}
+
 namespace PR45286 {
 struct Foo {
   void func(const std::string &) {}
Index: clang-tools-extra/clang-tidy/readability/RedundantStringCStrCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/readability/RedundantStringCStrCheck.cpp
+++ clang-tools-extra/clang-tidy/readability/RedundantStringCStrCheck.cpp
@@ -52,6 +52,10 @@
 
   if (Text.empty())
     return std::string();
+
+  // https://github.com/llvm/llvm-project/issues/56705
+  Text.consume_back("->");
+
   // Add leading '*'.
   if (needParensAfterUnaryOperator(ExprNode)) {
     return (llvm::Twine("*(") + Text + ")").str();


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D145730.503923.patch
Type: text/x-patch
Size: 1550 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20230309/d45fda62/attachment.bin>