[PATCH] D116577: [clang-tidy] Added "boost-use-range-based-for-loop" check

[Denis Mikhailov via Phabricator via cfe-commits]

denzor200 updated this revision to Diff 411548.
denzor200 marked 6 inline comments as done.

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D116577/new/

https://reviews.llvm.org/D116577

Files:
  clang-tools-extra/clang-tidy/boost/BoostTidyModule.cpp
  clang-tools-extra/clang-tidy/boost/CMakeLists.txt
  clang-tools-extra/clang-tidy/boost/UseRangeBasedForLoopCheck.cpp
  clang-tools-extra/clang-tidy/boost/UseRangeBasedForLoopCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/boost-use-range-based-for-loop.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/test/clang-tidy/checkers/boost-use-range-based-for-loop.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/boost-use-range-based-for-loop.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/boost-use-range-based-for-loop.cpp
@@ -0,0 +1,229 @@
+// RUN: %check_clang_tidy %s boost-use-range-based-for-loop %t
+
+namespace std {
+template 
+class list {
+public:
+  using iterator = T*;
+  using value_type = T;
+  iterator begin();
+  iterator end();
+};
+template 
+class map {};
+template 
+class tuple {};
+template 
+struct pair {
+  explicit pair(const First&, const Second&);
+};
+}
+
+#define BOOST_FOREACH(VAR, COL) for(VAR;(COL, false);)
+#define IDENTITY_TYPE(...) __VA_ARGS__
+
+void test_range_based_for_loop()
+{
+  std::list list_int;
+  BOOST_FOREACH( int i, list_int ) {
+// do something with i
+  }
+  // CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based for loop instead of BOOST_FOREACH [boost-use-range-based-for-loop]
+  // CHECK-FIXES: for( int i: list_int ) {
+
+  short array_short[] = {1,2,3};
+  BOOST_FOREACH(int val, array_short) {
+/// The short was implicitly converted to an int
+  }
+  // CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based
+  // CHECK-FIXES: for(int val: array_short) {
+}
+
+#define foreach_ BOOST_FOREACH
+#define foreach_2nd_ foreach_
+
+void test_range_based_for_loop_prettier()
+{
+  std::list l;
+  foreach_( int i, l ) {
+// do something with i
+  }
+  // CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based 
+  // CHECK-FIXES: for( int i: l ) {
+
+  short array_short[] = {1,2,3};
+  foreach_2nd_(int val, array_short) {
+/// The short was implicitly converted to an int
+  }
+  // CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based
+  // CHECK-FIXES: for(int val: array_short) {
+}
+
+template
+void test_range_based_for_loop_in_template_instantiation(const std::list& some_list
+	   , const Containter& some_containter)
+{
+  std::list list_int;
+  BOOST_FOREACH( int i, list_int ) {
+// do something with i
+  }
+  // CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based for loop instead of BOOST_FOREACH [boost-use-range-based-for-loop]
+  // CHECK-FIXES: for( int i: list_int ) {
+
+  // FIXME
+#if 0
+  BOOST_FOREACH( T i, some_list ) {
+// do something with i
+  }
+  // :[[@LINE-3]]:3: warning: use range-based for loop instead of BOOST_FOREACH [boost-use-range-based-for-loop]
+  // for( T i: some_list ) {
+#endif
+
+  BOOST_FOREACH( typename Containter::value_type i, some_containter ) {
+// do something with i
+  }
+  // CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based for loop instead of BOOST_FOREACH [boost-use-range-based-for-loop]
+  // CHECK-FIXES: BOOST_FOREACH( typename Containter::value_type i, some_containter ) {
+  // ^ so no fix-its here.
+}
+
+void test_range_based_for_loop_without_spaces()
+{
+  std::list> l;
+  BOOST_FOREACH(std::listi,l){
+// do something with i
+  }
+  // CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based 
+  // CHECK-FIXES: for(std::listi:l){
+}
+
+void test_range_based_for_loop_no_migration()
+{
+  // FIXME: implement these migrations and remove here tests
+  std::list l;
+  using rng = std::pair::iterator, std::list::iterator>;
+
+  int i;
+  BOOST_FOREACH( i, l ) {
+// do something with i
+  }
+  // CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based 
+  // CHECK-FIXES: BOOST_FOREACH( i, l ) {
+  // ^ so no fix-its here.
+
+  BOOST_FOREACH( int val, rng(l.begin(), l.end()) ) {
+// do something with i
+  }
+  // CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based 
+  // CHECK-FIXES: BOOST_FOREACH( int val, rng(l.begin(), l.end()) ) {
+  // ^ so no fix-its here.
+}
+
+void test_range_based_for_loop_keep_comments()
+{
+  std::list l;
+  std::list l2;
+
+  /* 1 */ BOOST_FOREACH( int i, l=l2 ) {
+// do something with i
+  }
+  // CHECK-MESSAGES: :[[@LINE-3]]:11: warning: use range-based
+  // CHECK-FIXES: /* 1 */ for( int i: l=l2 ) {
+
+  BOOST_FOREACH /* 1 */ ( int i, l=l2 ) {
+// do something with i
+  }
+  // CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based
+  // CHECK-FIXES: for /* 1 */ ( int i: l=l2 ) {
+
+  BOOST_FOREACH( /* 1 */ int i, l=l2 ) {
+// do something w

[PATCH] D116577: [clang-tidy] Added "boost-use-range-based-for-loop" check

[Denis Mikhailov via Phabricator via cfe-commits]

denzor200 added a comment.

My apologize for the delay! Diff was updated, implemented detection of known 
failure cases in the matcher and not issue a fixit, just only diagnostic.




Comment at: clang-tools-extra/clang-tidy/boost/UseRangeBasedForLoopCheck.cpp:68
+ ColToken->getLocation()),
+   (llvm::Twine("for(") + VariableString + " : ").str());
+  }

