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

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added inline comments.


================
Comment at: clang-tools-extra/clang-tidy/boost/UseRangeBasedForLoopCheck.cpp:60
+
+    auto Loc = MacroNameToken.getLocation();
+    auto Diag = Check->diag(Loc, "use range-based for loop instead of %0")
----------------
Please specify type explicitly here.


================
Comment at: clang-tools-extra/clang-tidy/boost/UseRangeBasedForLoopCheck.h:22
+/// http://clang.llvm.org/extra/clang-tidy/checks/boost-use-range-based-for-loop.html
+class UseRangeBasedForLoopCheck : public ClangTidyCheck {
+public:
----------------
Please add `isLanguageVersionSupported`.


================
Comment at: clang-tools-extra/docs/clang-tidy/checks/boost-use-range-based-for-loop.rst:6
+
+This check converts ``BOOST_FOREACH(..., ...)`` loops to use the new range-based loops in C++11.
+
----------------
Please synchronize this statement with statement in Release Notes.


================
Comment at: clang-tools-extra/docs/clang-tidy/checks/boost-use-range-based-for-loop.rst:21
+
+    // Will be changed to
+    std::list<int> list_int( /*...*/ );
----------------
It will be good idea to move it to separate code block and add something like `from` and `to`. See `modernize` checks as example. Same for other examples.


================
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).
+
----------------
Please separate with newline and follow 80 character limit. Also closing quote for `Null-terminated strings` is missing.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116577