Fix PR#44620 'readability-redundant-string-cstr quick-fix causes invalid code'

static void f2(std::string&&) {}
static void f() {
	std::string const s;
	f2(s.c_str()); // readability-redundant-string-cstr previously warning
}

Skips the problematic AST pattern in the matcher.
This commit is contained in:
Karasev Nikita 2020-02-18 15:32:03 -05:00 committed by Aaron Ballman
parent faa889b235
commit 47282b1b4b
2 changed files with 71 additions and 3 deletions

View File

@ -61,6 +61,55 @@ formatDereference(const ast_matchers::MatchFinder::MatchResult &Result,
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 @@ void RedundantStringCStrCheck::registerMatchers(
.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(

View File

@ -205,3 +205,18 @@ void dummy(const char*) {}
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());
}