[clang-tools-extra] 8a548bc - [clang-tidy] modernize-loop-convert reverse iteration support
Nathan James via cfe-commits
cfe-commits at lists.llvm.org
Fri Oct 16 06:16:42 PDT 2020
Author: Nathan James
Date: 2020-10-16T14:16:30+01:00
New Revision: 8a548bc203cfb0b2f830959cb7ec578c25512025
URL: https://github.com/llvm/llvm-project/commit/8a548bc203cfb0b2f830959cb7ec578c25512025
DIFF: https://github.com/llvm/llvm-project/commit/8a548bc203cfb0b2f830959cb7ec578c25512025.diff
LOG: [clang-tidy] modernize-loop-convert reverse iteration support
Enables support for transforming loops of the form
```
for (auto I = Cont.rbegin(), E = Cont.rend(); I != E;++I)
```
This is done automatically in C++20 mode using `std::ranges::reverse_view` but there are options to specify a different function to reverse iterator over a container.
This is the first step, down the line I'd like to possibly extend this support for array based loops
```
for (unsigned I = Arr.size() - 1;I >=0;--I) Arr[I]...
```
Currently if you pass a reversing function with no header in the options it will just assume that the function exists, however as we have the ASTContext it may be as wise to check before applying, or at least lower the confidence level if we can't find it.
Reviewed By: alexfh
Differential Revision: https://reviews.llvm.org/D82089
Added:
clang-tools-extra/test/clang-tidy/checkers/modernize-loop-convert-reverse.cpp
Modified:
clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp
clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.h
clang-tools-extra/clang-tidy/modernize/LoopConvertUtils.h
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/modernize-loop-convert.rst
Removed:
################################################################################
diff --git a/clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp b/clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp
index b90af1521baf..20791c60339a 100644
--- a/clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp
@@ -19,6 +19,8 @@
#include "llvm/ADT/StringRef.h"
#include "llvm/ADT/StringSwitch.h"
#include "llvm/Support/Casting.h"
+#include "llvm/Support/WithColor.h"
+#include "llvm/Support/raw_ostream.h"
#include <cassert>
#include <cstring>
#include <utility>
@@ -57,6 +59,7 @@ namespace modernize {
static const char LoopNameArray[] = "forLoopArray";
static const char LoopNameIterator[] = "forLoopIterator";
+static const char LoopNameReverseIterator[] = "forLoopReverseIterator";
static const char LoopNamePseudoArray[] = "forLoopPseudoArray";
static const char ConditionBoundName[] = "conditionBound";
static const char ConditionVarName[] = "conditionVar";
@@ -68,7 +71,6 @@ static const char ConditionEndVarName[] = "conditionEndVar";
static const char EndVarName[] = "endVar";
static const char DerefByValueResultName[] = "derefByValueResult";
static const char DerefByRefResultName[] = "derefByRefResult";
-
// shared matchers
static const TypeMatcher AnyType() { return anything(); }
@@ -150,10 +152,17 @@ StatementMatcher makeArrayLoopMatcher() {
/// - The iterator variables 'it', 'f', and 'h' are the same.
/// - The two containers on which 'begin' and 'end' are called are the same.
/// - If the end iterator variable 'g' is defined, it is the same as 'f'.
-StatementMatcher makeIteratorLoopMatcher() {
+StatementMatcher makeIteratorLoopMatcher(bool IsReverse) {
+
+ auto BeginNameMatcher = IsReverse ? hasAnyName("rbegin", "crbegin")
+ : hasAnyName("begin", "cbegin");
+
+ auto EndNameMatcher =
+ IsReverse ? hasAnyName("rend", "crend") : hasAnyName("end", "cend");
+
StatementMatcher BeginCallMatcher =
cxxMemberCallExpr(argumentCountIs(0),
- callee(cxxMethodDecl(hasAnyName("begin", "cbegin"))))
+ callee(cxxMethodDecl(BeginNameMatcher)))
.bind(BeginCallName);
DeclarationMatcher InitDeclMatcher =
@@ -167,7 +176,7 @@ StatementMatcher makeIteratorLoopMatcher() {
varDecl(hasInitializer(anything())).bind(EndVarName);
StatementMatcher EndCallMatcher = cxxMemberCallExpr(
- argumentCountIs(0), callee(cxxMethodDecl(hasAnyName("end", "cend"))));
+ argumentCountIs(0), callee(cxxMethodDecl(EndNameMatcher)));
StatementMatcher IteratorBoundMatcher =
expr(anyOf(ignoringParenImpCasts(
@@ -225,7 +234,7 @@ StatementMatcher makeIteratorLoopMatcher() {
hasArgument(
0, declRefExpr(to(varDecl(TestDerefReturnsByValue)
.bind(IncrementVarName))))))))
- .bind(LoopNameIterator);
+ .bind(IsReverse ? LoopNameReverseIterator : LoopNameIterator);
}
/// The matcher used for array-like containers (pseudoarrays).
@@ -325,7 +334,7 @@ StatementMatcher makePseudoArrayLoopMatcher() {
/// the output parameter isArrow is set to indicate whether the initialization
/// is called via . or ->.
static const Expr *getContainerFromBeginEndCall(const Expr *Init, bool IsBegin,
- bool *IsArrow) {
+ bool *IsArrow, bool IsReverse) {
// FIXME: Maybe allow declaration/initialization outside of the for loop.
const auto *TheCall =
dyn_cast_or_null<CXXMemberCallExpr>(digThroughConstructors(Init));
@@ -336,9 +345,11 @@ static const Expr *getContainerFromBeginEndCall(const Expr *Init, bool IsBegin,
if (!Member)
return nullptr;
StringRef Name = Member->getMemberDecl()->getName();
- StringRef TargetName = IsBegin ? "begin" : "end";
- StringRef ConstTargetName = IsBegin ? "cbegin" : "cend";
- if (Name != TargetName && Name != ConstTargetName)
+ if (!Name.consume_back(IsBegin ? "begin" : "end"))
+ return nullptr;
+ if (IsReverse && !Name.consume_back("r"))
+ return nullptr;
+ if (!Name.empty() && !Name.equals("c"))
return nullptr;
const Expr *SourceExpr = Member->getBase();
@@ -356,18 +367,19 @@ static const Expr *getContainerFromBeginEndCall(const Expr *Init, bool IsBegin,
/// must be a member.
static const Expr *findContainer(ASTContext *Context, const Expr *BeginExpr,
const Expr *EndExpr,
- bool *ContainerNeedsDereference) {
+ bool *ContainerNeedsDereference,
+ bool IsReverse) {
// Now that we know the loop variable and test expression, make sure they are
// valid.
bool BeginIsArrow = false;
bool EndIsArrow = false;
- const Expr *BeginContainerExpr =
- getContainerFromBeginEndCall(BeginExpr, /*IsBegin=*/true, &BeginIsArrow);
+ const Expr *BeginContainerExpr = getContainerFromBeginEndCall(
+ BeginExpr, /*IsBegin=*/true, &BeginIsArrow, IsReverse);
if (!BeginContainerExpr)
return nullptr;
- const Expr *EndContainerExpr =
- getContainerFromBeginEndCall(EndExpr, /*IsBegin=*/false, &EndIsArrow);
+ const Expr *EndContainerExpr = getContainerFromBeginEndCall(
+ EndExpr, /*IsBegin=*/false, &EndIsArrow, IsReverse);
// Disallow loops that try evil things like this (note the dot and arrow):
// for (IteratorType It = Obj.begin(), E = Obj->end(); It != E; ++It) { }
if (!EndContainerExpr || BeginIsArrow != EndIsArrow ||
@@ -475,27 +487,59 @@ static bool containerIsConst(const Expr *ContainerExpr, bool Dereference) {
LoopConvertCheck::RangeDescriptor::RangeDescriptor()
: ContainerNeedsDereference(false), DerefByConstRef(false),
- DerefByValue(false) {}
+ DerefByValue(false), NeedsReverseCall(false) {}
LoopConvertCheck::LoopConvertCheck(StringRef Name, ClangTidyContext *Context)
: ClangTidyCheck(Name, Context), TUInfo(new TUTrackingInfo),
MaxCopySize(Options.get("MaxCopySize", 16ULL)),
MinConfidence(Options.get("MinConfidence", Confidence::CL_Reasonable)),
- NamingStyle(Options.get("NamingStyle", VariableNamer::NS_CamelCase)) {}
+ NamingStyle(Options.get("NamingStyle", VariableNamer::NS_CamelCase)),
+ Inserter(Options.getLocalOrGlobal("IncludeStyle",
+ utils::IncludeSorter::IS_LLVM)),
+ UseCxx20IfAvailable(Options.get("UseCxx20ReverseRanges", true)),
+ ReverseFunction(Options.get("MakeReverseRangeFunction", "")),
+ ReverseHeader(Options.get("MakeReverseRangeHeader", "")) {
+
+ if (ReverseFunction.empty() && !ReverseHeader.empty()) {
+ llvm::WithColor::warning()
+ << "modernize-loop-convert: 'MakeReverseRangeHeader' is set but "
+ "'MakeReverseRangeFunction' is not, disabling reverse loop "
+ "transformation\n";
+ UseReverseRanges = false;
+ } else if (ReverseFunction.empty()) {
+ UseReverseRanges = UseCxx20IfAvailable && getLangOpts().CPlusPlus20;
+ } else {
+ UseReverseRanges = true;
+ }
+}
void LoopConvertCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
- Options.store(Opts, "MaxCopySize", std::to_string(MaxCopySize));
+ Options.store(Opts, "MaxCopySize", MaxCopySize);
Options.store(Opts, "MinConfidence", MinConfidence);
Options.store(Opts, "NamingStyle", NamingStyle);
+ Options.store(Opts, "IncludeStyle", Inserter.getStyle());
+ Options.store(Opts, "UseCxx20ReverseRanges", UseCxx20IfAvailable);
+ Options.store(Opts, "MakeReverseRangeFunction", ReverseFunction);
+ Options.store(Opts, "MakeReverseRangeHeader", ReverseHeader);
+}
+
+void LoopConvertCheck::registerPPCallbacks(const SourceManager &SM,
+ Preprocessor *PP,
+ Preprocessor *ModuleExpanderPP) {
+ Inserter.registerPreprocessor(PP);
}
void LoopConvertCheck::registerMatchers(MatchFinder *Finder) {
Finder->addMatcher(traverse(ast_type_traits::TK_AsIs, makeArrayLoopMatcher()),
this);
Finder->addMatcher(
- traverse(ast_type_traits::TK_AsIs, makeIteratorLoopMatcher()), this);
+ traverse(ast_type_traits::TK_AsIs, makeIteratorLoopMatcher(false)), this);
Finder->addMatcher(
traverse(ast_type_traits::TK_AsIs, makePseudoArrayLoopMatcher()), this);
+ if (UseReverseRanges)
+ Finder->addMatcher(
+ traverse(ast_type_traits::TK_AsIs, makeIteratorLoopMatcher(true)),
+ this);
}
/// Given the range of a single declaration, such as:
@@ -646,13 +690,29 @@ void LoopConvertCheck::doConversion(
}
}
- StringRef MaybeDereference = Descriptor.ContainerNeedsDereference ? "*" : "";
- std::string TypeString = Type.getAsString(getLangOpts());
- std::string Range = ("(" + TypeString + " " + VarName + " : " +
- MaybeDereference + Descriptor.ContainerString + ")")
- .str();
+ SmallString<128> Range;
+ llvm::raw_svector_ostream Output(Range);
+ Output << '(';
+ Type.print(Output, getLangOpts());
+ Output << ' ' << VarName << " : ";
+ if (Descriptor.NeedsReverseCall)
+ Output << getReverseFunction() << '(';
+ if (Descriptor.ContainerNeedsDereference)
+ Output << '*';
+ Output << Descriptor.ContainerString;
+ if (Descriptor.NeedsReverseCall)
+ Output << "))";
+ else
+ Output << ')';
FixIts.push_back(FixItHint::CreateReplacement(
CharSourceRange::getTokenRange(ParenRange), Range));
+
+ if (Descriptor.NeedsReverseCall && !getReverseHeader().empty()) {
+ if (Optional<FixItHint> Insertion = Inserter.createIncludeInsertion(
+ Context->getSourceManager().getFileID(Loop->getBeginLoc()),
+ getReverseHeader()))
+ FixIts.push_back(*Insertion);
+ }
diag(Loop->getForLoc(), "use range-based for loop instead") << FixIts;
TUInfo->getGeneratedDecls().insert(make_pair(Loop, VarName));
}
@@ -762,8 +822,9 @@ void LoopConvertCheck::determineRangeDescriptor(
const UsageResult &Usages, RangeDescriptor &Descriptor) {
Descriptor.ContainerString =
std::string(getContainerString(Context, Loop, ContainerExpr));
+ Descriptor.NeedsReverseCall = (FixerKind == LFK_ReverseIterator);
- if (FixerKind == LFK_Iterator)
+ if (FixerKind == LFK_Iterator || FixerKind == LFK_ReverseIterator)
getIteratorLoopQualifiers(Context, Nodes, Descriptor);
else
getArrayLoopQualifiers(Context, Nodes, ContainerExpr, Usages, Descriptor);
@@ -792,7 +853,7 @@ bool LoopConvertCheck::isConvertible(ASTContext *Context,
return false;
// FIXME: Try to put most of this logic inside a matcher.
- if (FixerKind == LFK_Iterator) {
+ if (FixerKind == LFK_Iterator || FixerKind == LFK_ReverseIterator) {
QualType InitVarType = InitVar->getType();
QualType CanonicalInitVarType = InitVarType.getCanonicalType();
@@ -831,6 +892,8 @@ void LoopConvertCheck::check(const MatchFinder::MatchResult &Result) {
FixerKind = LFK_Array;
} else if ((Loop = Nodes.getNodeAs<ForStmt>(LoopNameIterator))) {
FixerKind = LFK_Iterator;
+ } else if ((Loop = Nodes.getNodeAs<ForStmt>(LoopNameReverseIterator))) {
+ FixerKind = LFK_ReverseIterator;
} else {
Loop = Nodes.getNodeAs<ForStmt>(LoopNamePseudoArray);
assert(Loop && "Bad Callback. No for statement");
@@ -858,10 +921,11 @@ void LoopConvertCheck::check(const MatchFinder::MatchResult &Result) {
// With array loops, the container is often discovered during the
// ForLoopIndexUseVisitor traversal.
const Expr *ContainerExpr = nullptr;
- if (FixerKind == LFK_Iterator) {
- ContainerExpr = findContainer(Context, LoopVar->getInit(),
- EndVar ? EndVar->getInit() : EndCall,
- &Descriptor.ContainerNeedsDereference);
+ if (FixerKind == LFK_Iterator || FixerKind == LFK_ReverseIterator) {
+ ContainerExpr = findContainer(
+ Context, LoopVar->getInit(), EndVar ? EndVar->getInit() : EndCall,
+ &Descriptor.ContainerNeedsDereference,
+ /*IsReverse=*/FixerKind == LFK_ReverseIterator);
} else if (FixerKind == LFK_PseudoArray) {
ContainerExpr = EndCall->getImplicitObjectArgument();
Descriptor.ContainerNeedsDereference =
@@ -927,6 +991,23 @@ void LoopConvertCheck::check(const MatchFinder::MatchResult &Result) {
Finder.aliasFromForInit(), Loop, Descriptor);
}
+llvm::StringRef LoopConvertCheck::getReverseFunction() const {
+ if (!ReverseFunction.empty())
+ return ReverseFunction;
+ if (UseReverseRanges)
+ return "std::ranges::reverse_view";
+ return "";
+}
+
+llvm::StringRef LoopConvertCheck::getReverseHeader() const {
+ if (!ReverseHeader.empty())
+ return ReverseHeader;
+ if (UseReverseRanges && ReverseFunction.empty()) {
+ return "<ranges>";
+ }
+ return "";
+}
+
} // namespace modernize
} // namespace tidy
} // namespace clang
diff --git a/clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.h b/clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.h
index d26d87ee6ee5..b1eda4861451 100644
--- a/clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.h
+++ b/clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.h
@@ -10,6 +10,7 @@
#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_LOOP_CONVERT_H
#include "../ClangTidyCheck.h"
+#include "../utils/IncludeInserter.h"
#include "LoopConvertUtils.h"
namespace clang {
@@ -24,6 +25,8 @@ class LoopConvertCheck : public ClangTidyCheck {
}
void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+ void registerPPCallbacks(const SourceManager &SM, Preprocessor *PP,
+ Preprocessor *ModuleExpanderPP) override;
void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
private:
@@ -34,6 +37,7 @@ class LoopConvertCheck : public ClangTidyCheck {
bool DerefByValue;
std::string ContainerString;
QualType ElemType;
+ bool NeedsReverseCall;
};
void getAliasRange(SourceManager &SM, SourceRange &DeclRange);
@@ -67,10 +71,18 @@ class LoopConvertCheck : public ClangTidyCheck {
bool isConvertible(ASTContext *Context, const ast_matchers::BoundNodes &Nodes,
const ForStmt *Loop, LoopFixerKind FixerKind);
+ StringRef getReverseFunction() const;
+ StringRef getReverseHeader() const;
+
std::unique_ptr<TUTrackingInfo> TUInfo;
const unsigned long long MaxCopySize;
const Confidence::Level MinConfidence;
const VariableNamer::NamingStyle NamingStyle;
+ utils::IncludeInserter Inserter;
+ bool UseReverseRanges;
+ const bool UseCxx20IfAvailable;
+ std::string ReverseFunction;
+ std::string ReverseHeader;
};
} // namespace modernize
diff --git a/clang-tools-extra/clang-tidy/modernize/LoopConvertUtils.h b/clang-tools-extra/clang-tidy/modernize/LoopConvertUtils.h
index 4f75c9c7522f..7dd2c8ef107f 100644
--- a/clang-tools-extra/clang-tidy/modernize/LoopConvertUtils.h
+++ b/clang-tools-extra/clang-tidy/modernize/LoopConvertUtils.h
@@ -26,7 +26,12 @@ namespace clang {
namespace tidy {
namespace modernize {
-enum LoopFixerKind { LFK_Array, LFK_Iterator, LFK_PseudoArray };
+enum LoopFixerKind {
+ LFK_Array,
+ LFK_Iterator,
+ LFK_ReverseIterator,
+ LFK_PseudoArray
+};
/// A map used to walk the AST in reverse: maps child Stmt to parent Stmt.
typedef llvm::DenseMap<const clang::Stmt *, const clang::Stmt *> StmtParentMap;
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 1df398c01cab..6e6d46df4933 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -114,6 +114,11 @@ New checks
Changes in existing checks
^^^^^^^^^^^^^^^^^^^^^^^^^^
+- Improved :doc:`modernize-loop-convert
+ <clang-tidy/checks/modernize-loop-convert>` check.
+
+ Now able to transform iterator loops using ``rbegin`` and ``rend`` methods.
+
- Improved :doc:`readability-identifier-naming
<clang-tidy/checks/readability-identifier-naming>` check.
diff --git a/clang-tools-extra/docs/clang-tidy/checks/modernize-loop-convert.rst b/clang-tools-extra/docs/clang-tidy/checks/modernize-loop-convert.rst
index 82b27bbe9020..77f251831627 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/modernize-loop-convert.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/modernize-loop-convert.rst
@@ -118,6 +118,48 @@ After applying the check with minimum confidence level set to `reasonable` (defa
for (auto & elem : v)
cout << elem;
+Reverse Iterator Support
+------------------------
+
+The converter is also capable of transforming iterator loops which use
+``rbegin`` and ``rend`` for looping backwards over a container. Out of the box
+this will automatically happen in C++20 mode using the ``ranges`` library,
+however the check can be configured to work without C++20 by specifying a
+function to reverse a range and optionally the header file where that function
+lives.
+
+.. option:: UseCxx20ReverseRanges
+
+ When set to true convert loops when in C++20 or later mode using
+ ``std::ranges::reverse_view``.
+ Default value is ``true``.
+
+.. option:: MakeReverseRangeFunction
+
+ Specify the function used to reverse an iterator pair, the function should
+ accept a class with ``rbegin`` and ``rend`` methods and return a
+ class with ``begin`` and ``end`` methods methods that call the ``rbegin`` and
+ ``rend`` methods respectively. Common examples are ``ranges::reverse_view``
+ and ``llvm::reverse``.
+ Default value is an empty string.
+
+.. option:: MakeReverseRangeHeader
+
+ Specifies the header file where :option:`MakeReverseRangeFunction` is
+ declared. For the previous examples this option would be set to
+ ``range/v3/view/reverse.hpp`` and ``llvm/ADT/STLExtras.h`` respectively.
+ If this is an empty string and :option:`MakeReverseRangeFunction` is set,
+ the check will proceed on the assumption that the function is already
+ available in the translation unit.
+ This can be wrapped in angle brackets to signify to add the include as a
+ system include.
+ Default value is an empty string.
+
+.. option:: IncludeStyle
+
+ A string specifying which include-style is used, `llvm` or `google`. Default
+ is `llvm`.
+
Limitations
-----------
diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize-loop-convert-reverse.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize-loop-convert-reverse.cpp
new file mode 100644
index 000000000000..63da130e74c6
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/checkers/modernize-loop-convert-reverse.cpp
@@ -0,0 +1,115 @@
+// RUN: %check_clang_tidy -std=c++20 -check-suffixes=,RANGES %s modernize-loop-convert %t
+
+// RUN: %check_clang_tidy -check-suffixes=,CUSTOM,CUSTOM-NO-SYS %s modernize-loop-convert %t -- \
+// RUN: -config="{CheckOptions: [ \
+// RUN: {key: modernize-loop-convert.MakeReverseRangeFunction, value: 'llvm::reverse'}, \
+// RUN: {key: modernize-loop-convert.MakeReverseRangeHeader, value: 'llvm/ADT/STLExtras.h'}]}"
+
+// RUN: %check_clang_tidy -check-suffixes=,CUSTOM,CUSTOM-SYS %s modernize-loop-convert %t -- \
+// RUN: -config="{CheckOptions: [ \
+// RUN: {key: modernize-loop-convert.MakeReverseRangeFunction, value: 'llvm::reverse'}, \
+// RUN: {key: modernize-loop-convert.MakeReverseRangeHeader, value: '<llvm/ADT/STLExtras.h>'}]}"
+
+// RUN: %check_clang_tidy -check-suffixes=,CUSTOM,CUSTOM-NO-HEADER %s modernize-loop-convert %t -- \
+// RUN: -config="{CheckOptions: [ \
+// RUN: {key: modernize-loop-convert.MakeReverseRangeFunction, value: 'llvm::reverse'}]}"
+
+// Ensure the check doesn't transform reverse loops when not in c++20 mode or
+// when UseCxx20ReverseRanges has been disabled
+// RUN: clang-tidy %s -checks=-*,modernize-loop-convert -- -std=c++17 | count 0
+
+// RUN: clang-tidy %s -checks=-*,modernize-loop-convert -config="{CheckOptions: \
+// RUN: [{key: modernize-loop-convert.UseCxx20ReverseRanges, value: 'false'}] \
+// RUN: }" -- -std=c++20 | count 0
+
+// Ensure we get a warning if we supply the header argument without the
+// function argument.
+// RUN: clang-tidy %s -checks=-*,modernize-loop-convert -config="{CheckOptions: [ \
+// RUN: {key: modernize-loop-convert.MakeReverseRangeHeader, value: 'llvm/ADT/STLExtras.h'}]}" \
+// RUN: -- -std=c++17 2>&1 \
+// RUN: | FileCheck %s -check-prefix=CHECK-HEADER-NO-FUNC \
+// RUN: -implicit-check-not="{{warning|error}}:"
+
+// CHECK-HEADER-NO-FUNC: warning: modernize-loop-convert: 'MakeReverseRangeHeader' is set but 'MakeReverseRangeFunction' is not, disabling reverse loop transformation
+
+// Make sure appropiate headers are included
+// CHECK-FIXES-RANGES: #include <ranges>
+// CHECK-FIXES-CUSTOM-NO-SYS: #include "llvm/ADT/STLExtras.h"
+// CHECK-FIXES-CUSTOM-SYS: #include <llvm/ADT/STLExtras.h>
+
+// Make sure no header is included in this example
+// CHECK-FIXES-CUSTOM-NO-HEADER-NOT: #include
+
+template <typename T>
+struct Reversable {
+ using iterator = T *;
+ using const_iterator = const T *;
+
+ iterator begin();
+ iterator end();
+ iterator rbegin();
+ iterator rend();
+
+ const_iterator begin() const;
+ const_iterator end() const;
+ const_iterator rbegin() const;
+ const_iterator rend() const;
+
+ const_iterator cbegin() const;
+ const_iterator cend() const;
+ const_iterator crbegin() const;
+ const_iterator crend() const;
+};
+
+template <typename T>
+void observe(const T &);
+template <typename T>
+void mutate(T &);
+
+void constContainer(const Reversable<int> &Numbers) {
+ for (auto I = Numbers.rbegin(), E = Numbers.rend(); I != E; ++I) {
+ observe(*I);
+ }
+ // CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based for loop instead
+ // CHECK-FIXES-RANGES: for (int Number : std::ranges::reverse_view(Numbers)) {
+ // CHECK-FIXES-CUSTOM: for (int Number : llvm::reverse(Numbers)) {
+ // CHECK-FIXES-NEXT: observe(Number);
+ // CHECK-FIXES-NEXT: }
+
+ for (auto I = Numbers.crbegin(), E = Numbers.crend(); I != E; ++I) {
+ observe(*I);
+ }
+ // CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based for loop instead
+ // CHECK-FIXES-RANGES: for (int Number : std::ranges::reverse_view(Numbers)) {
+ // CHECK-FIXES-CUSTOM: for (int Number : llvm::reverse(Numbers)) {
+ // CHECK-FIXES-NEXT: observe(Number);
+ // CHECK-FIXES-NEXT: }
+
+ // Ensure these bad loops aren't transformed.
+ for (auto I = Numbers.rbegin(), E = Numbers.end(); I != E; ++I) {
+ observe(*I);
+ }
+ for (auto I = Numbers.begin(), E = Numbers.rend(); I != E; ++I) {
+ observe(*I);
+ }
+}
+
+void nonConstContainer(Reversable<int> &Numbers) {
+ for (auto I = Numbers.rbegin(), E = Numbers.rend(); I != E; ++I) {
+ mutate(*I);
+ }
+ // CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based for loop instead
+ // CHECK-FIXES-RANGES: for (int & Number : std::ranges::reverse_view(Numbers)) {
+ // CHECK-FIXES-CUSTOM: for (int & Number : llvm::reverse(Numbers)) {
+ // CHECK-FIXES-NEXT: mutate(Number);
+ // CHECK-FIXES-NEXT: }
+
+ for (auto I = Numbers.crbegin(), E = Numbers.crend(); I != E; ++I) {
+ observe(*I);
+ }
+ // CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based for loop instead
+ // CHECK-FIXES-RANGES: for (int Number : std::ranges::reverse_view(Numbers)) {
+ // CHECK-FIXES-CUSTOM: for (int Number : llvm::reverse(Numbers)) {
+ // CHECK-FIXES-NEXT: observe(Number);
+ // CHECK-FIXES-NEXT: }
+}
More information about the cfe-commits
mailing list