Skip to content

Allow specifying pipe syntax for use-ranges checks #98696

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jul 15, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 9 additions & 6 deletions clang-tools-extra/clang-tidy/boost/UseRangesCheck.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand All @@ -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
1 change: 1 addition & 0 deletions clang-tools-extra/clang-tidy/boost/UseRangesCheck.h
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ class UseRangesCheck : public utils::UseRangesCheck {

private:
bool IncludeBoostSystem;
bool UseReversePipe;
};

} // namespace clang::tidy::boost
Expand Down
13 changes: 12 additions & 1 deletion clang-tools-extra/clang-tidy/modernize/UseRangesCheck.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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
7 changes: 6 additions & 1 deletion clang-tools-extra/clang-tidy/modernize/UseRangesCheck.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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
Expand Down
18 changes: 11 additions & 7 deletions clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
1 change: 1 addition & 0 deletions clang-tools-extra/clang-tidy/utils/UseRangesCheck.h
Original file line number Diff line number Diff line change
Expand Up @@ -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> {
Expand Down
19 changes: 17 additions & 2 deletions clang-tools-extra/docs/clang-tidy/checks/boost/use-ranges.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
-------
Expand All @@ -170,3 +170,18 @@ 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.
Comment on lines +176 to +177
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: most of options put default value at the end of description


.. code-block:: c++

std::find(Items.rbegin(), Items.rend(), 0);

Transforms to:

.. code-block:: c++

boost::range::find(Items | boost::adaptors::reversed, 0);
18 changes: 16 additions & 2 deletions clang-tools-extra/docs/clang-tidy/checks/modernize/use-ranges.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
-------
Expand All @@ -127,3 +127,17 @@ 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);
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -0,0 +1,99 @@
#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_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
Original file line number Diff line number Diff line change
@@ -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);

}
Loading
Loading