Skip to content

Commit

Permalink
Allow specifying pipe syntax for use-ranges checks
Browse files Browse the repository at this point in the history
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`)
  • Loading branch information
njames93 committed Jul 12, 2024
1 parent 1ccd875 commit c3431c0
Show file tree
Hide file tree
Showing 15 changed files with 358 additions and 237 deletions.
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.

.. 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

0 comments on commit c3431c0

Please sign in to comment.