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

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.


================
Comment at: clang-tools-extra/clang-tidy/readability/RedundantStringCStrCheck.cpp:64
 
+// Trying to get CallExpr in which CxxConstructExpr is called
+const clang::CallExpr *
----------------
Missing a full stop at the end of the comment.


================
Comment at: clang-tools-extra/clang-tidy/readability/RedundantStringCStrCheck.cpp:66
+const clang::CallExpr *
+tryGetCallExprAncestorForCxxConstructExpr(const Expr *TheExpr,
+                                         ASTContext &Context) {
----------------
This function and the one below can be `static`.


================
Comment at: clang-tools-extra/clang-tidy/readability/RedundantStringCStrCheck.cpp:83
+
+// Check that ParamDecl of CallExprDecl has rvalue type
+bool checkParamDeclOfAncestorCallExprHasRValueRefType(
----------------
Missing full stop at the end of the comment.


================
Comment at: clang-tools-extra/clang-tidy/readability/RedundantStringCStrCheck.cpp:93
+        if (const auto *TheCallExprDecl =
+                dyn_cast<FunctionDecl>(TheCallExpr->getCalleeDecl())) {
+          if (TheCallExprDecl->getParamDecl(i)
----------------
What if the call expression has no callee decl, like a call through a function pointer? This might have to be `dyn_cast_or_null<>` (That's probably a good test case to add.)


================
Comment at: clang-tools-extra/clang-tidy/readability/RedundantStringCStrCheck.cpp:147
+  // If CxxConstructExpr is the part of some CallExpr we need to
+  // check that matched ParamDecl of the ancestor CallExpr is not rvalue
+  Finder->addMatcher(
----------------
Missing a full stop at the end of the comment.


================
Comment at: clang-tools-extra/test/clang-tidy/checkers/readability-redundant-string-cstr.cpp:209-210
+
+// Test for rvalue std::string
+
+void m1(std::string&& s) {
----------------
Missing full stop, can also remove the trailing newline.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74033