LegalizeAdulthood wrote:
> Now that this is chained, you might need to `clang-format` on it again.
> 
> Probably best to just `clang-format` the whole file before you submit.
`clang-format` was already applied


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D116577/new/

https://reviews.llvm.org/D116577

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D116577: [clang-tidy] Added "boost-use-range-based-for-loop" check

[Denis Mikhailov via Phabricator via cfe-commits]

denzor200 updated this revision to Diff 411556.

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D116577/new/

https://reviews.llvm.org/D116577

Files:
  clang-tools-extra/clang-tidy/boost/BoostTidyModule.cpp
  clang-tools-extra/clang-tidy/boost/CMakeLists.txt
  clang-tools-extra/clang-tidy/boost/UseRangeBasedForLoopCheck.cpp
  clang-tools-extra/clang-tidy/boost/UseRangeBasedForLoopCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/boost-use-range-based-for-loop.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/test/clang-tidy/checkers/boost-use-range-based-for-loop.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/boost-use-range-based-for-loop.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/boost-use-range-based-for-loop.cpp
@@ -0,0 +1,229 @@
+// RUN: %check_clang_tidy %s boost-use-range-based-for-loop %t
+
+namespace std {
+template 
+class list {
+public:
+  using iterator = T*;
+  using value_type = T;
+  iterator begin();
+  iterator end();
+};
+template 
+class map {};
+template 
+class tuple {};
+template 
+struct pair {
+  explicit pair(const First&, const Second&);
+};
+}
+
+#define BOOST_FOREACH(VAR, COL) for(VAR;(COL, false);)
+#define IDENTITY_TYPE(...) __VA_ARGS__
+
+void test_range_based_for_loop()
+{
+  std::list list_int;
+  BOOST_FOREACH( int i, list_int ) {
+// do something with i
+  }
+  // CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based for loop instead of BOOST_FOREACH [boost-use-range-based-for-loop]
+  // CHECK-FIXES: for( int i: list_int ) {
+
+  short array_short[] = {1,2,3};
+  BOOST_FOREACH(int val, array_short) {
+/// The short was implicitly converted to an int
+  }
+  // CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based
+  // CHECK-FIXES: for(int val: array_short) {
+}
+
+#define foreach_ BOOST_FOREACH
+#define foreach_2nd_ foreach_
+
+void test_range_based_for_loop_prettier()
+{
+  std::list l;
+  foreach_( int i, l ) {
+// do something with i
+  }
+  // CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based 
+  // CHECK-FIXES: for( int i: l ) {
+
+  short array_short[] = {1,2,3};
+  foreach_2nd_(int val, array_short) {
+/// The short was implicitly converted to an int
+  }
+  // CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based
+  // CHECK-FIXES: for(int val: array_short) {
+}
+
+template
+void test_range_based_for_loop_in_template_instantiation(const std::list& some_list
+	   , const Containter& some_containter)
+{
+  std::list list_int;
+  BOOST_FOREACH( int i, list_int ) {
+// do something with i
+  }
+  // CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based for loop instead of BOOST_FOREACH [boost-use-range-based-for-loop]
+  // CHECK-FIXES: for( int i: list_int ) {
+
+  // FIXME
+#if 0
+  BOOST_FOREACH( T i, some_list ) {
+// do something with i
+  }
+  // :[[@LINE-3]]:3: warning: use range-based for loop instead of BOOST_FOREACH [boost-use-range-based-for-loop]
+  // for( T i: some_list ) {
+#endif
+
+  BOOST_FOREACH( typename Containter::value_type i, some_containter ) {
+// do something with i
+  }
+  // CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based for loop instead of BOOST_FOREACH [boost-use-range-based-for-loop]
+  // CHECK-FIXES: BOOST_FOREACH( typename Containter::value_type i, some_containter ) {
+  // ^ so no fix-its here.
+}
+
+void test_range_based_for_loop_without_spaces()
+{
+  std::list> l;
+  BOOST_FOREACH(std::listi,l){
+// do something with i
+  }
+  // CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based 
+  // CHECK-FIXES: for(std::listi:l){
+}
+
+void test_range_based_for_loop_no_migration()
+{
+  // FIXME: implement these migrations and remove here tests
+  std::list l;
+  using rng = std::pair::iterator, std::list::iterator>;
+
+  int i;
+  BOOST_FOREACH( i, l ) {
+// do something with i
+  }
+  // CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based 
+  // CHECK-FIXES: BOOST_FOREACH( i, l ) {
+  // ^ so no fix-its here.
+
+  BOOST_FOREACH( int val, rng(l.begin(), l.end()) ) {
+// do something with i
+  }
+  // CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based 
+  // CHECK-FIXES: BOOST_FOREACH( int val, rng(l.begin(), l.end()) ) {
+  // ^ so no fix-its here.
+}
+
+void test_range_based_for_loop_keep_comments()
+{
+  std::list l;
+  std::list l2;
+
+  /* 1 */ BOOST_FOREACH( int i, l=l2 ) {
+// do something with i
+  }
+  // CHECK-MESSAGES: :[[@LINE-3]]:11: warning: use range-based
+  // CHECK-FIXES: /* 1 */ for( int i: l=l2 ) {
+
+  BOOST_FOREACH /* 1 */ ( int i, l=l2 ) {
+// do something with i
+  }
+  // CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based
+  // CHECK-FIXES: for /* 1 */ ( int i: l=l2 ) {
+
+  BOOST_FOREACH( /* 1 */ int i, l=l2 ) {
+// do something with i
+  }
+  // CHECK-MESSAGES: :[[@LINE-3]

[PATCH] D116577: [clang-tidy] Added "boost-use-range-based-for-loop" check

[Denis Mikhailov via Phabricator via cfe-commits]

denzor200 planned changes to this revision.
denzor200 added inline comments.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/boost-use-range-based-for-loop.cpp:22
+
+#define BOOST_FOREACH(VAR, COL) for(VAR;(COL, false);)
+#define IDENTITY_TYPE(...) __VA_ARGS__

Unfortunately, example does not correspond to the real `BOOST_FOREACH`, the 
tests do not reveal real problems. Will have to redo. I will come back to this 
patch in the future


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D116577/new/

https://reviews.llvm.org/D116577

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D116577: [clang-tidy] Added "boost-use-range-based-for-loop" check

[Denis Mikhailov via Phabricator via cfe-commits]

denzor200 created this revision.
Herald added subscribers: carlosgalvezp, xazax.hun, mgorny.
denzor200 requested review of this revision.
Herald added a project: clang-tools-extra.

Added a check for converting ``BOOST_FOREACH(..., ...)`` loops to use the new 
range-based loops in C++11.
This check have some limitations, see documentation for details.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D116577

Files:
  clang-tools-extra/clang-tidy/boost/BoostTidyModule.cpp
  clang-tools-extra/clang-tidy/boost/CMakeLists.txt
  clang-tools-extra/clang-tidy/boost/UseRangeBasedForLoopCheck.cpp
  clang-tools-extra/clang-tidy/boost/UseRangeBasedForLoopCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/boost-use-range-based-for-loop.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/test/clang-tidy/checkers/boost-use-range-based-for-loop.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/boost-use-range-based-for-loop.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/boost-use-range-based-for-loop.cpp
@@ -0,0 +1,41 @@
+// RUN: %check_clang_tidy %s boost-use-range-based-for-loop %t
+
+namespace std {
+template 
+class list {};
+}
+
+#define BOOST_FOREACH(VAR, COL) while(true)
+#define BOOST_REVERSE_FOREACH(VAR, COL) while(true)
+
+void test_range_based_for_loop()
+{
+  std::list list_int;
+  BOOST_FOREACH( int i, list_int ) {
+// do something with i
+  }
+  // CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based for loop instead of BOOST_FOREACH [boost-use-range-based-for-loop]
+  // CHECK-FIXES: for(int i : list_int ) {
+
+  short array_short[] = {1,2,3};
+  BOOST_FOREACH(int
+val, array_short) {
+/// The short was implicitly converted to an int
+  }
+  // CHECK-MESSAGES: :[[@LINE-4]]:3: warning: use range-based
+  // CHECK-FIXES: for(int val : array_short) {
+}
+
+#define foreach_ BOOST_FOREACH
+#define foreach_r_   BOOST_REVERSE_FOREACH
+
+void test_range_based_for_loop_prettier()
+{
+  std::list list_int;
+  foreach_( int i, list_int ) {
+// do something with i
+  }
+  // CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based 
+  // CHECK-FIXES: for(int i : list_int ) {
+}
+
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -51,6 +51,7 @@
`android-cloexec-pipe2 `_,
`android-cloexec-socket `_,
`android-comparison-in-temp-failure-retry `_,
+   `boost-use-range-based-for-loop `_, "Yes"
`boost-use-to-string `_, "Yes"
`bugprone-argument-comment `_, "Yes"
`bugprone-assert-side-effect `_,
Index: clang-tools-extra/docs/clang-tidy/checks/boost-use-range-based-for-loop.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/boost-use-range-based-for-loop.rst
@@ -0,0 +1,65 @@
+.. title:: clang-tidy - boost-use-range-based-for-loop
+
+boost-use-range-based-for-loop
+==
+
+This check converts ``BOOST_FOREACH(..., ...)`` loops to use the new range-based loops in C++11.
+
+Two kinds of loops can be converted correctly:
+
+-  Loops over arrays.
+-  Loops over STL containers.
+
+.. code-block:: c++
+
+std::list list_int( /*...*/ );
+BOOST_FOREACH( int i, list_int )
+{
+// do something with i
+}
+
+// Will be changed to
+std::list list_int( /*...*/ );
+for( int i: list_int )
+{
+// do something with i
+}
+
+Known Limitations
+-
+* Client code that use ``BOOST_FOREACH`` with "``std::pair`` of iterators" or with "Null-terminated strings (``char`` and ``wchar_t``)" or some other sequence types (everything that is not declared in the list "convertible correctly" above) will produce broken code (compilation error).
+
+.. code-block:: c++
+
+const char* cstr = "Hello world";
+BOOST_FOREACH( char c, cstr )
+{
+// do something with c
+}
+
+// Will be changed to
+const char* cstr = "Hello world";
+for( char c: cstr ) // won't compile
+{
+// do something with c
+}
+
+* First argument of the ``BOOST_FOREACH`` macro must be only new identifier definition, all other will produce a compilation error after migration.
+
+.. code-block:: c++
+
+std::list list_int( /*...*/ );
+int i;
+BOOST_FOREACH( i, list_int )
+{
+// do something with i
+}
+
+// Will be changed to
+std::list list_int( /*...*/ );
+int i;
+for( i: list_int ) // won't compile
+{
+// do something with i
+}
+
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/doc

[PATCH] D116577: [clang-tidy] Added "boost-use-range-based-for-loop" check

[Denis Mikhailov via Phabricator via cfe-commits]

denzor200 added a comment.

In D116577#3220757 , 
@LegalizeAdulthood wrote:

> I opened a similar issue for converting Qt's foreach to a range for loop 
> .
>
> However this check lands, it should be a simple generalization to have it 
> process Qt foreach loops as well, so perhaps a check name like 
> `modernize-foreach-to-range-for` would be better?

Generic code is a good practize, but i vote against the 
`modernize-foreach-to-range-for` check. My arguments:
-Not everyone uses //boost// and not everyone agrees to see "boost checks" in 
the already large //modernize// section. In the future, there will be a lot of 
these checks, not only `BOOST_FOREACH`. This is for example `Boost Assign`, 
`Boost Static Assert`, `Boost Format`, and so on, you can go on and on.
-All the same as above can be said about Qt.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D116577/new/

https://reviews.llvm.org/D116577

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D116577: [clang-tidy] Added "boost-use-range-based-for-loop" check

[Denis Mikhailov via Phabricator via cfe-commits]

denzor200 updated this revision to Diff 398516.
denzor200 marked 9 inline comments as done.
denzor200 added a comment.

review


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D116577/new/

https://reviews.llvm.org/D116577

Files:
  clang-tools-extra/clang-tidy/boost/BoostTidyModule.cpp
  clang-tools-extra/clang-tidy/boost/CMakeLists.txt
  clang-tools-extra/clang-tidy/boost/UseRangeBasedForLoopCheck.cpp
  clang-tools-extra/clang-tidy/boost/UseRangeBasedForLoopCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/boost-use-range-based-for-loop.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/test/clang-tidy/checkers/boost-use-range-based-for-loop.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/boost-use-range-based-for-loop.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/boost-use-range-based-for-loop.cpp
@@ -0,0 +1,39 @@
+// RUN: %check_clang_tidy %s boost-use-range-based-for-loop %t
+
+namespace std {
+template 
+class list {};
+}
+
+#define BOOST_FOREACH(VAR, COL) while(true)
+
+void test_range_based_for_loop()
+{
+  std::list list_int;
+  BOOST_FOREACH( int i, list_int ) {
+// do something with i
+  }
+  // CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based for loop instead of BOOST_FOREACH [boost-use-range-based-for-loop]
+  // CHECK-FIXES: for(int i : list_int ) {
+
+  short array_short[] = {1,2,3};
+  BOOST_FOREACH(int
+val, array_short) {
+/// The short was implicitly converted to an int
+  }
+  // CHECK-MESSAGES: :[[@LINE-4]]:3: warning: use range-based
+  // CHECK-FIXES: for(int val : array_short) {
+}
+
+#define foreach_ BOOST_FOREACH
+
+void test_range_based_for_loop_prettier()
+{
+  std::list list_int;
+  foreach_( int i, list_int ) {
+// do something with i
+  }
+  // CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based 
+  // CHECK-FIXES: for(int i : list_int ) {
+}
+
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -51,6 +51,7 @@
`android-cloexec-pipe2 `_,
`android-cloexec-socket `_,
`android-comparison-in-temp-failure-retry `_,
+   `boost-use-range-based-for-loop `_, "Yes"
`boost-use-to-string `_, "Yes"
`bugprone-argument-comment `_, "Yes"
`bugprone-assert-side-effect `_,
Index: clang-tools-extra/docs/clang-tidy/checks/boost-use-range-based-for-loop.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/boost-use-range-based-for-loop.rst
@@ -0,0 +1,79 @@
+.. title:: clang-tidy - boost-use-range-based-for-loop
+
+boost-use-range-based-for-loop
+==
+
+This check converts ``BOOST_FOREACH(..., ...)`` loops to use the new
+range-based loops in C++11.
+
+Two kinds of loops can be converted correctly:
+
+-  Loops over arrays.
+-  Loops over STL containers.
+
+Before:
+
+.. code-block:: c++
+
+std::list list_int( /*...*/ );
+BOOST_FOREACH( int i, list_int )
+{
+// do something with i
+}
+
+After:
+
+.. code-block:: c++
+
+std::list list_int( /*...*/ );
+for( int i: list_int )
+{
+// do something with i
+}
+
+Known Limitations
+-
+* Client code that use ``BOOST_FOREACH`` with:
+
+ - ``std::pair`` of iterators;
+ - Null-terminated strings (``char`` and ``wchar_t``);
+ - Some other undefined sequence types
+
+will produce broken code (compilation error). Please see list of correctly
+convertible loops above.
+
+.. code-block:: c++
+
+const char* cstr = "Hello world";
+BOOST_FOREACH( char c, cstr )
+{
+// do something with c
+}
+
+// Will be changed to
+const char* cstr = "Hello world";
+for( char c: cstr ) // won't compile
+{
+// do something with c
+}
+
+* First argument of the ``BOOST_FOREACH`` macro must be only new identifier
+  definition, all other will produce a compilation error after migration.
+
+.. code-block:: c++
+
+std::list list_int( /*...*/ );
+int i;
+BOOST_FOREACH( i, list_int )
+{
+// do something with i
+}
+
+// Will be changed to
+std::list list_int( /*...*/ );
+int i;
+for( i: list_int ) // won't compile
+{
+// do something with i
+}
+
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -76,6 +76,12 @@
 New checks
 ^^
 
+- New :doc:`boost-use-range-based-for-loop
+  ` check.
+
+  This check converts ``BOOST_FOREACH(..., ...)`` loops to use the new
+  range-based loops in C++11.
+
 - New :doc:`bugpro

[PATCH] D116577: [clang-tidy] Added "boost-use-range-based-for-loop" check

[Denis Mikhailov via Phabricator via cfe-commits]

denzor200 updated this revision to Diff 398517.
denzor200 added a comment.

clang-format


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D116577/new/

https://reviews.llvm.org/D116577

Files:
  clang-tools-extra/clang-tidy/boost/BoostTidyModule.cpp
  clang-tools-extra/clang-tidy/boost/CMakeLists.txt
  clang-tools-extra/clang-tidy/boost/UseRangeBasedForLoopCheck.cpp
  clang-tools-extra/clang-tidy/boost/UseRangeBasedForLoopCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/boost-use-range-based-for-loop.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/test/clang-tidy/checkers/boost-use-range-based-for-loop.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/boost-use-range-based-for-loop.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/boost-use-range-based-for-loop.cpp
@@ -0,0 +1,39 @@
+// RUN: %check_clang_tidy %s boost-use-range-based-for-loop %t
+
+namespace std {
+template 
+class list {};
+}
+
+#define BOOST_FOREACH(VAR, COL) while(true)
+
+void test_range_based_for_loop()
+{
+  std::list list_int;
+  BOOST_FOREACH( int i, list_int ) {
+// do something with i
+  }
+  // CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based for loop instead of BOOST_FOREACH [boost-use-range-based-for-loop]
+  // CHECK-FIXES: for(int i : list_int ) {
+
+  short array_short[] = {1,2,3};
+  BOOST_FOREACH(int
+val, array_short) {
+/// The short was implicitly converted to an int
+  }
+  // CHECK-MESSAGES: :[[@LINE-4]]:3: warning: use range-based
+  // CHECK-FIXES: for(int val : array_short) {
+}
+
+#define foreach_ BOOST_FOREACH
+
+void test_range_based_for_loop_prettier()
+{
+  std::list list_int;
+  foreach_( int i, list_int ) {
+// do something with i
+  }
+  // CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based 
+  // CHECK-FIXES: for(int i : list_int ) {
+}
+
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -51,6 +51,7 @@
`android-cloexec-pipe2 `_,
`android-cloexec-socket `_,
`android-comparison-in-temp-failure-retry `_,
+   `boost-use-range-based-for-loop `_, "Yes"
`boost-use-to-string `_, "Yes"
`bugprone-argument-comment `_, "Yes"
`bugprone-assert-side-effect `_,
Index: clang-tools-extra/docs/clang-tidy/checks/boost-use-range-based-for-loop.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/boost-use-range-based-for-loop.rst
@@ -0,0 +1,79 @@
+.. title:: clang-tidy - boost-use-range-based-for-loop
+
+boost-use-range-based-for-loop
+==
+
+This check converts ``BOOST_FOREACH(..., ...)`` loops to use the new
+range-based loops in C++11.
+
+Two kinds of loops can be converted correctly:
+
+-  Loops over arrays.
+-  Loops over STL containers.
+
+Before:
+
+.. code-block:: c++
+
+std::list list_int( /*...*/ );
+BOOST_FOREACH( int i, list_int )
+{
+// do something with i
+}
+
+After:
+
+.. code-block:: c++
+
+std::list list_int( /*...*/ );
+for( int i: list_int )
+{
+// do something with i
+}
+
+Known Limitations
+-
+* Client code that use ``BOOST_FOREACH`` with:
+
+ - ``std::pair`` of iterators;
+ - Null-terminated strings (``char`` and ``wchar_t``);
+ - Some other undefined sequence types
+
+will produce broken code (compilation error). Please see list of correctly
+convertible loops above.
+
+.. code-block:: c++
+
+const char* cstr = "Hello world";
+BOOST_FOREACH( char c, cstr )
+{
+// do something with c
+}
+
+// Will be changed to
+const char* cstr = "Hello world";
+for( char c: cstr ) // won't compile
+{
+// do something with c
+}
+
+* First argument of the ``BOOST_FOREACH`` macro must be only new identifier
+  definition, all other will produce a compilation error after migration.
+
+.. code-block:: c++
+
+std::list list_int( /*...*/ );
+int i;
+BOOST_FOREACH( i, list_int )
+{
+// do something with i
+}
+
+// Will be changed to
+std::list list_int( /*...*/ );
+int i;
+for( i: list_int ) // won't compile
+{
+// do something with i
+}
+
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -76,6 +76,12 @@
 New checks
 ^^
 
+- New :doc:`boost-use-range-based-for-loop
+  ` check.
+
+  This check converts ``BOOST_FOREACH(..., ...)`` loops to use the new
+  range-based loops in C++11.
+
 - New :doc:`bugprone-stringview-nullptr
   ` check.
 
In

[PATCH] D116577: [clang-tidy] Added "boost-use-range-based-for-loop" check

[Denis Mikhailov via Phabricator via cfe-commits]

denzor200 marked an inline comment as done.
denzor200 added inline comments.



Comment at: clang-tools-extra/clang-tidy/boost/CMakeLists.txt:8
   BoostTidyModule.cpp
+  UseRangeBasedForLoopCheck.cpp
   UseToStringCheck.cpp

LegalizeAdulthood wrote:
> I am wondering if this check is better placed in the modernize module?
> Maybe as an enhancement to the existing `modernize-loop-convert`?
I vote against the "modernize" section for this check. See arguments above. 



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/boost-use-range-based-for-loop.rst:30
+-
+* Client code that use ``BOOST_FOREACH`` with "``std::pair`` of iterators" or 
with "Null-terminated strings (``char`` and ``wchar_t``)" or some other 
sequence types (everything that is not declared in the list "convertible 
correctly" above) will produce broken code (compilation error).
+

LegalizeAdulthood wrote:
> Eugene.Zelenko wrote:
> > Please separate with newline and follow 80 character limit. Also closing 
> > quote for `Null-terminated strings` is missing.
> It would be better to detect known failure cases in the matcher and not issue 
> a fixit but perhaps issue a diagnostic.
I do not understand how I can determine type of expression `list_int` (for 
example) at the preprocessing stage. This trick was not found in any of the 
existing checks.



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/boost-use-range-based-for-loop.rst:47
+
+* First argument of the ``BOOST_FOREACH`` macro must be only new identifier 
definition, all other will produce a compilation error after migration.
+

LegalizeAdulthood wrote:
> Can't you detect this in the matcher and not issue a fixit in that case?
Yes, i can just parse the string token `char c` into mini AST and then 
determine if this is a variable declaration.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D116577/new/

https://reviews.llvm.org/D116577

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits