From a85eb11129ce941e91b5bc3b188e469aaca4425f Mon Sep 17 00:00:00 2001 From: Nathan James Date: Thu, 4 Mar 2021 18:58:42 +0000 Subject: [PATCH] [clang-tidy] Extend LoopConvert on array with `!=` comparison Enables transforming loops of the form: ``` for (int i = 0; I != container.size(); ++I) { container[I]...; } for (int i = 0; I != N; ++I) { FixedArrSizeN[I]...; } ``` Reviewed By: aaron.ballman Differential Revision: https://reviews.llvm.org/D97940 --- .../clang-tidy/modernize/LoopConvertCheck.cpp | 55 +++++++++++----------- .../checkers/modernize-loop-convert-basic.cpp | 54 +++++++++++++++++++++ 2 files changed, 81 insertions(+), 28 deletions(-) diff --git a/clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp b/clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp index 9df343f..9f6ef08 100644 --- a/clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp +++ b/clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp @@ -83,6 +83,17 @@ static const StatementMatcher incrementVarMatcher() { return declRefExpr(to(varDecl(equalsBoundNode(InitVarName)))); } +static StatementMatcher +arrayConditionMatcher(internal::Matcher LimitExpr) { + return binaryOperator( + anyOf(allOf(hasOperatorName("<"), hasLHS(integerComparisonMatcher()), + hasRHS(LimitExpr)), + allOf(hasOperatorName(">"), hasLHS(LimitExpr), + hasRHS(integerComparisonMatcher())), + allOf(hasOperatorName("!="), + hasOperands(integerComparisonMatcher(), LimitExpr)))); +} + /// The matcher for loops over arrays. /// \code /// for (int i = 0; i < 3 + 2; ++i) { ... } @@ -99,18 +110,12 @@ StatementMatcher makeArrayLoopMatcher() { StatementMatcher ArrayBoundMatcher = expr(hasType(isInteger())).bind(ConditionBoundName); - return forStmt( - unless(isInTemplateInstantiation()), - hasLoopInit(declStmt(hasSingleDecl(initToZeroMatcher()))), - hasCondition(anyOf( - binaryOperator(hasOperatorName("<"), - hasLHS(integerComparisonMatcher()), - hasRHS(ArrayBoundMatcher)), - binaryOperator(hasOperatorName(">"), hasLHS(ArrayBoundMatcher), - hasRHS(integerComparisonMatcher())))), - hasIncrement( - unaryOperator(hasOperatorName("++"), - hasUnaryOperand(incrementVarMatcher())))) + return forStmt(unless(isInTemplateInstantiation()), + hasLoopInit(declStmt(hasSingleDecl(initToZeroMatcher()))), + hasCondition(arrayConditionMatcher(ArrayBoundMatcher)), + hasIncrement( + unaryOperator(hasOperatorName("++"), + hasUnaryOperand(incrementVarMatcher())))) .bind(LoopNameArray); } @@ -278,22 +283,16 @@ StatementMatcher makePseudoArrayLoopMatcher() { declRefExpr(to(varDecl(equalsBoundNode(EndVarName))))), EndInitMatcher)); - return forStmt( - unless(isInTemplateInstantiation()), - hasLoopInit( - anyOf(declStmt(declCountIs(2), - containsDeclaration(0, initToZeroMatcher()), - containsDeclaration(1, EndDeclMatcher)), - declStmt(hasSingleDecl(initToZeroMatcher())))), - hasCondition(anyOf( - binaryOperator(hasOperatorName("<"), - hasLHS(integerComparisonMatcher()), - hasRHS(IndexBoundMatcher)), - binaryOperator(hasOperatorName(">"), hasLHS(IndexBoundMatcher), - hasRHS(integerComparisonMatcher())))), - hasIncrement( - unaryOperator(hasOperatorName("++"), - hasUnaryOperand(incrementVarMatcher())))) + return forStmt(unless(isInTemplateInstantiation()), + hasLoopInit( + anyOf(declStmt(declCountIs(2), + containsDeclaration(0, initToZeroMatcher()), + containsDeclaration(1, EndDeclMatcher)), + declStmt(hasSingleDecl(initToZeroMatcher())))), + hasCondition(arrayConditionMatcher(IndexBoundMatcher)), + hasIncrement( + unaryOperator(hasOperatorName("++"), + hasUnaryOperand(incrementVarMatcher())))) .bind(LoopNamePseudoArray); } diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize-loop-convert-basic.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize-loop-convert-basic.cpp index 06b1b73..b2cd0e2 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/modernize-loop-convert-basic.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/modernize-loop-convert-basic.cpp @@ -95,6 +95,33 @@ void f() { // CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based for loop instead // CHECK-FIXES: for (auto & Tea : Teas) // CHECK-FIXES-NEXT: Tea.g(); + + for (int I = 0; N > I; ++I) { + printf("Fibonacci number %d has address %p\n", Arr[I], &Arr[I]); + Sum += Arr[I] + 2; + } + // CHECK-MESSAGES: :[[@LINE-4]]:3: warning: use range-based for loop instead + // CHECK-FIXES: for (int & I : Arr) + // CHECK-FIXES-NEXT: printf("Fibonacci number %d has address %p\n", I, &I); + // CHECK-FIXES-NEXT: Sum += I + 2; + + for (int I = 0; N != I; ++I) { + printf("Fibonacci number %d has address %p\n", Arr[I], &Arr[I]); + Sum += Arr[I] + 2; + } + // CHECK-MESSAGES: :[[@LINE-4]]:3: warning: use range-based for loop instead + // CHECK-FIXES: for (int & I : Arr) + // CHECK-FIXES-NEXT: printf("Fibonacci number %d has address %p\n", I, &I); + // CHECK-FIXES-NEXT: Sum += I + 2; + + for (int I = 0; I != N; ++I) { + printf("Fibonacci number %d has address %p\n", Arr[I], &Arr[I]); + Sum += Arr[I] + 2; + } + // CHECK-MESSAGES: :[[@LINE-4]]:3: warning: use range-based for loop instead + // CHECK-FIXES: for (int & I : Arr) + // CHECK-FIXES-NEXT: printf("Fibonacci number %d has address %p\n", I, &I); + // CHECK-FIXES-NEXT: Sum += I + 2; } const int *constArray() { @@ -589,6 +616,33 @@ void f() { // CHECK-FIXES: for (int I : *Cv) // CHECK-FIXES-NEXT: printf("Fibonacci number is %d\n", I); // CHECK-FIXES-NEXT: Sum += I + 2; + + for (int I = 0, E = V.size(); E > I; ++I) { + printf("Fibonacci number is %d\n", V[I]); + Sum += V[I] + 2; + } + // CHECK-MESSAGES: :[[@LINE-4]]:3: warning: use range-based for loop instead + // CHECK-FIXES: for (int I : V) + // CHECK-FIXES-NEXT: printf("Fibonacci number is %d\n", I); + // CHECK-FIXES-NEXT: Sum += I + 2; + + for (int I = 0, E = V.size(); I != E; ++I) { + printf("Fibonacci number is %d\n", V[I]); + Sum += V[I] + 2; + } + // CHECK-MESSAGES: :[[@LINE-4]]:3: warning: use range-based for loop instead + // CHECK-FIXES: for (int I : V) + // CHECK-FIXES-NEXT: printf("Fibonacci number is %d\n", I); + // CHECK-FIXES-NEXT: Sum += I + 2; + + for (int I = 0, E = V.size(); E != I; ++I) { + printf("Fibonacci number is %d\n", V[I]); + Sum += V[I] + 2; + } + // CHECK-MESSAGES: :[[@LINE-4]]:3: warning: use range-based for loop instead + // CHECK-FIXES: for (int I : V) + // CHECK-FIXES-NEXT: printf("Fibonacci number is %d\n", I); + // CHECK-FIXES-NEXT: Sum += I + 2; } // Ensure that 'const auto &' is used with containers of non-trivial types. -- 2.7.4