f00kat created this revision.
f00kat added reviewers: aaron.ballman, alexfh.
f00kat added projects: clang-tools-extra, clang.
Herald added subscribers: cfe-commits, xazax.hun.
#include <string>
static void f2(std::string&&) {
}
static void f() {
std::string const s;
f2(s.c_str()); // readability-redundant-string-cstr warning
}
For this case I decide to do nothing by skipping it in the matcher. Another way
is to fix with 'std::move(s)' but I`m not sure that it is correct.
Repository:
rG LLVM Github Monorepo
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,9 @@
void invalid(const NotAString &s) {
dummy(s.c_str());
}
+
+// Test for rvalue std::string
+
+void m1(std::string&& s) {
+ m1(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,54 @@
return (llvm::Twine("*") + Text).str();
}
+// Trying to get CallExpr in which CxxConstructExpr is called
+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
+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 *TheCallExprDecl =
+ dyn_cast<FunctionDecl>(TheCallExpr->getCalleeDecl())) {
+ if (TheCallExprDecl->getParamDecl(i)
+ ->getType()
+ ->isRValueReferenceType())
+ return true;
+ }
+ }
+ }
+ }
+
+ return false;
+}
+
+AST_MATCHER(CXXConstructExpr,
+ matchedParamDeclOfAncestorCallExprHasRValueRefType) {
+ return checkParamDeclOfAncestorCallExprHasRValueRefType(
+ &Node, Finder->getASTContext());
+}
+
} // end namespace
void RedundantStringCStrCheck::registerMatchers(
@@ -95,9 +143,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(
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,9 @@
void invalid(const NotAString &s) {
dummy(s.c_str());
}
+
+// Test for rvalue std::string
+
+void m1(std::string&& s) {
+ m1(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,54 @@
return (llvm::Twine("*") + Text).str();
}
+// Trying to get CallExpr in which CxxConstructExpr is called
+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
+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 *TheCallExprDecl =
+ dyn_cast<FunctionDecl>(TheCallExpr->getCalleeDecl())) {
+ if (TheCallExprDecl->getParamDecl(i)
+ ->getType()
+ ->isRValueReferenceType())
+ return true;
+ }
+ }
+ }
+ }
+
+ return false;
+}
+
+AST_MATCHER(CXXConstructExpr,
+ matchedParamDeclOfAncestorCallExprHasRValueRefType) {
+ return checkParamDeclOfAncestorCallExprHasRValueRefType(
+ &Node, Finder->getASTContext());
+}
+
} // end namespace
void RedundantStringCStrCheck::registerMatchers(
@@ -95,9 +143,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(
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
