[PATCH] D82089: [clang-tidy] modernize-loop-convert reverse iteration support

Eugene Zelenko via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Jun 18 07:02:41 PDT 2020


Eugene.Zelenko added a comment.

It'll be interesting to run improved check over LLVM code base.



================
Comment at: clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp:983
+    const LangOptions &LangOpts)
+    : IsEnabled(false), UseCxx20IfAvailable(UseCxx20IfAvailable),
+      Function(FunctionName), Header(HeaderName) {
----------------
I think //IsEnabled// should be initialized as member or in constructor body because of non-trivial logic.


================
Comment at: clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp:989
+        << "' is set but '" << MakeReverseRangeFunction
+        << "' is not, Disabling reverse loop transformation\n";
+    return;
----------------
Something need to be changed in:  //not, Disabling// - comma to dot or capitalization.


================
Comment at: clang-tools-extra/docs/clang-tidy/checks/modernize-loop-convert.rst:123
+------------------------
+The converter is also capable of transforming iterator loops which use 
+``rbegin`` and ``rend`` for looping backwards over a container. Out of the box 
----------------
Please separate with empty line.


================
Comment at: clang-tools-extra/docs/clang-tidy/checks/modernize-loop-convert.rst:141
+   class with ``begin`` and ``end`` methods methods that call the ``rbegin`` and
+   ``rend`` methods respectively. Common examples are ranges::reverse_view and 
+   llvm::reverse.
----------------
Please highlight ranges::reverse_view and llvm::reverse with double back-ticks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82089





More information about the cfe-commits mailing list