njames93 created this revision. njames93 added reviewers: aaron.ballman, alexfh, LegalizeAdulthood. Herald added subscribers: carlosgalvezp, xazax.hun. Herald added a project: All. njames93 requested review of this revision. Herald added a project: clang-tools-extra. Herald added a subscriber: cfe-commits.
Fixes https://github.com/llvm/llvm-project/issues/56705. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D130639 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 @@ -246,3 +246,15 @@ Foo.func2((Str.c_str())); } } // namespace PR45286 + +template<typename T> +struct SmartPtr{ + T* operator->()const; + T& operator*()const; +}; + +void smartPtrTest(SmartPtr<std::string> X){ + f1(X->c_str()); + // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: redundant call to 'c_str' [readability-redundant-string-cstr] + // CHECK-FIXES: f1(*X); +} 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 @@ -38,29 +38,6 @@ return false; } -// Format a pointer to an expression: prefix with '*' but simplify -// when it already begins with '&'. Return empty string on failure. -std::string -formatDereference(const ast_matchers::MatchFinder::MatchResult &Result, - const Expr &ExprNode) { - if (const auto *Op = dyn_cast<clang::UnaryOperator>(&ExprNode)) { - if (Op->getOpcode() == UO_AddrOf) { - // Strip leading '&'. - return std::string(tooling::fixit::getText( - *Op->getSubExpr()->IgnoreParens(), *Result.Context)); - } - } - StringRef Text = tooling::fixit::getText(ExprNode, *Result.Context); - - if (Text.empty()) - return std::string(); - // Add leading '*'. - if (needParensAfterUnaryOperator(ExprNode)) { - return (llvm::Twine("*(") + Text + ")").str(); - } - return (llvm::Twine("*") + Text).str(); -} - AST_MATCHER(MaterializeTemporaryExpr, isBoundToLValue) { return Node.isBoundToLvalueReference(); } @@ -180,18 +157,19 @@ const auto *Call = Result.Nodes.getNodeAs<CallExpr>("call"); const auto *Arg = Result.Nodes.getNodeAs<Expr>("arg"); const auto *Member = Result.Nodes.getNodeAs<MemberExpr>("member"); - bool Arrow = Member->isArrow(); - // Replace the "call" node with the "arg" node, prefixed with '*' - // if the call was using '->' rather than '.'. - std::string ArgText = - Arrow ? formatDereference(Result, *Arg) - : tooling::fixit::getText(*Arg, *Result.Context).str(); - if (ArgText.empty()) - return; - - diag(Call->getBeginLoc(), "redundant call to %0") - << Member->getMemberDecl() - << FixItHint::CreateReplacement(Call->getSourceRange(), ArgText); + + auto D = diag(Call->getBeginLoc(), "redundant call to %0") + << Member->getMemberDecl() + << FixItHint::CreateRemoval(CharSourceRange::getTokenRange( + {Member->getOperatorLoc(), Call->getEndLoc()})); + + if (Member->isArrow()) { + if (needParensAfterUnaryOperator(*Arg)) + D << FixItHint::CreateInsertion(Arg->getBeginLoc(), "*(") + << FixItHint::CreateInsertion(Member->getOperatorLoc(), ")"); + else + D << FixItHint::CreateInsertion(Arg->getBeginLoc(), "*"); + } } } // namespace readability
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 @@ -246,3 +246,15 @@ Foo.func2((Str.c_str())); } } // namespace PR45286 + +template<typename T> +struct SmartPtr{ + T* operator->()const; + T& operator*()const; +}; + +void smartPtrTest(SmartPtr<std::string> X){ + f1(X->c_str()); + // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: redundant call to 'c_str' [readability-redundant-string-cstr] + // CHECK-FIXES: f1(*X); +} 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 @@ -38,29 +38,6 @@ return false; } -// Format a pointer to an expression: prefix with '*' but simplify -// when it already begins with '&'. Return empty string on failure. -std::string -formatDereference(const ast_matchers::MatchFinder::MatchResult &Result, - const Expr &ExprNode) { - if (const auto *Op = dyn_cast<clang::UnaryOperator>(&ExprNode)) { - if (Op->getOpcode() == UO_AddrOf) { - // Strip leading '&'. - return std::string(tooling::fixit::getText( - *Op->getSubExpr()->IgnoreParens(), *Result.Context)); - } - } - StringRef Text = tooling::fixit::getText(ExprNode, *Result.Context); - - if (Text.empty()) - return std::string(); - // Add leading '*'. - if (needParensAfterUnaryOperator(ExprNode)) { - return (llvm::Twine("*(") + Text + ")").str(); - } - return (llvm::Twine("*") + Text).str(); -} - AST_MATCHER(MaterializeTemporaryExpr, isBoundToLValue) { return Node.isBoundToLvalueReference(); } @@ -180,18 +157,19 @@ const auto *Call = Result.Nodes.getNodeAs<CallExpr>("call"); const auto *Arg = Result.Nodes.getNodeAs<Expr>("arg"); const auto *Member = Result.Nodes.getNodeAs<MemberExpr>("member"); - bool Arrow = Member->isArrow(); - // Replace the "call" node with the "arg" node, prefixed with '*' - // if the call was using '->' rather than '.'. - std::string ArgText = - Arrow ? formatDereference(Result, *Arg) - : tooling::fixit::getText(*Arg, *Result.Context).str(); - if (ArgText.empty()) - return; - - diag(Call->getBeginLoc(), "redundant call to %0") - << Member->getMemberDecl() - << FixItHint::CreateReplacement(Call->getSourceRange(), ArgText); + + auto D = diag(Call->getBeginLoc(), "redundant call to %0") + << Member->getMemberDecl() + << FixItHint::CreateRemoval(CharSourceRange::getTokenRange( + {Member->getOperatorLoc(), Call->getEndLoc()})); + + if (Member->isArrow()) { + if (needParensAfterUnaryOperator(*Arg)) + D << FixItHint::CreateInsertion(Arg->getBeginLoc(), "*(") + << FixItHint::CreateInsertion(Member->getOperatorLoc(), ")"); + else + D << FixItHint::CreateInsertion(Arg->getBeginLoc(), "*"); + } } } // namespace readability
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits