ccotter created this revision.
Herald added a subscriber: carlosgalvezp.
Herald added a reviewer: njames93.
Herald added a project: All.
ccotter requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.

This patch allows clang-tidy to replace traditional for-loops where the
container type inherits its `begin`/`end` methods from a base class.

Test plan: Added unit test cases that confirm the tool replaces the new
pattern.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D140307

Files:
  clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp
  
clang-tools-extra/test/clang-tidy/checkers/modernize/Inputs/loop-convert/structures.h
  clang-tools-extra/test/clang-tidy/checkers/modernize/loop-convert-basic.cpp


Index: 
clang-tools-extra/test/clang-tidy/checkers/modernize/loop-convert-basic.cpp
===================================================================
--- clang-tools-extra/test/clang-tidy/checkers/modernize/loop-convert-basic.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/modernize/loop-convert-basic.cpp
@@ -575,6 +575,7 @@
 const dependent<NonTriviallyCopyable> *Pconstv;
 
 transparent<dependent<int>> Cv;
+dependent_derived<int> VD;
 
 void f() {
   int Sum = 0;
@@ -653,6 +654,15 @@
   // 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 = VD.size(); E != I; ++I) {
+    printf("Fibonacci number is %d\n", VD[I]);
+    Sum += VD[I] + 2;
+  }
+  // CHECK-MESSAGES: :[[@LINE-4]]:3: warning: use range-based for loop instead
+  // CHECK-FIXES: for (int I : VD)
+  // 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.
Index: 
clang-tools-extra/test/clang-tidy/checkers/modernize/Inputs/loop-convert/structures.h
===================================================================
--- 
clang-tools-extra/test/clang-tidy/checkers/modernize/Inputs/loop-convert/structures.h
+++ 
clang-tools-extra/test/clang-tidy/checkers/modernize/Inputs/loop-convert/structures.h
@@ -126,6 +126,10 @@
   void constFoo() const;
 };
 
+template<typename ElemType>
+class dependent_derived : public dependent<ElemType> {
+};
+
 template<typename First, typename Second>
 class doublyDependent{
  public:
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
@@ -251,17 +251,18 @@
   // functions called begin() and end() taking the container as an argument
   // are also allowed.
   TypeMatcher RecordWithBeginEnd = qualType(anyOf(
-      qualType(
-          isConstQualified(),
-          hasUnqualifiedDesugaredType(recordType(hasDeclaration(cxxRecordDecl(
-              hasMethod(cxxMethodDecl(hasName("begin"), isConst())),
-              hasMethod(cxxMethodDecl(hasName("end"),
-                                      isConst()))))   // hasDeclaration
-                                                 ))), // qualType
+      qualType(isConstQualified(),
+               hasUnqualifiedDesugaredType(recordType(hasDeclaration(
+                   cxxRecordDecl(isSameOrDerivedFrom(cxxRecordDecl(
+                       hasMethod(cxxMethodDecl(hasName("begin"), isConst())),
+                       hasMethod(cxxMethodDecl(hasName("end"),
+                                               isConst())))))) // 
hasDeclaration
+                                                      ))),     // qualType
       qualType(unless(isConstQualified()),
                hasUnqualifiedDesugaredType(recordType(hasDeclaration(
-                   cxxRecordDecl(hasMethod(hasName("begin")),
-                                 hasMethod(hasName("end"))))))) // qualType
+                   cxxRecordDecl(isSameOrDerivedFrom(cxxRecordDecl(
+                       hasMethod(hasName("begin")),
+                       hasMethod(hasName("end"))))))))) // qualType
       ));
 
   StatementMatcher SizeCallMatcher = cxxMemberCallExpr(


Index: clang-tools-extra/test/clang-tidy/checkers/modernize/loop-convert-basic.cpp
===================================================================
--- clang-tools-extra/test/clang-tidy/checkers/modernize/loop-convert-basic.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/modernize/loop-convert-basic.cpp
@@ -575,6 +575,7 @@
 const dependent<NonTriviallyCopyable> *Pconstv;
 
 transparent<dependent<int>> Cv;
+dependent_derived<int> VD;
 
 void f() {
   int Sum = 0;
@@ -653,6 +654,15 @@
   // 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 = VD.size(); E != I; ++I) {
+    printf("Fibonacci number is %d\n", VD[I]);
+    Sum += VD[I] + 2;
+  }
+  // CHECK-MESSAGES: :[[@LINE-4]]:3: warning: use range-based for loop instead
+  // CHECK-FIXES: for (int I : VD)
+  // 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.
Index: clang-tools-extra/test/clang-tidy/checkers/modernize/Inputs/loop-convert/structures.h
===================================================================
--- clang-tools-extra/test/clang-tidy/checkers/modernize/Inputs/loop-convert/structures.h
+++ clang-tools-extra/test/clang-tidy/checkers/modernize/Inputs/loop-convert/structures.h
@@ -126,6 +126,10 @@
   void constFoo() const;
 };
 
+template<typename ElemType>
+class dependent_derived : public dependent<ElemType> {
+};
+
 template<typename First, typename Second>
 class doublyDependent{
  public:
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
@@ -251,17 +251,18 @@
   // functions called begin() and end() taking the container as an argument
   // are also allowed.
   TypeMatcher RecordWithBeginEnd = qualType(anyOf(
-      qualType(
-          isConstQualified(),
-          hasUnqualifiedDesugaredType(recordType(hasDeclaration(cxxRecordDecl(
-              hasMethod(cxxMethodDecl(hasName("begin"), isConst())),
-              hasMethod(cxxMethodDecl(hasName("end"),
-                                      isConst()))))   // hasDeclaration
-                                                 ))), // qualType
+      qualType(isConstQualified(),
+               hasUnqualifiedDesugaredType(recordType(hasDeclaration(
+                   cxxRecordDecl(isSameOrDerivedFrom(cxxRecordDecl(
+                       hasMethod(cxxMethodDecl(hasName("begin"), isConst())),
+                       hasMethod(cxxMethodDecl(hasName("end"),
+                                               isConst())))))) // hasDeclaration
+                                                      ))),     // qualType
       qualType(unless(isConstQualified()),
                hasUnqualifiedDesugaredType(recordType(hasDeclaration(
-                   cxxRecordDecl(hasMethod(hasName("begin")),
-                                 hasMethod(hasName("end"))))))) // qualType
+                   cxxRecordDecl(isSameOrDerivedFrom(cxxRecordDecl(
+                       hasMethod(hasName("begin")),
+                       hasMethod(hasName("end"))))))))) // qualType
       ));
 
   StatementMatcher SizeCallMatcher = cxxMemberCallExpr(
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
  • [PATCH] D140307: Match derive... Chris Cotter via Phabricator via cfe-commits

Reply via email to