llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang-tools-extra Author: mitchell (zeyi2) <details> <summary>Changes</summary> Avoid emitting warnings in pre-C++20 mode for range-based loops over temporary range expressions. They can't be safely rewritten to `std::any_of`/`std::all_of` by reusing the range expression directly. Closes https://github.com/llvm/llvm-project/issues/185593 --- Full diff: https://github.com/llvm/llvm-project/pull/185791.diff 4 Files Affected: - (modified) clang-tools-extra/clang-tidy/readability/UseAnyOfAllOfCheck.cpp (+15) - (modified) clang-tools-extra/docs/ReleaseNotes.rst (+4) - (modified) clang-tools-extra/test/clang-tidy/checkers/readability/use-anyofallof-cpp20.cpp (+20) - (modified) clang-tools-extra/test/clang-tidy/checkers/readability/use-anyofallof.cpp (+35) ``````````diff diff --git a/clang-tools-extra/clang-tidy/readability/UseAnyOfAllOfCheck.cpp b/clang-tools-extra/clang-tidy/readability/UseAnyOfAllOfCheck.cpp index 63649150b17e3..28be6b2891025 100644 --- a/clang-tools-extra/clang-tidy/readability/UseAnyOfAllOfCheck.cpp +++ b/clang-tools-extra/clang-tidy/readability/UseAnyOfAllOfCheck.cpp @@ -84,11 +84,22 @@ static bool isViableLoop(const CXXForRangeStmt &S, ASTContext &Context) { }); } +static bool isIteratingOverTemporary(const Expr *Init) { + if (const auto *EWC = dyn_cast<ExprWithCleanups>(Init)) + Init = EWC->getSubExpr(); + Init = Init->IgnoreParenImpCasts(); + return isa<MaterializeTemporaryExpr>(Init) || Init->isPRValue(); +} + void UseAnyOfAllOfCheck::check(const MatchFinder::MatchResult &Result) { if (const auto *S = Result.Nodes.getNodeAs<CXXForRangeStmt>("any_of_loop")) { if (!isViableLoop(*S, *Result.Context)) return; + if (!getLangOpts().CPlusPlus20 && + isIteratingOverTemporary(S->getRangeInit())) + return; + diag(S->getForLoc(), "replace loop by 'std%select{|::ranges}0::any_of()'") << getLangOpts().CPlusPlus20; } else if (const auto *S = @@ -96,6 +107,10 @@ void UseAnyOfAllOfCheck::check(const MatchFinder::MatchResult &Result) { if (!isViableLoop(*S, *Result.Context)) return; + if (!getLangOpts().CPlusPlus20 && + isIteratingOverTemporary(S->getRangeInit())) + return; + diag(S->getForLoc(), "replace loop by 'std%select{|::ranges}0::all_of()'") << getLangOpts().CPlusPlus20; } diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 66290bc80f754..90f95e3b24a71 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -336,6 +336,10 @@ Changes in existing checks <clang-tidy/checks/readability/suspicious-call-argument>` check by avoiding a crash from invalid ``Abbreviations`` option. +- Improved :doc:`readability-use-anyofallof + <clang-tidy/checks/readability/use-anyofallof>` check by avoiding false + positives in pre-C++20 mode when iterating over temporary range expressions. + Removed checks ^^^^^^^^^^^^^^ diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability/use-anyofallof-cpp20.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability/use-anyofallof-cpp20.cpp index c13ae002ae5b6..83f51995ece5f 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/readability/use-anyofallof-cpp20.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/readability/use-anyofallof-cpp20.cpp @@ -1,5 +1,7 @@ // RUN: %check_clang_tidy -std=c++20-or-later %s readability-use-anyofallof %t +#include <vector> + bool good_any_of() { int v[] = {1, 2, 3}; // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: replace loop by 'std::ranges::any_of()' @@ -17,3 +19,21 @@ bool good_all_of() { return false; return true; } + +std::vector<int> get_dummy_vec(); + +bool good_any_of_temporary_vector() { + // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: replace loop by 'std::ranges::any_of()' + for (int i : get_dummy_vec()) + if (i) + return true; + return false; +} + +bool good_all_of_temporary_vector() { + // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: replace loop by 'std::ranges::all_of()' + for (int i : get_dummy_vec()) + if (i) + return false; + return true; +} diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability/use-anyofallof.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability/use-anyofallof.cpp index 7f8f16488d37a..e6f8830686ef3 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/readability/use-anyofallof.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/readability/use-anyofallof.cpp @@ -1,6 +1,9 @@ // RUN: %check_clang_tidy -std=c++14,c++17 %s readability-use-anyofallof %t -- -- -fexceptions // FIXME: Fix the checker to work in C++20 mode. +#include <utility> +#include <vector> + bool good_any_of() { int v[] = {1, 2, 3}; // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: replace loop by 'std::any_of()' [readability-use-anyofallof] @@ -182,3 +185,35 @@ bool good_all_of() { return false; return true; } + +std::vector<int> get_dummy_vec(); + +bool good_any_of_temporary_vector() { + for (int i : get_dummy_vec()) + if (i) + return true; + return false; +} + +bool good_all_of_temporary_vector() { + for (int i : get_dummy_vec()) + if (i) + return false; + return true; +} + +bool good_xvalue(std::vector<int>& v) { + // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: replace loop by 'std::any_of()' [readability-use-anyofallof] + for (int i : std::move(v)) + if (i) + return true; + return false; +} + +bool good_xvalue_all_of(std::vector<int>& v) { + // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: replace loop by 'std::all_of()' [readability-use-anyofallof] + for (int i : std::move(v)) + if (i) + return false; + return true; +} `````````` </details> https://github.com/llvm/llvm-project/pull/185791 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
