[PATCH] D84831: [clang-tidy] Fix RedundantStringCStrCheck with r values

[Nathan James via Phabricator via cfe-commits]

njames93 created this revision.
njames93 added reviewers: aaron.ballman, gribozavr2, alexfh.
Herald added subscribers: cfe-commits, xazax.hun.
Herald added a project: clang.
njames93 requested review of this revision.

The previous fix for this, https://reviews.llvm.org/D76761, Passed test cases but failed in the real world as std::string has a non trivial destructor so creates a CXXBindTemporaryExpr.

This handles that shortfall and updates the test case std::basic_string implementation to use a non trivial destructor to reflect real world behaviour.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D84831

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
@@ -15,6 +15,8 @@
   basic_string();
   basic_string(const C *p, const A &a = A());
 
+  ~basic_string();
+
   const C *c_str() const;
   const C *data() const;
 
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
@@ -92,16 +92,18 @@
                         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(ast_type_traits::TK_AsIs,
-               cxxConstructExpr(StringConstructorExpr,
-                                hasArgument(0, StringCStrCallExpr),
-                                unless(hasParent(materializeTemporaryExpr(
-                                    unless(isBoundToLValue())))))),
+      traverse(
+          ast_type_traits::TK_AsIs,
+          cxxConstructExpr(
+              StringConstructorExpr, hasArgument(0, StringCStrCallExpr),
+              unless(anyOf(HasRValueTempParent, hasParent(cxxBindTemporaryExpr(
+                                                    HasRValueTempParent)))))),
       this);
 
   // Detect: 's == str.c_str()'  ->  's == str'


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D84831.281489.patch
Type: text/x-patch
Size: 2004 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20200729/86a5968f/attachment-0001.bin>