Author: Nathan James Date: 2020-03-31T13:27:32+01:00 New Revision: 3807079d705fe04c5c3bde8f848ec922b5771f15
URL: https://github.com/llvm/llvm-project/commit/3807079d705fe04c5c3bde8f848ec922b5771f15 DIFF: https://github.com/llvm/llvm-project/commit/3807079d705fe04c5c3bde8f848ec922b5771f15.diff LOG: [clang-tidy] Fix crash in readability-redundant-string-cstr Summary: Addresses [[ https://bugs.llvm.org/show_bug.cgi?id=45286 | clang-tidy-11: Crash in DynTypedMatcher::matches during readability-redundant-string-cstr check ]] Reviewers: aaron.ballman, alexfh, gribozavr2 Reviewed By: gribozavr2 Subscribers: xazax.hun, cfe-commits Tags: #clang, #clang-tools-extra Differential Revision: https://reviews.llvm.org/D76761 Added: Modified: clang-tools-extra/clang-tidy/readability/RedundantStringCStrCheck.cpp clang-tools-extra/test/clang-tidy/checkers/readability-redundant-string-cstr.cpp Removed: ################################################################################ diff --git a/clang-tools-extra/clang-tidy/readability/RedundantStringCStrCheck.cpp b/clang-tools-extra/clang-tidy/readability/RedundantStringCStrCheck.cpp index 8975f294373c..e41cdfcc08d8 100644 --- a/clang-tools-extra/clang-tidy/readability/RedundantStringCStrCheck.cpp +++ b/clang-tools-extra/clang-tidy/readability/RedundantStringCStrCheck.cpp @@ -61,53 +61,8 @@ 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 (unsigned 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()); +AST_MATCHER(MaterializeTemporaryExpr, isBoundToLValue) { + return Node.isBoundToLvalueReference(); } } // end namespace @@ -141,11 +96,11 @@ void RedundantStringCStrCheck::registerMatchers( // 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( - cxxConstructExpr( - StringConstructorExpr, hasArgument(0, StringCStrCallExpr), - unless(matchedParamDeclOfAncestorCallExprHasRValueRefType())), - this); + Finder->addMatcher(cxxConstructExpr(StringConstructorExpr, + hasArgument(0, StringCStrCallExpr), + unless(hasParent(materializeTemporaryExpr( + unless(isBoundToLValue()))))), + this); // Detect: 's == str.c_str()' -> 's == str' Finder->addMatcher( diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability-redundant-string-cstr.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability-redundant-string-cstr.cpp index 1773dc57a8d8..2561b81805bd 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/readability-redundant-string-cstr.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/readability-redundant-string-cstr.cpp @@ -220,3 +220,27 @@ void m1(std::string&&) { m1tp m1p2 = m1; m1p2(s.c_str()); } + +namespace PR45286 { +struct Foo { + void func(const std::string &) {} + void func2(std::string &&) {} +}; + +void bar() { + std::string Str{"aaa"}; + Foo Foo; + Foo.func(Str.c_str()); + // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: redundant call to 'c_str' [readability-redundant-string-cstr] + // CHECK-FIXES: {{^ }}Foo.func(Str);{{$}} + + // Ensure it doesn't transform Binding to r values + Foo.func2(Str.c_str()); + + // Ensure its not confused by parens + Foo.func((Str.c_str())); + // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: redundant call to 'c_str' [readability-redundant-string-cstr] + // CHECK-FIXES: {{^ }}Foo.func((Str));{{$}} + Foo.func2((Str.c_str())); +} +} // namespace PR45286 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits