Another patch for modernize-loop-convert.

Summary:
1. Avoid converting loops that iterate over the size of a container and don't use its elements, as this would result in an unused-result warning.
2. Never capture the elements by value on lambdas, thus avoiding doing unnecessary copies and errors with non-copyable types.
3. The 'const auto &' instead of 'auto &' substitution on const containers now works on arrays and pseudoarrays as well.
4. The error about multiple replacements in the same macro call is now documented in the tests (not solved though).
5. Due to [1], I had to add a dummy usage of the range element (like "(void) *It;" or similars) on the tests that had empty loops.
6. I removed the braces from the CHECK comments. I think that there is no need for them, and they confuse vim.

Reviewers: klimek

Subscribers: alexfh, cfe-commits

Differential Revision: http://reviews.llvm.org/D12734

llvm-svn: 247399
This commit is contained in:
Angel Garcia Gomez 2015-09-11 10:02:07 +00:00
parent ef3cf01d1c
commit bb9ca54bbe
8 changed files with 320 additions and 124 deletions

View File

@ -381,6 +381,20 @@ static bool usagesReturnRValues(const UsageResult &Usages) {
return true;
}
/// \brief Returns true if the container is const-qualified.
static bool containerIsConst(const Expr *ContainerExpr, bool Dereference) {
if (const auto *VDec = getReferencedVariable(ContainerExpr)) {
QualType CType = VDec->getType();
if (Dereference) {
if (!CType->isPointerType())
return false;
CType = CType->getPointeeType();
}
return CType.isConstQualified();
}
return false;
}
LoopConvertCheck::LoopConvertCheck(StringRef Name, ClangTidyContext *Context)
: ClangTidyCheck(Name, Context), TUInfo(new TUTrackingInfo),
MinConfidence(StringSwitch<Confidence::Level>(
@ -401,6 +415,11 @@ void LoopConvertCheck::doConversion(
StringRef ContainerString, const UsageResult &Usages,
const DeclStmt *AliasDecl, bool AliasUseRequired, bool AliasFromForInit,
const ForStmt *TheLoop, RangeDescriptor Descriptor) {
// If there aren't any usages, converting the loop would generate an unused
// variable warning.
if (Usages.size() == 0)
return;
auto Diag = diag(TheLoop->getForLoc(), "use range-based for loop instead");
std::string VarName;
@ -436,11 +455,23 @@ void LoopConvertCheck::doConversion(
VarName = Namer.createIndexName();
// First, replace all usages of the array subscript expression with our new
// variable.
for (const auto &I : Usages) {
std::string ReplaceText = I.IsArrow ? VarName + "." : VarName;
for (const auto &Usage : Usages) {
std::string ReplaceText;
if (Usage.Expression) {
// If this is an access to a member through the arrow operator, after
// the replacement it must be accessed through the '.' operator.
ReplaceText = Usage.Kind == Usage::UK_MemberThroughArrow ? VarName + "."
: VarName;
} else {
// The Usage expression is only null in case of lambda captures (which
// are VarDecl). If the index is captured by value, add '&' to capture
// by reference instead.
ReplaceText =
Usage.Kind == Usage::UK_CaptureByCopy ? "&" + VarName : VarName;
}
TUInfo->getReplacedVars().insert(std::make_pair(TheLoop, IndexVar));
Diag << FixItHint::CreateReplacement(
CharSourceRange::getTokenRange(I.Range), ReplaceText);
CharSourceRange::getTokenRange(Usage.Range), ReplaceText);
}
}
@ -537,16 +568,24 @@ void LoopConvertCheck::findAndVerifyUsages(
if (FixerKind == LFK_Array) {
// The array being indexed by IndexVar was discovered during traversal.
ContainerExpr = Finder.getContainerIndexed()->IgnoreParenImpCasts();
// Very few loops are over expressions that generate arrays rather than
// array variables. Consider loops over arrays that aren't just represented
// by a variable to be risky conversions.
if (!getReferencedVariable(ContainerExpr) &&
!isDirectMemberExpr(ContainerExpr))
ConfidenceLevel.lowerTo(Confidence::CL_Risky);
// Use 'const' if the array is const.
if (containerIsConst(ContainerExpr, Descriptor.ContainerNeedsDereference))
Descriptor.DerefByConstRef = true;
} else if (FixerKind == LFK_PseudoArray) {
if (!Descriptor.DerefByValue && !Descriptor.DerefByConstRef) {
const UsageResult &Usages = Finder.getUsages();
if (usagesAreConst(Usages)) {
if (usagesAreConst(Usages) ||
containerIsConst(ContainerExpr,
Descriptor.ContainerNeedsDereference)) {
Descriptor.DerefByConstRef = true;
} else if (usagesReturnRValues(Usages)) {
// If the index usages (dereference, subscript, at) return RValues,
@ -558,7 +597,7 @@ void LoopConvertCheck::findAndVerifyUsages(
if (!U.Expression || U.Expression->getType().isNull())
continue;
QualType Type = U.Expression->getType().getCanonicalType();
if (U.IsArrow) {
if (U.Kind == Usage::UK_MemberThroughArrow) {
if (!Type->isPointerType())
continue;
Type = Type->getPointeeType();
@ -625,6 +664,7 @@ void LoopConvertCheck::check(const MatchFinder::MatchResult &Result) {
// expression the loop variable is being tested against instead.
const auto *EndCall = Nodes.getStmtAs<CXXMemberCallExpr>(EndCallName);
const auto *BoundExpr = Nodes.getStmtAs<Expr>(ConditionBoundName);
// If the loop calls end()/size() after each iteration, lower our confidence
// level.
if (FixerKind != LFK_Array && !EndVar)

View File

@ -27,9 +27,9 @@ public:
private:
struct RangeDescriptor {
bool ContainerNeedsDereference;
bool DerefByConstRef;
bool DerefByValue;
bool IsTriviallyCopyable;
bool DerefByConstRef;
};
void doConversion(ASTContext *Context, const VarDecl *IndexVar,

View File

@ -560,7 +560,7 @@ bool ForLoopIndexUseVisitor::TraverseMemberExpr(MemberExpr *Member) {
// If something complicated is happening (i.e. the next token isn't an
// arrow), give up on making this work.
if (!ArrowLoc.isInvalid()) {
addUsage(Usage(ResultExpr, /*IsArrow=*/true,
addUsage(Usage(ResultExpr, Usage::UK_MemberThroughArrow,
SourceRange(Base->getExprLoc(), ArrowLoc)));
return true;
}
@ -762,7 +762,10 @@ bool ForLoopIndexUseVisitor::TraverseLambdaCapture(LambdaExpr *LE,
// FIXME: if the index is captured, it will count as an usage and the
// alias (if any) won't work, because it is only used in case of having
// exactly one usage.
addUsage(Usage(nullptr, false, C->getLocation()));
addUsage(Usage(nullptr,
C->getCaptureKind() == LCK_ByCopy ? Usage::UK_CaptureByCopy
: Usage::UK_CaptureByRef,
C->getLocation()));
}
}
return VisitorBase::TraverseLambdaCapture(LE, C);

View File

@ -197,14 +197,39 @@ private:
/// \brief The information needed to describe a valid convertible usage
/// of an array index or iterator.
struct Usage {
enum UsageKind {
// Regular usages of the loop index (the ones not specified below). Some
// examples:
// \code
// int X = 8 * Arr[i];
// ^~~~~~
// f(param1, param2, *It);
// ^~~
// if (Vec[i].SomeBool) {}
// ^~~~~~
// \endcode
UK_Default,
// Indicates whether this is an access to a member through the arrow
// operator on pointers or iterators.
UK_MemberThroughArrow,
// If the variable is being captured by a lambda, indicates whether the
// capture was done by value or by reference.
UK_CaptureByCopy,
UK_CaptureByRef
};
// The expression that is going to be converted. Null in case of lambda
// captures.
const Expr *Expression;
bool IsArrow;
UsageKind Kind;
// Range that covers this usage.
SourceRange Range;
explicit Usage(const Expr *E)
: Expression(E), IsArrow(false), Range(Expression->getSourceRange()) {}
Usage(const Expr *E, bool IsArrow, SourceRange Range)
: Expression(E), IsArrow(IsArrow), Range(std::move(Range)) {}
: Expression(E), Kind(UK_Default), Range(Expression->getSourceRange()) {}
Usage(const Expr *E, UsageKind Kind, SourceRange Range)
: Expression(E), Kind(Kind), Range(std::move(Range)) {}
};
/// \brief A class to encapsulate lowering of the tool's confidence level.

View File

@ -59,8 +59,9 @@ struct X {
};
template<typename ElemType>
class dependent{
class dependent {
public:
dependent<ElemType>();
struct iterator_base {
const ElemType& operator*()const;
iterator_base& operator ++();

View File

@ -7,6 +7,7 @@ namespace Array {
const int N = 6;
const int NMinusOne = N - 1;
int arr[N] = {1, 2, 3, 4, 5, 6};
const int constArr[N] = {1, 2, 3, 4, 5, 6};
int (*pArr)[N] = &arr;
void f() {
@ -17,10 +18,9 @@ void f() {
int k;
}
// CHECK-MESSAGES: :[[@LINE-4]]:3: warning: use range-based for loop instead [modernize-loop-convert]
// CHECK-FIXES: for (auto & elem : arr) {
// CHECK-FIXES: for (auto & elem : arr)
// CHECK-FIXES-NEXT: sum += elem;
// CHECK-FIXES-NEXT: int k;
// CHECK-FIXES-NEXT: }
for (int i = 0; i < N; ++i) {
printf("Fibonacci number is %d\n", arr[i]);
@ -53,9 +53,8 @@ void f() {
arr[i] += 1;
}
// CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based for loop instead
// CHECK-FIXES: for (auto & elem : arr) {
// CHECK-FIXES: for (auto & elem : arr)
// CHECK-FIXES-NEXT: elem += 1;
// CHECK-FIXES-NEXT: }
for (int i = 0; i < N; ++i) {
int x = arr[i] + 2;
@ -77,9 +76,8 @@ void f() {
sum += arr[i];
}
// CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based for loop instead
// CHECK-FIXES: for (auto & elem : arr) {
// CHECK-FIXES: for (auto & elem : arr)
// CHECK-FIXES-NEXT: sum += elem;
// CHECK-FIXES-NEXT: }
for (int i = 0; i < N; ++i) {
printf("Fibonacci number %d has address %p\n", arr[i], &arr[i]);
@ -95,9 +93,17 @@ void f() {
teas[i].g();
}
// CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based for loop instead
// CHECK-FIXES: for (auto & tea : teas) {
// CHECK-FIXES: for (auto & tea : teas)
// CHECK-FIXES-NEXT: tea.g();
// CHECK-FIXES-NEXT: }
}
void constArray() {
for (int i = 0; i < N; ++i) {
printf("2 * %d = %d\n", constArr[i], constArr[i] + constArr[i]);
}
// CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based for loop instead
// CHECK-FIXES: for (const auto & elem : constArr)
// CHECK-FIXES-NEXT: printf("2 * %d = %d\n", elem, elem + elem);
}
struct HasArr {
@ -108,17 +114,15 @@ struct HasArr {
printf("%d", Arr[i]);
}
// CHECK-MESSAGES: :[[@LINE-3]]:5: warning: use range-based for loop instead
// CHECK-FIXES: for (auto & elem : Arr) {
// CHECK-FIXES: for (auto & elem : Arr)
// CHECK-FIXES-NEXT: printf("%d", elem);
// CHECK-FIXES-NEXT: }
for (int i = 0; i < N; ++i) {
printf("%d", ValArr[i].x);
}
// CHECK-MESSAGES: :[[@LINE-3]]:5: warning: use range-based for loop instead
// CHECK-FIXES: for (auto & elem : ValArr) {
// CHECK-FIXES: for (auto & elem : ValArr)
// CHECK-FIXES-NEXT: printf("%d", elem.x);
// CHECK-FIXES-NEXT: }
}
void explicitThis() {
@ -126,21 +130,19 @@ struct HasArr {
printf("%d", this->Arr[i]);
}
// CHECK-MESSAGES: :[[@LINE-3]]:5: warning: use range-based for loop instead
// CHECK-FIXES: for (auto & elem : this->Arr) {
// CHECK-FIXES: for (auto & elem : this->Arr)
// CHECK-FIXES-NEXT: printf("%d", elem);
// CHECK-FIXES-NEXT: }
for (int i = 0; i < N; ++i) {
printf("%d", this->ValArr[i].x);
}
// CHECK-MESSAGES: :[[@LINE-3]]:5: warning: use range-based for loop instead
// CHECK-FIXES: for (auto & elem : this->ValArr) {
// CHECK-FIXES: for (auto & elem : this->ValArr)
// CHECK-FIXES-NEXT: printf("%d", elem.x);
// CHECK-FIXES-NEXT: }
}
};
// Loops whose bounds are value-dependent shold not be converted.
// Loops whose bounds are value-dependent should not be converted.
template <int N>
void dependentExprBound() {
for (int i = 0; i < N; ++i)
@ -263,7 +265,7 @@ void f() {
printf("Fibonacci number is %d\n", *it);
}
// CHECK-MESSAGES: :[[@LINE-4]]:3: warning: use range-based for loop instead
// CHECK-FIXES: for (auto & elem : v) {
// CHECK-FIXES: for (auto & elem : v)
// CHECK-FIXES-NEXT: printf("Fibonacci number is %d\n", elem);
for (dependent<int>::iterator it(v.begin()), e = v.end();
@ -271,7 +273,7 @@ void f() {
printf("Fibonacci number is %d\n", *it);
}
// CHECK-MESSAGES: :[[@LINE-4]]:3: warning: use range-based for loop instead
// CHECK-FIXES: for (auto & elem : v) {
// CHECK-FIXES: for (auto & elem : v)
// CHECK-FIXES-NEXT: printf("Fibonacci number is %d\n", elem);
doublyDependent<int, int> intmap;
@ -285,13 +287,14 @@ void f() {
// PtrSet's iterator dereferences by value so auto & can't be used.
{
PtrSet<int *> int_ptrs;
for (PtrSet<int *>::iterator I = int_ptrs.begin(),
E = int_ptrs.end();
PtrSet<int *> val_int_ptrs;
for (PtrSet<int *>::iterator I = val_int_ptrs.begin(),
E = val_int_ptrs.end();
I != E; ++I) {
(void) *I;
}
// CHECK-MESSAGES: :[[@LINE-4]]:5: warning: use range-based for loop instead
// CHECK-FIXES: for (auto int_ptr : int_ptrs) {
// CHECK-MESSAGES: :[[@LINE-5]]:5: warning: use range-based for loop instead
// CHECK-FIXES: for (auto val_int_ptr : val_int_ptrs)
}
// This container uses an iterator where the derefence type is a typedef of
@ -302,9 +305,10 @@ void f() {
for (TypedefDerefContainer<int>::iterator I = int_ptrs.begin(),
E = int_ptrs.end();
I != E; ++I) {
(void) *I;
}
// CHECK-MESSAGES: :[[@LINE-4]]:5: warning: use range-based for loop instead
// CHECK-FIXES: for (auto & int_ptr : int_ptrs) {
// CHECK-MESSAGES: :[[@LINE-5]]:5: warning: use range-based for loop instead
// CHECK-FIXES: for (auto & int_ptr : int_ptrs)
}
{
@ -314,10 +318,8 @@ void f() {
for (RValueDerefContainer<int>::iterator I = container.begin(),
E = container.end();
I != E; ++I) {
(void) *I;
}
// CHECK-FIXES: for (RValueDerefContainer<int>::iterator I = container.begin(),
// CHECK-FIXES-NEXT: E = container.end();
// CHECK-FIXES-NEXT: I != E; ++I) {
}
}
@ -350,17 +352,11 @@ void different_type() {
it != e; ++it) {
printf("Fibonacci number is %d\n", *it);
}
// CHECK-FIXES: for (dependent<int>::const_iterator it = v.begin(), e = v.end();
// CHECK-FIXES-NEXT: it != e; ++it) {
// CHECK-FIXES-NEXT: printf("Fibonacci number is %d\n", *it);
for (dependent<int>::const_iterator it(v.begin()), e = v.end();
it != e; ++it) {
printf("Fibonacci number is %d\n", *it);
}
// CHECK-FIXES: for (dependent<int>::const_iterator it(v.begin()), e = v.end();
// CHECK-FIXES-NEXT: it != e; ++it) {
// CHECK-FIXES-NEXT: printf("Fibonacci number is %d\n", *it);
}
// Tests to ensure that an implicit 'this' is picked up as the container.
@ -380,42 +376,45 @@ public:
void doSomething() const;
void doLoop() {
for (iterator I = begin(), E = end(); I != E; ++I) {
}
for (iterator I = begin(), E = end(); I != E; ++I)
(void) *I;
// CHECK-MESSAGES: :[[@LINE-2]]:5: warning: use range-based for loop instead
// CHECK-FIXES: for (auto & elem : *this) {
// CHECK-FIXES: for (auto & elem : *this)
for (iterator I = C::begin(), E = C::end(); I != E; ++I) {
}
for (iterator I = C::begin(), E = C::end(); I != E; ++I)
(void) *I;
// CHECK-MESSAGES: :[[@LINE-2]]:5: warning: use range-based for loop instead
// CHECK-FIXES: for (auto & elem : *this) {
// CHECK-FIXES: for (auto & elem : *this)
for (iterator I = begin(), E = end(); I != E; ++I) {
(void) *I;
doSomething();
}
for (iterator I = begin(); I != end(); ++I) {
}
for (iterator I = begin(); I != end(); ++I)
(void) *I;
// CHECK-MESSAGES: :[[@LINE-2]]:5: warning: use range-based for loop instead
// CHECK-FIXES: for (auto & elem : *this) {
// CHECK-FIXES: for (auto & elem : *this)
for (iterator I = begin(); I != end(); ++I) {
(void) *I;
doSomething();
}
}
void doLoop() const {
for (const_iterator I = begin(), E = end(); I != E; ++I) {
}
for (const_iterator I = begin(), E = end(); I != E; ++I)
(void) *I;
// CHECK-MESSAGES: :[[@LINE-2]]:5: warning: use range-based for loop instead
// CHECK-FIXES: for (auto & elem : *this) {
// CHECK-FIXES: for (auto & elem : *this)
for (const_iterator I = C::begin(), E = C::end(); I != E; ++I) {
}
for (const_iterator I = C::begin(), E = C::end(); I != E; ++I)
(void) *I;
// CHECK-MESSAGES: :[[@LINE-2]]:5: warning: use range-based for loop instead
// CHECK-FIXES: for (auto & elem : *this) {
// CHECK-FIXES: for (auto & elem : *this)
for (const_iterator I = begin(), E = end(); I != E; ++I) {
(void) *I;
doSomething();
}
}
@ -431,10 +430,10 @@ public:
void doLoop() {
// The implicit 'this' will have an Implicit cast to const C2* wrapped
// around it. Make sure the replacement still happens.
for (iterator I = begin(), E = end(); I != E; ++I) {
}
for (iterator I = begin(), E = end(); I != E; ++I)
(void) *I;
// CHECK-MESSAGES: :[[@LINE-2]]:5: warning: use range-based for loop instead
// CHECK-FIXES: for (auto & elem : *this) {
// CHECK-FIXES: for (auto & elem : *this)
}
};
@ -445,6 +444,8 @@ namespace PseudoArray {
const int N = 6;
dependent<int> v;
dependent<int> *pv;
const dependent<int> constv;
const dependent<int> *pconstv;
transparent<dependent<int>> cv;
@ -500,21 +501,62 @@ void f() {
// CHECK-FIXES-NEXT: sum += elem + 2;
}
void constness() {
int sum = 0;
for (int i = 0, e = constv.size(); i < e; ++i) {
printf("Fibonacci number is %d\n", constv[i]);
sum += constv[i] + 2;
}
// CHECK-MESSAGES: :[[@LINE-4]]:3: warning: use range-based for loop instead
// CHECK-FIXES: for (const auto & elem : constv)
// CHECK-FIXES-NEXT: printf("Fibonacci number is %d\n", elem);
// CHECK-FIXES-NEXT: sum += elem + 2;
for (int i = 0, e = constv.size(); i < e; ++i) {
printf("Fibonacci number is %d\n", constv.at(i));
sum += constv.at(i) + 2;
}
// CHECK-MESSAGES: :[[@LINE-4]]:3: warning: use range-based for loop instead
// CHECK-FIXES: for (const auto & elem : constv)
// CHECK-FIXES-NEXT: printf("Fibonacci number is %d\n", elem);
// CHECK-FIXES-NEXT: sum += elem + 2;
for (int i = 0, e = pconstv->size(); i < e; ++i) {
printf("Fibonacci number is %d\n", pconstv->at(i));
sum += pconstv->at(i) + 2;
}
// CHECK-MESSAGES: :[[@LINE-4]]:3: warning: use range-based for loop instead
// CHECK-FIXES: for (const auto & elem : *pconstv)
// CHECK-FIXES-NEXT: printf("Fibonacci number is %d\n", elem);
// CHECK-FIXES-NEXT: sum += elem + 2;
// This test will fail if size() isn't called repeatedly, since it
// returns unsigned int, and 0 is deduced to be signed int.
// FIXME: Insert the necessary explicit conversion, or write out the types
// explicitly.
for (int i = 0; i < pconstv->size(); ++i) {
printf("Fibonacci number is %d\n", (*pconstv).at(i));
sum += (*pconstv)[i] + 2;
}
// CHECK-MESSAGES: :[[@LINE-4]]:3: warning: use range-based for loop instead
// CHECK-FIXES: for (const auto & elem : *pconstv)
// CHECK-FIXES-NEXT: printf("Fibonacci number is %d\n", elem);
// CHECK-FIXES-NEXT: sum += elem + 2;
}
// Check for loops that don't mention containers.
void noContainer() {
for (auto i = 0; i < v.size(); ++i) {
}
// CHECK-MESSAGES: :[[@LINE-2]]:3: warning: use range-based for loop instead
// CHECK-FIXES: for (const auto & elem : v) {
for (auto i = 0; i < v.size(); ++i)
;
// CHECK-MESSAGES: :[[@LINE-2]]:3: warning: use range-based for loop instead
// CHECK-FIXES: for (const auto & elem : v)
}
struct NoBeginEnd {
unsigned size() const;
unsigned& operator[](int);
const unsigned& operator[](int) const;
};
struct NoConstBeginEnd {
@ -522,6 +564,8 @@ struct NoConstBeginEnd {
unsigned size() const;
unsigned* begin();
unsigned* end();
unsigned& operator[](int);
const unsigned& operator[](int) const;
};
struct ConstBeginEnd {
@ -529,30 +573,34 @@ struct ConstBeginEnd {
unsigned size() const;
unsigned* begin() const;
unsigned* end() const;
unsigned& operator[](int);
const unsigned& operator[](int) const;
};
// Shouldn't transform pseudo-array uses if the container doesn't provide
// begin() and end() of the right const-ness.
void NoBeginEndTest() {
NoBeginEnd NBE;
for (unsigned i = 0, e = NBE.size(); i < e; ++i) {
}
for (unsigned i = 0, e = NBE.size(); i < e; ++i)
printf("%d\n", NBE[i]);
const NoConstBeginEnd const_NCBE;
for (unsigned i = 0, e = const_NCBE.size(); i < e; ++i) {
}
for (unsigned i = 0, e = const_NCBE.size(); i < e; ++i)
printf("%d\n", const_NCBE[i]);
ConstBeginEnd CBE;
for (unsigned i = 0, e = CBE.size(); i < e; ++i) {
}
for (unsigned i = 0, e = CBE.size(); i < e; ++i)
printf("%d\n", CBE[i]);
// CHECK-MESSAGES: :[[@LINE-2]]:3: warning: use range-based for loop instead
// CHECK-FIXES: for (const auto & elem : CBE) {
// CHECK-FIXES: for (auto & elem : CBE)
// CHECK-FIXES-NEXT: printf("%d\n", elem);
const ConstBeginEnd const_CBE;
for (unsigned i = 0, e = const_CBE.size(); i < e; ++i) {
}
for (unsigned i = 0, e = const_CBE.size(); i < e; ++i)
printf("%d\n", const_CBE[i]);
// CHECK-MESSAGES: :[[@LINE-2]]:3: warning: use range-based for loop instead
// CHECK-FIXES: for (const auto & elem : const_CBE) {
// CHECK-FIXES: for (const auto & elem : const_CBE)
// CHECK-FIXES-NEXT: printf("%d\n", elem);
}
struct DerefByValue {
@ -570,7 +618,7 @@ void derefByValueTest() {
printf("%d\n", DBV[i]);
}
// CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based for loop instead
// CHECK-FIXES: for (auto elem : DBV) {
// CHECK-FIXES: for (auto elem : DBV)
// CHECK-FIXES-NEXT: printf("%d\n", elem);
for (unsigned i = 0, e = DBV.size(); i < e; ++i) {
@ -578,8 +626,8 @@ void derefByValueTest() {
printf("%d\n", DBV[i]);
}
// CHECK-MESSAGES: :[[@LINE-4]]:3: warning: use range-based for loop instead
// CHECK-FIXES: for (auto elem : DBV) {
// CHECK-FIXES-NEXT: auto f = [DBV, elem]() {};
// CHECK-FIXES: for (auto elem : DBV)
// CHECK-FIXES-NEXT: auto f = [DBV, &elem]() {};
// CHECK-FIXES-NEXT: printf("%d\n", elem);
}

View File

@ -14,10 +14,9 @@ void f() {
int b = arr[i][a];
}
// CHECK-MESSAGES: :[[@LINE-4]]:3: warning: use range-based for loop instead
// CHECK-FIXES: for (auto & elem : arr) {
// CHECK-FIXES: for (auto & elem : arr)
// CHECK-FIXES-NEXT: int a = 0;
// CHECK-FIXES-NEXT: int b = elem[a];
// CHECK-FIXES-NEXT: }
for (int j = 0; j < M; ++j) {
int a = 0;
@ -121,7 +120,7 @@ void aliasing() {
}
// CHECK-MESSAGES: :[[@LINE-5]]:3: warning: use range-based for loop instead
// CHECK-FIXES: for (auto alias : IntArr)
// CHECK-FIXES-NEXT: if (alias) {
// CHECK-FIXES-NEXT: if (alias)
for (unsigned i = 0; i < N; ++i) {
while (int alias = IntArr[i]) {
@ -130,7 +129,7 @@ void aliasing() {
}
// CHECK-MESSAGES: :[[@LINE-5]]:3: warning: use range-based for loop instead
// CHECK-FIXES: for (auto alias : IntArr)
// CHECK-FIXES-NEXT: while (alias) {
// CHECK-FIXES-NEXT: while (alias)
for (unsigned i = 0; i < N; ++i) {
switch (int alias = IntArr[i]) {
@ -140,7 +139,7 @@ void aliasing() {
}
// CHECK-MESSAGES: :[[@LINE-6]]:3: warning: use range-based for loop instead
// CHECK-FIXES: for (auto alias : IntArr)
// CHECK-FIXES-NEXT: switch (alias) {
// CHECK-FIXES-NEXT: switch (alias)
for (unsigned i = 0; i < N; ++i) {
for (int alias = IntArr[i]; alias < N; ++alias) {
@ -149,7 +148,7 @@ void aliasing() {
}
// CHECK-MESSAGES: :[[@LINE-5]]:3: warning: use range-based for loop instead
// CHECK-FIXES: for (auto alias : IntArr)
// CHECK-FIXES-NEXT: for (; alias < N; ++alias) {
// CHECK-FIXES-NEXT: for (; alias < N; ++alias)
for (unsigned i = 0; i < N; ++i) {
for (unsigned j = 0; int alias = IntArr[i]; ++j) {
@ -158,14 +157,14 @@ void aliasing() {
}
// CHECK-MESSAGES: :[[@LINE-5]]:3: warning: use range-based for loop instead
// CHECK-FIXES: for (auto alias : IntArr)
// CHECK-FIXES-NEXT: for (unsigned j = 0; alias; ++j) {
// CHECK-FIXES-NEXT: for (unsigned j = 0; alias; ++j)
struct IntRef { IntRef(const int& i); };
for (int i = 0; i < N; ++i) {
IntRef Int(IntArr[i]);
}
// CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based for loop instead
// CHECK-FIXES: for (auto & elem : IntArr) {
// CHECK-FIXES: for (auto & elem : IntArr)
// CHECK-FIXES-NEXT: IntRef Int(elem);
}
@ -288,7 +287,7 @@ void macroConflict() {
}
// CHECK-MESSAGES: :[[@LINE-5]]:3: warning: use range-based for loop instead
// CHECK-FIXES: for (auto & DEFs_it : DEFs)
// CHECK-FIXES-NEXT: if (DEFs_it == DEF) {
// CHECK-FIXES-NEXT: if (DEFs_it == DEF)
// CHECK-FIXES-NEXT: printf("I found %d\n", DEFs_it);
}
@ -315,8 +314,8 @@ void typeConflict() {
T Vals;
// Using the name "Val", although it is the name of an existing struct, is
// safe in this loop since it will only exist within this scope.
for (T::iterator it = Vals.begin(), e = Vals.end(); it != e; ++it) {
}
for (T::iterator it = Vals.begin(), e = Vals.end(); it != e; ++it)
(void) *it;
// CHECK-MESSAGES: :[[@LINE-2]]:3: warning: use range-based for loop instead
// CHECK-FIXES: for (auto & Val : Vals)
@ -333,8 +332,8 @@ void typeConflict() {
U TDs;
// Naming the variable "TD" within this loop is safe because the typedef
// was never used within the loop.
for (U::iterator it = TDs.begin(), e = TDs.end(); it != e; ++it) {
}
for (U::iterator it = TDs.begin(), e = TDs.end(); it != e; ++it)
(void) *it;
// CHECK-MESSAGES: :[[@LINE-2]]:3: warning: use range-based for loop instead
// CHECK-FIXES: for (auto & TD : TDs)
@ -342,8 +341,9 @@ void typeConflict() {
for (U::iterator it = TDs.begin(), e = TDs.end(); it != e; ++it) {
TD V;
V.x = 5;
(void) *it;
}
// CHECK-MESSAGES: :[[@LINE-4]]:3: warning: use range-based for loop instead
// CHECK-MESSAGES: :[[@LINE-5]]:3: warning: use range-based for loop instead
// CHECK-FIXES: for (auto & TDs_it : TDs)
// CHECK-FIXES-NEXT: TD V;
// CHECK-FIXES-NEXT: V.x = 5;
@ -456,8 +456,8 @@ void f() {
}
}
// CHECK-MESSAGES: :[[@LINE-5]]:3: warning: use range-based for loop instead
// CHECK-FIXES: for (auto & elem : NestT) {
// CHECK-FIXES-NEXT: for (T::iterator TI = (elem).begin(), TE = (elem).end(); TI != TE; ++TI) {
// CHECK-FIXES: for (auto & elem : NestT)
// CHECK-FIXES-NEXT: for (T::iterator TI = (elem).begin(), TE = (elem).end(); TI != TE; ++TI)
// CHECK-FIXES-NEXT: printf("%d", *TI);
// The inner loop is also convertible.
@ -468,8 +468,8 @@ void f() {
}
}
// CHECK-MESSAGES: :[[@LINE-5]]:3: warning: use range-based for loop instead
// CHECK-FIXES: for (const auto & elem : NestS) {
// CHECK-FIXES-NEXT: for (S::const_iterator SI = (elem).begin(), SE = (elem).end(); SI != SE; ++SI) {
// CHECK-FIXES: for (const auto & elem : NestS)
// CHECK-FIXES-NEXT: for (S::const_iterator SI = (elem).begin(), SE = (elem).end(); SI != SE; ++SI)
// CHECK-FIXES-NEXT: printf("%d", *SI);
for (Nested<S>::const_iterator I = NestS.begin(), E = NestS.end(); I != E; ++I) {
@ -480,7 +480,7 @@ void f() {
}
}
// CHECK-MESSAGES: :[[@LINE-7]]:3: warning: use range-based for loop instead
// CHECK-FIXES: for (const auto & s : NestS) {
// CHECK-FIXES: for (const auto & s : NestS)
for (Nested<S>::iterator I = NestS.begin(), E = NestS.end(); I != E; ++I) {
S &s = *I;
@ -490,7 +490,7 @@ void f() {
}
}
// CHECK-MESSAGES: :[[@LINE-7]]:3: warning: use range-based for loop instead
// CHECK-FIXES: for (auto & s : NestS) {
// CHECK-FIXES: for (auto & s : NestS)
Foo foo;
for (Nested<S>::const_iterator I = NestS.begin(), E = NestS.end(); I != E; ++I) {
@ -501,7 +501,7 @@ void f() {
}
}
// CHECK-MESSAGES: :[[@LINE-7]]:3: warning: use range-based for loop instead
// CHECK-FIXES: for (const auto & s : NestS) {
// CHECK-FIXES: for (const auto & s : NestS)
for (Nested<S>::iterator I = NestS.begin(), E = NestS.end(); I != E; ++I) {
S &s = *I;
@ -511,7 +511,7 @@ void f() {
}
}
// CHECK-MESSAGES: :[[@LINE-7]]:3: warning: use range-based for loop instead
// CHECK-FIXES: for (auto & s : NestS) {
// CHECK-FIXES: for (auto & s : NestS)
}
@ -623,7 +623,7 @@ void f() {
printf("Fibonacci number is %d\n", *it);
}
// CHECK-MESSAGES: :[[@LINE-4]]:3: warning: use range-based for loop instead
// CHECK-FIXES: for (auto & elem : v) {
// CHECK-FIXES: for (auto & elem : v)
// CHECK-FIXES-NEXT: printf("Fibonacci number is %d\n", elem);
for (dependent<int>::iterator it(v.begin());
@ -631,7 +631,7 @@ void f() {
printf("Fibonacci number is %d\n", *it);
}
// CHECK-MESSAGES: :[[@LINE-4]]:3: warning: use range-based for loop instead
// CHECK-FIXES: for (auto & elem : v) {
// CHECK-FIXES: for (auto & elem : v)
// CHECK-FIXES-NEXT: printf("Fibonacci number is %d\n", elem);
doublyDependent<int, int> intmap;
@ -684,6 +684,9 @@ void different_type() {
namespace Macros {
#define TWO_PARAM(x, y) if (x == y) {}
#define THREE_PARAM(x, y, z) if (x == y) {z;}
const int N = 10;
int arr[N];
@ -692,12 +695,28 @@ void messing_with_macros() {
printf("Value: %d\n", arr[i]);
}
// CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based for loop instead
// CHECK-FIXES: for (auto & elem : arr) {
// CHECK-FIXES: for (auto & elem : arr)
// CHECK-FIXES-NEXT: printf("Value: %d\n", elem);
for (int i = 0; i < N; ++i) {
printf("Value: %d\n", CONT arr[i]);
}
// FIXME: Right now, clang-tidy does not allow to make insertions in several
// arguments of the same macro call. The following code:
// \code
// for (int i = 0; i < N; ++i) {
// TWO_PARAM(arr[i], arr[i]);
// THREE_PARAM(arr[i], arr[i], arr[i]);
// }
// \endcode
// Should be converted to this:
// \code
// for (auto & elem : arr) {
// TWO_PARAM(elem, elem);
// THREE_PARAM(elem, elem, elem);
// }
// \endcode
}
} // namespace Macros
@ -708,12 +727,14 @@ template <class Container>
void set_union(Container &container) {
for (typename Container::const_iterator SI = container.begin(),
SE = container.end(); SI != SE; ++SI) {
(void) *SI;
}
S s;
for (S::iterator SI = s.begin(), SE = s.end(); SI != SE; ++SI) {
}
for (S::iterator SI = s.begin(), SE = s.end(); SI != SE; ++SI)
(void) *SI;
// CHECK-MESSAGES: :[[@LINE-2]]:3: warning: use range-based for loop instead
// CHECK-FIXES: for (auto & elem : s) {
// CHECK-FIXES: for (auto & elem : s)
}
void template_instantiation() {
@ -735,7 +756,7 @@ void capturesIndex() {
auto F1 = [Arr, I]() { int R1 = Arr[I] + 1; };
// CHECK-MESSAGES: :[[@LINE-2]]:3: warning: use range-based for loop instead
// CHECK-FIXES: for (auto & elem : Arr)
// CHECK-FIXES-NEXT: auto F1 = [Arr, elem]() { int R1 = elem + 1; };
// CHECK-FIXES-NEXT: auto F1 = [Arr, &elem]() { int R1 = elem + 1; };
for (int I = 0; I < N; ++I)
auto F2 = [Arr, &I]() { int R2 = Arr[I] + 3; };
@ -749,8 +770,7 @@ void capturesIndex() {
auto F3 = [&Arr, I]() { int R3 = Arr[I]; };
// CHECK-MESSAGES: :[[@LINE-2]]:3: warning: use range-based for loop instead
// CHECK-FIXES: for (auto & elem : Arr)
// CHECK-FIXES-NEXT: auto F3 = [&Arr, elem]() { int R3 = elem; };
// FIXME: this does two copies instead of one. Capture elem by ref?
// CHECK-FIXES-NEXT: auto F3 = [&Arr, &elem]() { int R3 = elem; };
for (int I = 0; I < N; ++I)
@ -764,8 +784,7 @@ void capturesIndex() {
auto F5 = [&Arr, I]() { int &R5 = Arr[I]; };
// CHECK-MESSAGES: :[[@LINE-2]]:3: warning: use range-based for loop instead
// CHECK-FIXES: for (auto & elem : Arr)
// CHECK-FIXES-NEXT: auto F5 = [&Arr, elem]() { int &R5 = elem; };
// FIXME: this does one copy instead of none. Capture elem by ref?
// CHECK-FIXES-NEXT: auto F5 = [&Arr, &elem]() { int &R5 = elem; };
for (int I = 0; I < N; ++I)
@ -782,10 +801,9 @@ void capturesIndex() {
}
// CHECK-MESSAGES: :[[@LINE-6]]:3: warning: use range-based for loop instead
// CHECK-FIXES: for (auto & elem : Arr)
// CHECK-FIXES-NEXT: auto F = [Arr, elem](int k) {
// CHECK-FIXES-NEXT: auto F = [Arr, &elem](int k)
// CHECK-FIXES-NEXT: printf("%d\n", elem + k);
// CHECK-FIXES-NEXT: };
// CHECK-FIXES-NEXT: F(elem);
// CHECK-FIXES: F(elem);
}
void implicitCapture() {
@ -815,7 +833,7 @@ void implicitCapture() {
}
// CHECK-MESSAGES: :[[@LINE-6]]:3: warning: use range-based for loop instead
// CHECK-FIXES: for (auto & elem : Arr)
// CHECK-FIXES-NEXT: auto G3 = [&]() {
// CHECK-FIXES-NEXT: auto G3 = [&]()
// CHECK-FIXES-NEXT: int R3 = elem;
// CHECK-FIXES-NEXT: int J3 = elem + R3;
@ -826,7 +844,7 @@ void implicitCapture() {
}
// CHECK-MESSAGES: :[[@LINE-5]]:3: warning: use range-based for loop instead
// CHECK-FIXES: for (auto & elem : Arr)
// CHECK-FIXES-NEXT: auto G4 = [=]() {
// CHECK-FIXES-NEXT: auto G4 = [=]()
// CHECK-FIXES-NEXT: int R4 = elem + 5;
// Alias by value.
@ -838,7 +856,7 @@ void implicitCapture() {
}
// CHECK-MESSAGES: :[[@LINE-6]]:3: warning: use range-based for loop instead
// CHECK-FIXES: for (auto R5 : Arr)
// CHECK-FIXES-NEXT: auto G5 = [&]() {
// CHECK-FIXES-NEXT: auto G5 = [&]()
// CHECK-FIXES: int J5 = 8 + R5;
// Alias by reference.
@ -850,7 +868,7 @@ void implicitCapture() {
}
// CHECK-MESSAGES: :[[@LINE-6]]:3: warning: use range-based for loop instead
// CHECK-FIXES: for (auto & R6 : Arr)
// CHECK-FIXES-NEXT: auto G6 = [&]() {
// CHECK-FIXES-NEXT: auto G6 = [&]()
// CHECK-FIXES: int J6 = -1 + R6;
}
@ -883,6 +901,30 @@ void iterators() {
auto H5 = [=]() { int R = *I; };
}
void captureByValue() {
// When the index is captured by value, we should replace this by a capture
// by reference. This avoids extra copies.
// FIXME: this could change semantics on array or pseudoarray loops if the
// container is captured by copy.
const int N = 10;
int Arr[N];
dependent<int> Dep;
for (int I = 0; I < N; ++I) {
auto C1 = [&Arr, I]() { if (Arr[I] == 1); };
}
// CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based for loop instead
// CHECK-FIXES: for (auto & elem : Arr)
// CHECK-FIXES-NEXT: auto C1 = [&Arr, &elem]() { if (elem == 1); };
for (unsigned I = 0; I < Dep.size(); ++I) {
auto C2 = [&Dep, I]() { if (Dep[I] == 2); };
}
// CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based for loop instead
// CHECK-FIXES: for (auto & elem : Dep)
// CHECK-FIXES-NEXT: auto C2 = [&Dep, &elem]() { if (elem == 2); };
}
} // namespace Lambdas
namespace InitLists {

View File

@ -290,7 +290,6 @@ const int N = 6;
dependent<int> v;
dependent<int> *pv;
transparent<dependent<int>> cv;
int Sum = 0;
// Checks for the Index start and end:
@ -473,3 +472,41 @@ void complexContainer() {
}
} // namespace NegativeMultiEndCall
namespace NoUsages {
const int N = 6;
int arr[N] = {1, 2, 3, 4, 5, 6};
S s;
dependent<int> v;
int Count = 0;
void foo();
void f() {
for (int I = 0; I < N; ++I) {}
for (int I = 0; I < N; ++I)
printf("Hello world\n");
for (int I = 0; I < N; ++I)
++Count;
for (int I = 0; I < N; ++I)
foo();
for (S::iterator I = s.begin(), E = s.end(); I != E; ++I) {}
for (S::iterator I = s.begin(), E = s.end(); I != E; ++I)
printf("Hello world\n");
for (S::iterator I = s.begin(), E = s.end(); I != E; ++I)
++Count;
for (S::iterator I = s.begin(), E = s.end(); I != E; ++I)
foo();
for (int I = 0; I < v.size(); ++I) {}
for (int I = 0; I < v.size(); ++I)
printf("Hello world\n");
for (int I = 0; I < v.size(); ++I)
++Count;
for (int I = 0; I < v.size(); ++I)
foo();
}
} // namespace NoUsages