llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang-tools-extra Author: Nathan James (njames93) <details> <summary>Changes</summary> Add `UseReversePipe` option to (boost|modernize)-use-ranges checks. This controls whether to create a reverse view using function syntax (`reverse(Range)`) or pipe syntax (`Range | reverse`) --- Patch is 28.59 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/98696.diff 15 Files Affected: - (modified) clang-tools-extra/clang-tidy/boost/UseRangesCheck.cpp (+9-6) - (modified) clang-tools-extra/clang-tidy/boost/UseRangesCheck.h (+1) - (modified) clang-tools-extra/clang-tidy/modernize/UseRangesCheck.cpp (+12-1) - (modified) clang-tools-extra/clang-tidy/modernize/UseRangesCheck.h (+6-1) - (modified) clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp (+11-7) - (modified) clang-tools-extra/clang-tidy/utils/UseRangesCheck.h (+1) - (modified) clang-tools-extra/docs/clang-tidy/checks/boost/use-ranges.rst (+19-2) - (modified) clang-tools-extra/docs/clang-tidy/checks/modernize/use-ranges.rst (+17-2) - (added) clang-tools-extra/test/clang-tidy/checkers/boost/Inputs/use-ranges/fake_boost.h (+29) - (added) clang-tools-extra/test/clang-tidy/checkers/boost/Inputs/use-ranges/fake_std.h (+100) - (added) clang-tools-extra/test/clang-tidy/checkers/boost/use-ranges-pipe.cpp (+18) - (modified) clang-tools-extra/test/clang-tidy/checkers/boost/use-ranges.cpp (+4-107) - (added) clang-tools-extra/test/clang-tidy/checkers/modernize/Inputs/use-ranges/fake_std.h (+111) - (added) clang-tools-extra/test/clang-tidy/checkers/modernize/use-ranges-pipe.cpp (+18) - (modified) clang-tools-extra/test/clang-tidy/checkers/modernize/use-ranges.cpp (+6-111) ``````````diff diff --git a/clang-tools-extra/clang-tidy/boost/UseRangesCheck.cpp b/clang-tools-extra/clang-tidy/boost/UseRangesCheck.cpp index 9351a1c90ae5..4022ea0cdaf5 100644 --- a/clang-tools-extra/clang-tidy/boost/UseRangesCheck.cpp +++ b/clang-tools-extra/clang-tidy/boost/UseRangesCheck.cpp @@ -331,12 +331,15 @@ utils::UseRangesCheck::ReplacerMap UseRangesCheck::getReplacerMap() const { UseRangesCheck::UseRangesCheck(StringRef Name, ClangTidyContext *Context) : utils::UseRangesCheck(Name, Context), - IncludeBoostSystem(Options.get("IncludeBoostSystem", true)) {} + IncludeBoostSystem(Options.get("IncludeBoostSystem", true)), + UseReversePipe(Options.get("UseReversePipe", false)) {} void UseRangesCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) { utils::UseRangesCheck::storeOptions(Opts); Options.store(Opts, "IncludeBoostSystem", IncludeBoostSystem); + Options.store(Opts, "UseReversePipe", UseReversePipe); } + DiagnosticBuilder UseRangesCheck::createDiag(const CallExpr &Call) { DiagnosticBuilder D = diag(Call.getBeginLoc(), "use a %0 version of this algorithm"); @@ -362,10 +365,10 @@ UseRangesCheck::getReverseDescriptor() const { {"::boost::rbegin", "::boost::rend"}, {"::boost::const_rbegin", "::boost::const_rend"}, }; - return ReverseIteratorDescriptor{"boost::adaptors::reverse", - IncludeBoostSystem - ? "<boost/range/adaptor/reversed.hpp>" - : "boost/range/adaptor/reversed.hpp", - Refs}; + return ReverseIteratorDescriptor{ + UseReversePipe ? "boost::adaptors::reversed" : "boost::adaptors::reverse", + IncludeBoostSystem ? "<boost/range/adaptor/reversed.hpp>" + : "boost/range/adaptor/reversed.hpp", + Refs, UseReversePipe}; } } // namespace clang::tidy::boost diff --git a/clang-tools-extra/clang-tidy/boost/UseRangesCheck.h b/clang-tools-extra/clang-tidy/boost/UseRangesCheck.h index a59ced12a6c4..b081c4c479b9 100644 --- a/clang-tools-extra/clang-tidy/boost/UseRangesCheck.h +++ b/clang-tools-extra/clang-tidy/boost/UseRangesCheck.h @@ -36,6 +36,7 @@ class UseRangesCheck : public utils::UseRangesCheck { private: bool IncludeBoostSystem; + bool UseReversePipe; }; } // namespace clang::tidy::boost diff --git a/clang-tools-extra/clang-tidy/modernize/UseRangesCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseRangesCheck.cpp index 5c7b315f4317..b0a31ad53be3 100644 --- a/clang-tools-extra/clang-tidy/modernize/UseRangesCheck.cpp +++ b/clang-tools-extra/clang-tidy/modernize/UseRangesCheck.cpp @@ -166,6 +166,15 @@ utils::UseRangesCheck::ReplacerMap UseRangesCheck::getReplacerMap() const { return Result; } +UseRangesCheck::UseRangesCheck(StringRef Name, ClangTidyContext *Context) + : utils::UseRangesCheck(Name, Context), + UseReversePipe(Options.get("UseReversePipe", false)) {} + +void UseRangesCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) { + utils::UseRangesCheck::storeOptions(Opts); + Options.store(Opts, "UseReversePipe", UseReversePipe); +} + bool UseRangesCheck::isLanguageVersionSupported( const LangOptions &LangOpts) const { return LangOpts.CPlusPlus20; @@ -180,6 +189,8 @@ std::optional<UseRangesCheck::ReverseIteratorDescriptor> UseRangesCheck::getReverseDescriptor() const { static const std::pair<StringRef, StringRef> Refs[] = { {"::std::rbegin", "::std::rend"}, {"::std::crbegin", "::std::crend"}}; - return ReverseIteratorDescriptor{"std::views::reverse", "<ranges>", Refs}; + return ReverseIteratorDescriptor{UseReversePipe ? "std::views::reverse" + : "std::ranges::reverse_view", + "<ranges>", Refs, UseReversePipe}; } } // namespace clang::tidy::modernize diff --git a/clang-tools-extra/clang-tidy/modernize/UseRangesCheck.h b/clang-tools-extra/clang-tidy/modernize/UseRangesCheck.h index 2f7613dd1cd2..2f4cace653cf 100644 --- a/clang-tools-extra/clang-tidy/modernize/UseRangesCheck.h +++ b/clang-tools-extra/clang-tidy/modernize/UseRangesCheck.h @@ -20,7 +20,9 @@ namespace clang::tidy::modernize { /// http://clang.llvm.org/extra/clang-tidy/checks/modernize/use-ranges.html class UseRangesCheck : public utils::UseRangesCheck { public: - using utils::UseRangesCheck::UseRangesCheck; + UseRangesCheck(StringRef CheckName, ClangTidyContext *Context); + + void storeOptions(ClangTidyOptions::OptionMap &Options) override; ReplacerMap getReplacerMap() const override; @@ -31,6 +33,9 @@ class UseRangesCheck : public utils::UseRangesCheck { getReverseDescriptor() const override; bool isLanguageVersionSupported(const LangOptions &LangOpts) const override; + +private: + bool UseReversePipe; }; } // namespace clang::tidy::modernize diff --git a/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp b/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp index 9c59e4651953..e2daa5010e2a 100644 --- a/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp +++ b/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp @@ -242,16 +242,20 @@ void UseRangesCheck::check(const MatchFinder::MatchResult &Result) { Diag << Inserter.createIncludeInsertion( Result.SourceManager->getFileID(Call->getBeginLoc()), *ReverseDescriptor->ReverseHeader); + StringRef ArgText = Lexer::getSourceText( + CharSourceRange::getTokenRange(ArgExpr->getSourceRange()), + Result.Context->getSourceManager(), Result.Context->getLangOpts()); + SmallString<128> ReplaceText; + if (ReverseDescriptor->IsPipeSyntax) + ReplaceText.assign( + {ArgText, " | ", ReverseDescriptor->ReverseAdaptorName}); + else + ReplaceText.assign( + {ReverseDescriptor->ReverseAdaptorName, "(", ArgText, ")"}); Diag << FixItHint::CreateReplacement( Call->getArg(Replace == Indexes::Second ? Second : First) ->getSourceRange(), - SmallString<128>{ - ReverseDescriptor->ReverseAdaptorName, "(", - Lexer::getSourceText( - CharSourceRange::getTokenRange(ArgExpr->getSourceRange()), - Result.Context->getSourceManager(), - Result.Context->getLangOpts()), - ")"}); + ReplaceText); } ToRemove.push_back(Replace == Indexes::Second ? First : Second); } diff --git a/clang-tools-extra/clang-tidy/utils/UseRangesCheck.h b/clang-tools-extra/clang-tidy/utils/UseRangesCheck.h index 8227d8f7bbbd..927e9694b0ec 100644 --- a/clang-tools-extra/clang-tidy/utils/UseRangesCheck.h +++ b/clang-tools-extra/clang-tidy/utils/UseRangesCheck.h @@ -38,6 +38,7 @@ class UseRangesCheck : public ClangTidyCheck { StringRef ReverseAdaptorName; std::optional<StringRef> ReverseHeader; ArrayRef<std::pair<StringRef, StringRef>> FreeReverseNames; + bool IsPipeSyntax = false; }; class Replacer : public llvm::RefCountedBase<Replacer> { diff --git a/clang-tools-extra/docs/clang-tidy/checks/boost/use-ranges.rst b/clang-tools-extra/docs/clang-tidy/checks/boost/use-ranges.rst index 39be52fdcf7b..01a3fdf10b62 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/boost/use-ranges.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/boost/use-ranges.rst @@ -154,8 +154,8 @@ Transforms to: .. code-block:: c++ - auto AreSame = std::equal(boost::adaptors::reverse(Items1), - boost::adaptors::reverse(Items2)); + auto AreSame = boost::range::equal(boost::adaptors::reverse(Items1), + boost::adaptors::reverse(Items2)); Options ------- @@ -170,3 +170,20 @@ Options If `true` (default value) the boost headers are included as system headers with angle brackets (`#include <boost.hpp>`), otherwise quotes are used (`#include "boost.hpp"`). + +.. option:: UseReversePipe + + When `true` (default `false`), Fixes which involve reverse ranges will use the + pipe adaptor syntax instead of the function syntax + + .. code-block:: c++ + + std::find(Items.rbegin(), Items.rend(), 0); + + Transforms to: + + .. code-block:: c++ + + boost::range::find(Items | boost::adaptors::reversed, 0); + + diff --git a/clang-tools-extra/docs/clang-tidy/checks/modernize/use-ranges.rst b/clang-tools-extra/docs/clang-tidy/checks/modernize/use-ranges.rst index 86af6b0eeb8e..ff59c90c6eb6 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/modernize/use-ranges.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/modernize/use-ranges.rst @@ -116,8 +116,8 @@ Transforms to: .. code-block:: c++ - auto AreSame = std::equal(std::views::reverse(Items1), - std::views::reverse(Items2)); + auto AreSame = std::ranges::equal(std::ranges::reverse_view(Items1), + std::ranges::reverse_view(Items2)); Options ------- @@ -127,3 +127,18 @@ Options A string specifying which include-style is used, `llvm` or `google`. Default is `llvm`. +.. option:: UseReversePipe + + When `true` (default `false`), Fixes which involve reverse ranges will use the + pipe adaptor syntax instead of the function syntax + + .. code-block:: c++ + + std::find(Items.rbegin(), Items.rend(), 0); + + Transforms to: + + .. code-block:: c++ + + std::ranges::find(Items | std::views::reverse, 0); + diff --git a/clang-tools-extra/test/clang-tidy/checkers/boost/Inputs/use-ranges/fake_boost.h b/clang-tools-extra/test/clang-tidy/checkers/boost/Inputs/use-ranges/fake_boost.h new file mode 100644 index 000000000000..3664367a6011 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/boost/Inputs/use-ranges/fake_boost.h @@ -0,0 +1,29 @@ +#ifndef USE_RANGES_FAKE_BOOST_H +#define USE_RANGES_FAKE_BOOST_H + +namespace boost { +namespace range_adl_barrier { + +template <typename T> void *begin(T &); +template <typename T> void *end(T &); +template <typename T> void *const_begin(const T &); +template <typename T> void *const_end(const T &); +} // namespace range_adl_barrier + +using namespace range_adl_barrier; + +template <typename T> void *rbegin(T &); +template <typename T> void *rend(T &); + +template <typename T> void *const_rbegin(T &); +template <typename T> void *const_rend(T &); +namespace algorithm { + +template <class InputIterator, class T, class BinaryOperation> +T reduce(InputIterator first, InputIterator last, T init, BinaryOperation bOp) { + return init; +} +} // namespace algorithm +} // namespace boost + +#endif // USE_RANGES_FAKE_BOOST_H diff --git a/clang-tools-extra/test/clang-tidy/checkers/boost/Inputs/use-ranges/fake_std.h b/clang-tools-extra/test/clang-tidy/checkers/boost/Inputs/use-ranges/fake_std.h new file mode 100644 index 000000000000..13910c02465d --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/boost/Inputs/use-ranges/fake_std.h @@ -0,0 +1,100 @@ +#ifndef USE_RANGES_FAKE_STD_H +#define USE_RANGES_FAKE_STD_H +namespace std { + +template <typename T> class vector { +public: +public: + using iterator = T *; + using const_iterator = const T *; + using reverse_iterator = T*; + using reverse_const_iterator = const T*; + + constexpr const_iterator begin() const; + constexpr const_iterator end() const; + constexpr const_iterator cbegin() const; + constexpr const_iterator cend() const; + constexpr iterator begin(); + constexpr iterator end(); + constexpr reverse_const_iterator rbegin() const; + constexpr reverse_const_iterator rend() const; + constexpr reverse_const_iterator crbegin() const; + constexpr reverse_const_iterator crend() const; + constexpr reverse_iterator rbegin(); + constexpr reverse_iterator rend(); +}; + +template <typename Container> constexpr auto begin(const Container &Cont) { + return Cont.begin(); +} + +template <typename Container> constexpr auto begin(Container &Cont) { + return Cont.begin(); +} + +template <typename Container> constexpr auto end(const Container &Cont) { + return Cont.end(); +} + +template <typename Container> constexpr auto end(Container &Cont) { + return Cont.end(); +} + +template <typename Container> constexpr auto cbegin(const Container &Cont) { + return Cont.cbegin(); +} + +template <typename Container> constexpr auto cend(const Container &Cont) { + return Cont.cend(); +} +// Find +template< class InputIt, class T > +InputIt find(InputIt first, InputIt last, const T& value); + +template <typename Iter> void reverse(Iter begin, Iter end); + +template <class InputIt1, class InputIt2> +bool includes(InputIt1 first1, InputIt1 last1, InputIt2 first2, InputIt2 last2); + +template <class ForwardIt1, class ForwardIt2> +bool is_permutation(ForwardIt1 first1, ForwardIt1 last1, ForwardIt2 first2, + ForwardIt2 last2); + +template <class BidirIt> +bool next_permutation(BidirIt first, BidirIt last); + +inline namespace inline_test{ + +template <class ForwardIt1, class ForwardIt2> +bool equal(ForwardIt1 first1, ForwardIt1 last1, + ForwardIt2 first2, ForwardIt2 last2); + +template <class RandomIt> +void push_heap(RandomIt first, RandomIt last); + +template <class InputIt, class OutputIt, class UnaryPred> +OutputIt copy_if(InputIt first, InputIt last, OutputIt d_first, UnaryPred pred); + +template <class ForwardIt> +ForwardIt is_sorted_until(ForwardIt first, ForwardIt last); + +template <class InputIt> +void reduce(InputIt first, InputIt last); + +template <class InputIt, class T> +T reduce(InputIt first, InputIt last, T init); + +template <class InputIt, class T, class BinaryOp> +T reduce(InputIt first, InputIt last, T init, BinaryOp op) { + // Need a definition to suppress undefined_internal_type when invoked with lambda + return init; +} + +template <class InputIt, class T> +T accumulate(InputIt first, InputIt last, T init); + +} // namespace inline_test + +} // namespace std + +#endif // USE_RANGES_FAKE_STD_H diff --git a/clang-tools-extra/test/clang-tidy/checkers/boost/use-ranges-pipe.cpp b/clang-tools-extra/test/clang-tidy/checkers/boost/use-ranges-pipe.cpp new file mode 100644 index 000000000000..c0ce37484009 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/boost/use-ranges-pipe.cpp @@ -0,0 +1,18 @@ +// RUN: %check_clang_tidy -std=c++14 %s boost-use-ranges %t -check-suffixes=,PIPE \ +// RUN: -config="{CheckOptions: { \ +// RUN: boost-use-ranges.UseReversePipe: true }}" -- -I %S/Inputs/use-ranges/ +// RUN: %check_clang_tidy -std=c++14 %s boost-use-ranges %t -check-suffixes=,NOPIPE -- -I %S/Inputs/use-ranges/ + +// CHECK-FIXES: #include <boost/algorithm/cxx11/is_sorted.hpp> +// CHECK-FIXES: #include <boost/range/adaptor/reversed.hpp> + +#include "fake_std.h" + +void stdLib() { + std::vector<int> I; + std::is_sorted_until(I.rbegin(), I.rend()); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use a boost version of this algorithm + // CHECK-FIXES-NOPIPE: boost::algorithm::is_sorted_until(boost::adaptors::reverse(I)); + // CHECK-FIXES-PIPE: boost::algorithm::is_sorted_until(I | boost::adaptors::reversed); + +} diff --git a/clang-tools-extra/test/clang-tidy/checkers/boost/use-ranges.cpp b/clang-tools-extra/test/clang-tidy/checkers/boost/use-ranges.cpp index 3f3d6f1abec9..06e70267da83 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/boost/use-ranges.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/boost/use-ranges.cpp @@ -1,5 +1,5 @@ -// RUN: %check_clang_tidy -std=c++14 %s boost-use-ranges %t -// RUN: %check_clang_tidy -std=c++17 %s boost-use-ranges %t -check-suffixes=,CPP17 +// RUN: %check_clang_tidy -std=c++14 %s boost-use-ranges %t -- -- -I %S/Inputs/use-ranges/ +// RUN: %check_clang_tidy -std=c++17 %s boost-use-ranges %t -check-suffixes=,CPP17 -- -I %S/Inputs/use-ranges/ // CHECK-FIXES: #include <boost/range/algorithm/find.hpp> // CHECK-FIXES: #include <boost/range/algorithm/reverse.hpp> @@ -13,111 +13,8 @@ // CHECK-FIXES: #include <boost/range/adaptor/reversed.hpp> // CHECK-FIXES: #include <boost/range/numeric.hpp> -namespace std { - -template <typename T> class vector { -public: - using iterator = T *; - using const_iterator = const T *; - constexpr const_iterator begin() const; - constexpr const_iterator end() const; - constexpr const_iterator cbegin() const; - constexpr const_iterator cend() const; - constexpr iterator begin(); - constexpr iterator end(); -}; - -template <typename Container> constexpr auto begin(const Container &Cont) { - return Cont.begin(); -} - -template <typename Container> constexpr auto begin(Container &Cont) { - return Cont.begin(); -} - -template <typename Container> constexpr auto end(const Container &Cont) { - return Cont.end(); -} - -template <typename Container> constexpr auto end(Container &Cont) { - return Cont.end(); -} - -template <typename Container> constexpr auto cbegin(const Container &Cont) { - return Cont.cbegin(); -} - -template <typename Container> constexpr auto cend(const Container &Cont) { - return Cont.cend(); -} -// Find -template< class InputIt, class T > -InputIt find(InputIt first, InputIt last, const T& value); - -template <typename Iter> void reverse(Iter begin, Iter end); - -template <class InputIt1, class InputIt2> -bool includes(InputIt1 first1, InputIt1 last1, InputIt2 first2, InputIt2 last2); - -template <class ForwardIt1, class ForwardIt2> -bool is_permutation(ForwardIt1 first1, ForwardIt1 last1, ForwardIt2 first2, - ForwardIt2 last2); - -template <class BidirIt> -bool next_permutation(BidirIt first, BidirIt last); - -template <class ForwardIt1, class ForwardIt2> -bool equal(ForwardIt1 first1, ForwardIt1 last1, - ForwardIt2 first2, ForwardIt2 last2); - -template <class RandomIt> -void push_heap(RandomIt first, RandomIt last); - -template <class InputIt, class OutputIt, class UnaryPred> -OutputIt copy_if(InputIt first, InputIt last, OutputIt d_first, UnaryPred pred); - -template <class ForwardIt> -ForwardIt is_sorted_until(ForwardIt first, ForwardIt last); - -template <class InputIt> -void reduce(InputIt first, InputIt last); - -template <class InputIt, class T> -T reduce(InputIt first, InputIt last, T init); - -template <class InputIt, class T, class BinaryOp> -T reduce(InputIt first, InputIt last, T init, BinaryOp op) { - // Need a definition to suppress undefined_internal_type when invoked with lambda - return init; -} - -template <class InputIt, class T> -T accumulate(InputIt first, InputIt last, T init); - -} // namespace std - -namespace boost { -namespace range_adl_barrier { -template <typename T> void *begin(T &); -template <typename T> void *end(T &); -template <typename T> void *const_begin(const T &); -template <typename T> void *const_end(const T &); -} // namespace range_adl_barrier -using namespace range_adl_barrier; - -template <typename T> void *rbegin(T &); -template <typename T> void *rend(T &); - -template <typename T> void *const_rbegin(T &); -template <typename T> void *const_rend(T &); -namespace algorithm { - -template <class InputIterator, class T, class BinaryOperation> -T reduce(InputIterator first, InputIterator last, T init, BinaryOperation bOp) { - return init; -} -} // namespace algorithm -} // namespace boost +#include "fake_boost.h" +#include "fake_std.h" bool returnTrue(int val) { return true; diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/Inputs/use-ranges/fake_std.h b/clang-tools-extra/test/clang-tidy/checkers/modernize/Inputs/use-ranges/fake_std.h new file mode 100644 index 000000000000..987ee4e35b3b --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/Inputs/use-ranges/fake_std.h @@ -0,0 +1,111 @@ +#ifndef USE_RANGES_FAKE_STD_H +#define USE_RANGES_FAKE_STD_H + +namespace std { + +template <typename T> class vector { +public: + using iterator = T *; + using const_iterator = const T *; + using reverse_iterator = T*; + using reverse_const_iterator = const T*; + + constexpr const_iterator begin() const; + constexpr const_iterator end() const; + constexpr const_iterator cbegin() const; + constexpr const_iterator cend() const; + constexpr iterator begin(); + constexpr iterator end(); + constexpr reverse_const_iterator rbegin() const; + constexpr reverse_const_iterator rend() const; + constexpr reverse_const_iterator crbegin() const; + constexpr reverse_const_iterat... [truncated] `````````` </details> https://github.com/llvm/llvm-project/pull/98696 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits