[PATCH] D74033: [clang-tidy] Fix PR#44620 'readability-redundant-string-cstr quick-fix causes invalid code'

[Karasev Nikita via Phabricator via cfe-commits]

f00kat updated this revision to Diff 243833.
f00kat added a comment.

1. Add full stops at the end of the lines.
2. Add some static.
3. Fix function 'checkParamDeclOfAncestorCallExprHasRValueRefType' to work well with the function pointer calls.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74033/new/

https://reviews.llvm.org/D74033

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
@@ -205,3 +205,18 @@
 void invalid(const NotAString &s) {
   dummy(s.c_str());
 }
+
+// Test for rvalue std::string.
+void m1(std::string&&) {
+  std::string s;
+
+  m1(s.c_str());
+
+  void (*m1p1)(std::string&&);
+  m1p1 = m1;
+  m1p1(s.c_str());
+
+  using m1tp = void (*)(std::string &&);
+  m1tp m1p2 = m1;
+  m1p2(s.c_str());  
+}
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
@@ -61,6 +61,55 @@
   return (llvm::Twine("*") + Text).str();
 }
 
+// Trying to get CallExpr in which CxxConstructExpr is called.
+static const clang::CallExpr *
+tryGetCallExprAncestorForCxxConstructExpr(const Expr *TheExpr,
+                                          ASTContext &Context) {
+  // We skip nodes such as CXXBindTemporaryExpr, MaterializeTemporaryExpr.
+  for (ast_type_traits::DynTypedNode DynParent : Context.getParents(*TheExpr)) {
+    if (const auto *Parent = DynParent.get<Expr>()) {
+      if (const auto *TheCallExpr = dyn_cast<CallExpr>(Parent))
+        return TheCallExpr;
+
+      if (const clang::CallExpr *TheCallExpr =
+              tryGetCallExprAncestorForCxxConstructExpr(Parent, Context))
+        return TheCallExpr;
+    }
+  }
+
+  return nullptr;
+}
+
+// Check that ParamDecl of CallExprDecl has rvalue type.
+static bool checkParamDeclOfAncestorCallExprHasRValueRefType(
+    const Expr *TheCxxConstructExpr, ASTContext &Context) {
+  if (const clang::CallExpr *TheCallExpr =
+          tryGetCallExprAncestorForCxxConstructExpr(TheCxxConstructExpr,
+                                                    Context)) {
+    for (int i = 0; i < TheCallExpr->getNumArgs(); ++i) {
+      const Expr *Arg = TheCallExpr->getArg(i);
+      if (Arg->getSourceRange() == TheCxxConstructExpr->getSourceRange()) {
+        if (const auto *TheCallExprFuncProto =
+                TheCallExpr->getCallee()
+                    ->getType()
+                    ->getPointeeType()
+                    ->getAs<FunctionProtoType>()) {
+          if (TheCallExprFuncProto->getParamType(i)->isRValueReferenceType())
+            return true;
+        }
+      }
+    }
+  }
+
+  return false;
+}
+
+AST_MATCHER(CXXConstructExpr,
+            matchedParamDeclOfAncestorCallExprHasRValueRefType) {
+  return checkParamDeclOfAncestorCallExprHasRValueRefType(
+      &Node, Finder->getASTContext());
+}
+
 } // end namespace
 
 void RedundantStringCStrCheck::registerMatchers(
@@ -95,9 +144,13 @@
           .bind("call");
 
   // Detect redundant 'c_str()' calls through a string constructor.
-  Finder->addMatcher(cxxConstructExpr(StringConstructorExpr,
-                                      hasArgument(0, StringCStrCallExpr)),
-                     this);
+  // If CxxConstructExpr is the part of some CallExpr we need to
+  // check that matched ParamDecl of the ancestor CallExpr is not rvalue.
+  Finder->addMatcher(
+      cxxConstructExpr(
+          StringConstructorExpr, hasArgument(0, StringCStrCallExpr),
+          unless(matchedParamDeclOfAncestorCallExprHasRValueRefType())),
+      this);
 
   // Detect: 's == str.c_str()'  ->  's == str'
   Finder->addMatcher(


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D74033.243833.patch
Type: text/x-patch
Size: 3655 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20200211/abf5c356/attachment-0001.bin>