njames93 created this revision.
njames93 added reviewers: alexfh, Eugene.Zelenko, angelgarcia, aaron.ballman,
klimek.
Herald added subscribers: cfe-commits, xazax.hun.
Herald added a project: clang.
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.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D82089
Files:
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
clang-tools-extra/test/clang-tidy/checkers/modernize-loop-convert-reverse.cpp
Index: clang-tools-extra/test/clang-tidy/checkers/modernize-loop-convert-reverse.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/modernize-loop-convert-reverse.cpp
@@ -0,0 +1,120 @@
+// RUN: %check_clang_tidy -std=c++20 -check-suffix=RANGES %s modernize-loop-convert %t
+
+// RUN: %check_clang_tidy -check-suffix=CUSTOM %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'}]}"
+
+// 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
+
+// RUN: %check_clang_tidy -check-suffix=CUSTOM-NO-HEADER %s modernize-loop-convert %t -- \
+// RUN: -config="{CheckOptions: [ \
+// RUN: {key: modernize-loop-convert.MakeReverseRangeFunction, value: 'globalReverse'}]}"
+
+// Ensure we get a warning if we only supply one of the required reverse range arguments.
+// RUN: %check_clang_tidy -check-suffix=BAD-CUSTOM %s modernize-loop-convert %t -- \
+// RUN: -config="{CheckOptions: [ \
+// RUN: {key: modernize-loop-convert.MakeReverseRangeHeader, value: 'ranges/v3/views/reverse.hpp'}]}"
+
+// CHECK-MESSAGES-BAD-CUSTOM: 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 <algorithm>
+// CHECK-FIXES-CUSTOM: #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) {
+ // CHECK-MESSAGES-RANGES: :[[@LINE+3]]:3: warning: use range-based for loop instead
+ // CHECK-MESSAGES-CUSTOM: :[[@LINE+2]]:3: warning: use range-based for loop instead
+ // CHECK-MESSAGES-CUSTOM-NO-HEADER: :[[@LINE+1]]:3: warning: use range-based for loop instead
+ for (auto I = Numbers.rbegin(), E = Numbers.rend(); I != E; ++I) {
+ observe(*I);
+ // CHECK-FIXES-RANGES: for (int Number : std::ranges::reverse_view(Numbers)) {
+ // CHECK-FIXES-RANGES-NEXT: observe(Number);
+ // CHECK-FIXES-CUSTOM: for (int Number : llvm::reverse(Numbers)) {
+ // CHECK-FIXES-CUSTOM-NEXT: observe(Number);
+ // CHECK-FIXES-CUSTOM-NO-HEADER: for (int Number : globalReverse(Numbers)) {
+ // CHECK-FIXES-CUSTOM-NO-HEADER-NEXT: observe(Number);
+ }
+ // CHECK-MESSAGES-RANGES: :[[@LINE+3]]:3: warning: use range-based for loop instead
+ // CHECK-MESSAGES-CUSTOM: :[[@LINE+2]]:3: warning: use range-based for loop instead
+ // CHECK-MESSAGES-CUSTOM-NO-HEADER: :[[@LINE+1]]:3: warning: use range-based for loop instead
+ for (auto I = Numbers.crbegin(), E = Numbers.crend(); I != E; ++I) {
+ observe(*I);
+ // CHECK-FIXES-RANGES: for (int Number : std::ranges::reverse_view(Numbers)) {
+ // CHECK-FIXES-RANGES-NEXT: observe(Number);
+ // CHECK-FIXES-CUSTOM: for (int Number : llvm::reverse(Numbers)) {
+ // CHECK-FIXES-CUSTOM-NEXT: observe(Number);
+ // CHECK-FIXES-CUSTOM-NO-HEADER: for (int Number : globalReverse(Numbers)) {
+ // CHECK-FIXES-CUSTOM-NO-HEADER-NEXT: observe(Number);
+ }
+
+ // 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) {
+ // CHECK-MESSAGES-RANGES: :[[@LINE+3]]:3: warning: use range-based for loop instead
+ // CHECK-MESSAGES-CUSTOM: :[[@LINE+2]]:3: warning: use range-based for loop instead
+ // CHECK-MESSAGES-CUSTOM-NO-HEADER: :[[@LINE+1]]:3: warning: use range-based for loop instead
+ for (auto I = Numbers.rbegin(), E = Numbers.rend(); I != E; ++I) {
+ mutate(*I);
+ // CHECK-FIXES-RANGES: for (int & Number : std::ranges::reverse_view(Numbers)) {
+ // CHECK-FIXES-RANGES-NEXT: mutate(Number);
+ // CHECK-FIXES-CUSTOM: for (int & Number : llvm::reverse(Numbers)) {
+ // CHECK-FIXES-CUSTOM-NEXT: mutate(Number);
+ // CHECK-FIXES-CUSTOM-NO-HEADER: for (int & Number : globalReverse(Numbers)) {
+ // CHECK-FIXES-CUSTOM-NO-HEADER-NEXT: mutate(Number);
+ }
+ // CHECK-MESSAGES-RANGES: :[[@LINE+3]]:3: warning: use range-based for loop instead
+ // CHECK-MESSAGES-CUSTOM: :[[@LINE+2]]:3: warning: use range-based for loop instead
+ // CHECK-MESSAGES-CUSTOM-NO-HEADER: :[[@LINE+1]]:3: warning: use range-based for loop instead
+ for (auto I = Numbers.crbegin(), E = Numbers.crend(); I != E; ++I) {
+ observe(*I);
+ // CHECK-FIXES-RANGES: for (int Number : std::ranges::reverse_view(Numbers)) {
+ // CHECK-FIXES-RANGES-NEXT: observe(Number);
+ // CHECK-FIXES-CUSTOM: for (int Number : llvm::reverse(Numbers)) {
+ // CHECK-FIXES-CUSTOM-NEXT: observe(Number);
+ // CHECK-FIXES-CUSTOM-NO-HEADER: for (int Number : globalReverse(Numbers)) {
+ // CHECK-FIXES-CUSTOM-NO-HEADER-NEXT: observe(Number);
+ }
+}
Index: clang-tools-extra/docs/clang-tidy/checks/modernize-loop-convert.rst
===================================================================
--- clang-tools-extra/docs/clang-tidy/checks/modernize-loop-convert.rst
+++ clang-tools-extra/docs/clang-tidy/checks/modernize-loop-convert.rst
@@ -118,6 +118,40 @@
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.
+ Default value is an empty string.
+
Limitations
-----------
Index: clang-tools-extra/docs/ReleaseNotes.rst
===================================================================
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -195,7 +195,12 @@
Changes in existing checks
^^^^^^^^^^^^^^^^^^^^^^^^^^
-- Improved :doc:'readability-identifier-naming
+- 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.
Now able to rename member references in class template definitions with
Index: clang-tools-extra/clang-tidy/modernize/LoopConvertUtils.h
===================================================================
--- clang-tools-extra/clang-tidy/modernize/LoopConvertUtils.h
+++ clang-tools-extra/clang-tidy/modernize/LoopConvertUtils.h
@@ -26,7 +26,12 @@
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;
Index: clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.h
===================================================================
--- clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.h
+++ 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 @@
}
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,27 @@
bool DerefByValue;
std::string ContainerString;
QualType ElemType;
+ bool NeedsReverseCall;
+ };
+
+ class ReverseRangeDetail {
+ public:
+ ReverseRangeDetail(bool UseCxx20IfAvailable, std::string FunctionName,
+ std::string HeaderName, const LangOptions &LangOpts);
+
+ bool isEnabled() const { return IsEnabled; }
+ bool shouldIncludeHeader() const {
+ return isEnabled() && !getHeader().empty();
+ }
+ bool isUsingCxx20() const { return UseCxx20IfAvailable; }
+ StringRef getFunction() const { return Function; }
+ StringRef getHeader() const { return Header; }
+
+ private:
+ bool IsEnabled;
+ bool UseCxx20IfAvailable;
+ std::string Function;
+ std::string Header;
};
void getAliasRange(SourceManager &SM, SourceRange &DeclRange);
@@ -71,6 +95,9 @@
const unsigned long long MaxCopySize;
const Confidence::Level MinConfidence;
const VariableNamer::NamingStyle NamingStyle;
+ std::unique_ptr<utils::IncludeInserter> Inserter;
+ utils::IncludeSorter::IncludeStyle IncludeStyle;
+ const ReverseRangeDetail ReverseRange;
};
} // namespace modernize
Index: clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp
+++ clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp
@@ -19,6 +19,7 @@
#include "llvm/ADT/StringRef.h"
#include "llvm/ADT/StringSwitch.h"
#include "llvm/Support/Casting.h"
+#include "llvm/Support/WithColor.h"
#include <cassert>
#include <cstring>
#include <utility>
@@ -32,6 +33,7 @@
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";
@@ -43,7 +45,9 @@
static const char EndVarName[] = "endVar";
static const char DerefByValueResultName[] = "derefByValueResult";
static const char DerefByRefResultName[] = "derefByRefResult";
-
+static const char MakeReverseRangeFunction[] = "MakeReverseRangeFunction";
+static const char MakeReverseRangeHeader[] = "MakeReverseRangeHeader";
+static const char UseCxx20ReverseRanges[] = "UseCxx20ReverseRanges";
static ArrayRef<std::pair<StringRef, Confidence::Level>>
getConfidenceMapping() {
static constexpr std::pair<StringRef, Confidence::Level> Mapping[] = {
@@ -144,11 +148,17 @@
/// - 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(anyOf(hasName("begin"), hasName("cbegin")))))
+ cxxMemberCallExpr(argumentCountIs(0),
+ callee(cxxMethodDecl(BeginNameMatcher)))
.bind(BeginCallName);
DeclarationMatcher InitDeclMatcher =
@@ -162,8 +172,7 @@
varDecl(hasInitializer(anything())).bind(EndVarName);
StatementMatcher EndCallMatcher = cxxMemberCallExpr(
- argumentCountIs(0),
- callee(cxxMethodDecl(anyOf(hasName("end"), hasName("cend")))));
+ argumentCountIs(0), callee(cxxMethodDecl(EndNameMatcher)));
StatementMatcher IteratorBoundMatcher =
expr(anyOf(ignoringParenImpCasts(
@@ -221,7 +230,7 @@
hasArgument(
0, declRefExpr(to(varDecl(TestDerefReturnsByValue)
.bind(IncrementVarName))))))))
- .bind(LoopNameIterator);
+ .bind(IsReverse ? LoopNameReverseIterator : LoopNameIterator);
}
/// The matcher used for array-like containers (pseudoarrays).
@@ -322,7 +331,7 @@
/// 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));
@@ -333,9 +342,11 @@
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();
@@ -353,18 +364,19 @@
/// 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 ||
@@ -472,7 +484,10 @@
LoopConvertCheck::RangeDescriptor::RangeDescriptor()
: ContainerNeedsDereference(false), DerefByConstRef(false),
- DerefByValue(false) {}
+ DerefByValue(false), NeedsReverseCall(false) {}
+
+static constexpr StringRef RangesMakeReverse = "std::ranges::reverse_view";
+static constexpr StringRef RangesMakeReverseHeader = "algorithm";
LoopConvertCheck::LoopConvertCheck(StringRef Name, ClangTidyContext *Context)
: ClangTidyCheck(Name, Context), TUInfo(new TUTrackingInfo),
@@ -480,21 +495,41 @@
MinConfidence(Options.get("MinConfidence", getConfidenceMapping(),
Confidence::CL_Reasonable)),
NamingStyle(Options.get("NamingStyle", getStyleMapping(),
- VariableNamer::NS_CamelCase)) {}
+ VariableNamer::NS_CamelCase)),
+ ReverseRange(Options.get(UseCxx20ReverseRanges, true),
+ Options.get(MakeReverseRangeFunction, ""),
+ Options.get(MakeReverseRangeHeader, ""), getLangOpts()) {}
void LoopConvertCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
Options.store(Opts, "MaxCopySize", std::to_string(MaxCopySize));
Options.store(Opts, "MinConfidence", MinConfidence, getConfidenceMapping());
Options.store(Opts, "NamingStyle", NamingStyle, getStyleMapping());
+ Options.store(Opts, UseCxx20ReverseRanges, ReverseRange.isUsingCxx20());
+ Options.store(Opts, MakeReverseRangeFunction, ReverseRange.getFunction());
+ Options.store(Opts, MakeReverseRangeHeader, ReverseRange.getHeader());
+}
+
+void LoopConvertCheck::registerPPCallbacks(const SourceManager &SM,
+ Preprocessor *PP,
+ Preprocessor *ModuleExpanderPP) {
+ if (ReverseRange.shouldIncludeHeader()) {
+ Inserter = std::make_unique<utils::IncludeInserter>(SM, getLangOpts(),
+ IncludeStyle);
+ PP->addPPCallbacks(Inserter->CreatePPCallbacks());
+ }
}
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 (ReverseRange.isEnabled())
+ Finder->addMatcher(
+ traverse(ast_type_traits::TK_AsIs, makeIteratorLoopMatcher(true)),
+ this);
}
/// Given the range of a single declaration, such as:
@@ -648,8 +683,20 @@
StringRef MaybeDereference = Descriptor.ContainerNeedsDereference ? "*" : "";
std::string TypeString = Type.getAsString(getLangOpts());
std::string Range = ("(" + TypeString + " " + VarName + " : " +
- MaybeDereference + Descriptor.ContainerString + ")")
+ (Descriptor.NeedsReverseCall
+ ? llvm::Twine(ReverseRange.getFunction(), "(")
+ : llvm::Twine()) +
+ MaybeDereference + Descriptor.ContainerString +
+ (Descriptor.NeedsReverseCall ? ")" : "") + ")")
.str();
+
+ if (Descriptor.NeedsReverseCall && ReverseRange.shouldIncludeHeader()) {
+ StringRef Header = ReverseRange.getHeader();
+ if (Optional<FixItHint> Insertion = Inserter->CreateIncludeInsertion(
+ Context->getSourceManager().getFileID(Loop->getBeginLoc()), Header,
+ /*IsAngled*/ Header == RangesMakeReverseHeader))
+ FixIts.push_back(*Insertion);
+ }
FixIts.push_back(FixItHint::CreateReplacement(
CharSourceRange::getTokenRange(ParenRange), Range));
diag(Loop->getForLoc(), "use range-based for loop instead") << FixIts;
@@ -761,8 +808,9 @@
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);
@@ -791,7 +839,7 @@
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();
@@ -830,6 +878,8 @@
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");
@@ -857,10 +907,11 @@
// 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 =
@@ -926,6 +977,29 @@
Finder.aliasFromForInit(), Loop, Descriptor);
}
+LoopConvertCheck::ReverseRangeDetail::ReverseRangeDetail(
+ bool UseCxx20IfAvailable, std::string FunctionName, std::string HeaderName,
+ const LangOptions &LangOpts)
+ : IsEnabled(false), UseCxx20IfAvailable(UseCxx20IfAvailable),
+ Function(FunctionName), Header(HeaderName) {
+ if (Function.empty() && !Header.empty()) {
+ llvm::WithColor::warning()
+ << "modernize-loop-convert: '" << MakeReverseRangeHeader
+ << "' is set but '" << MakeReverseRangeFunction
+ << "' is not, Disabling reverse loop transformation\n";
+ return;
+ }
+ if (Function.empty()) {
+ if (UseCxx20IfAvailable && LangOpts.CPlusPlus20) {
+ IsEnabled = true;
+ Function = RangesMakeReverse.str();
+ Header = RangesMakeReverseHeader.str();
+ }
+ return;
+ }
+ IsEnabled = true;
+}
+
} // namespace modernize
} // namespace tidy
} // namespace clang
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
