[PATCH] D103188: [clang-tidy] modernize-loop-convert: limit use of auto

[Edward O via Phabricator via cfe-commits]

eddy-geek created this revision.
Herald added a subscriber: xazax.hun.
eddy-geek requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.

Fixes bug 35694 
Adds option AutoTypeNameLength, default 0 (keep current behaviour).
Note that the default could be set to 5 in the future
(like for modernize-use-auto.MinTypeNameLength).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D103188

Files:
  clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp
  clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.h
  clang-tools-extra/docs/clang-tidy/checks/modernize-loop-convert.rst
  clang-tools-extra/test/clang-tidy/checkers/modernize-loop-convert-auto.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/modernize-loop-convert-auto.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/modernize-loop-convert-auto.cpp
@@ -0,0 +1,37 @@
+// RUN: %check_clang_tidy %s modernize-loop-convert %t -- \
+// RUN:   -config="{CheckOptions: [{key: modernize-loop-convert.AutoTypeNameLength, value: '22'}]}" \
+// RUN:   -- -I %S/Inputs/modernize-loop-convert
+
+#include "structures.h"
+
+const int n = 10;
+int arr[n];
+int nums[n];
+
+void autotype() {
+  Val Teas[N];
+  for (int I = 0; I < N; ++I) {
+Teas[I].g();
+  }
+  // CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based for loop instead
+  // CHECK-FIXES: for (Tea & Tea : Teas)
+  // CHECK-FIXES-NEXT: Tea.g();
+
+  // NonTriviallyCopyable has length 21 < 22
+  const NonTriviallyCopyable NonCopy[N]{};
+  for (int I = 0; I < N; ++I) {
+printf("2 * %d = %d\n", NonCopy[I].X, NonCopy[I].X + NonCopy[I].X);
+  }
+  // CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based for loop instead
+  // CHECK-FIXES: for (const NonTriviallyCopyable & I : NonCopy)
+  // CHECK-FIXES-NEXT: printf("2 * %d = %d\n", I.X, I.X + I.X);
+
+  // TriviallyCopyableButBig has length 23 > 22
+  const TriviallyCopyableButBig Big[N]{};
+  for (int I = 0; I < N; ++I) {
+printf("2 * %d = %d\n", Big[I].X, Big[I].X + Big[I].X);
+  }
+  // CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based for loop instead
+  // CHECK-FIXES: for (const auto & I : Big)
+  // CHECK-FIXES-NEXT: printf("2 * %d = %d\n", I.X, I.X + I.X);
+}
Index: clang-tools-extra/docs/clang-tidy/checks/modernize-loop-convert.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/modernize-loop-convert.rst
+++ clang-tools-extra/docs/clang-tidy/checks/modernize-loop-convert.rst
@@ -121,23 +121,23 @@
 Reverse Iterator Support
 
 
-The converter is also capable of transforming iterator loops which use 
-``rbegin`` and ``rend`` for looping backwards over a container. Out of the box 
-this will automatically happen in C++20 mode using the ``ranges`` library, 
-however the check can be configured to work without C++20 by specifying a 
+The converter is also capable of transforming iterator loops which use
+``rbegin`` and ``rend`` for looping backwards over a container. Out of the box
+this will automatically happen in C++20 mode using the ``ranges`` library,
+however the check can be configured to work without C++20 by specifying a
 function to reverse a range and optionally the header file where that function
 lives.
 
 .. option:: UseCxx20ReverseRanges
-  
-   When set to true convert loops when in C++20 or later mode using 
+
+   When set to true convert loops when in C++20 or later mode using
``std::ranges::reverse_view``.
Default value is ``true``.
 
 .. option:: MakeReverseRangeFunction
 
-   Specify the function used to reverse an iterator pair, the function should 
-   accept a class with ``rbegin`` and ``rend`` methods and return a 
+   Specify the function used to reverse an iterator pair, the function should
+   accept a class with ``rbegin`` and ``rend`` methods and return a
class with ``begin`` and ``end`` methods methods that call the ``rbegin`` and
``rend`` methods respectively. Common examples are ``ranges::reverse_view``
and ``llvm::reverse``.
@@ -146,10 +146,10 @@
 .. option:: MakeReverseRangeHeader
 
Specifies the header file where :option:`MakeReverseRangeFunction` is
-   declared. For the previous examples this option would be set to 
+   declared. For the previous examples this option would be set to
``range/v3/view/reverse.hpp`` and ``llvm/ADT/STLExtras.h`` respectively.
-   If this is an empty string and :option:`MakeReverseRangeFunction` is set, 
-   the check will proceed on the assumption that the function is already 
+   If this is an empty string and :option:`MakeReverseRangeFunction` is set,
+   the check will proceed on the assumption that the function is already
available in the translation unit.
This can be wrapped in angle brackets to signify to add the include as a
syst

[PATCH] D103188: [clang-tidy] modernize-loop-convert: limit use of auto

[Edward O via Phabricator via cfe-commits]

eddy-geek added a comment.

I haven't been able to build locally so far -- OOM on my machine, not sure if I 
can do something lighter than:

  cmake -G Ninja -DLLVM_ENABLE_PROJECTS=clang\;clang-tools-extra 
-DCMAKE_BUILD_TYPE=Release -DCMAKE_INSTALL_PREFIX=$PWD 
-DCMAKE_EXPORT_COMPILE_COMMANDS=ON ../llvm
  ninja check-clang-tools

but I 'm still looking for feedback in the meantime.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103188

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

[PATCH] D103188: [clang-tidy] modernize-loop-convert: limit use of auto

[Edward O via Phabricator via cfe-commits]

eddy-geek updated this revision to Diff 348028.
eddy-geek added a comment.

Fix test build


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103188

Files:
  clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp
  clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.h
  clang-tools-extra/docs/clang-tidy/checks/modernize-loop-convert.rst
  clang-tools-extra/test/clang-tidy/checkers/modernize-loop-convert-auto.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/modernize-loop-convert-auto.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/modernize-loop-convert-auto.cpp
@@ -0,0 +1,35 @@
+// RUN: %check_clang_tidy %s modernize-loop-convert %t -- \
+// RUN:   -config="{CheckOptions: [{key: modernize-loop-convert.AutoTypeNameLength, value: '22'}]}" \
+// RUN:   -- -I %S/Inputs/modernize-loop-convert
+
+#include "structures.h"
+
+const int N = 6;
+
+void autotype() {
+  Val Teas[N];
+  for (int I = 0; I < N; ++I) {
+Teas[I].g();
+  }
+  // CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based for loop instead
+  // CHECK-FIXES: for (Tea & Tea : Teas)
+  // CHECK-FIXES-NEXT: Tea.g();
+
+  // NonTriviallyCopyable has length 21 < 22
+  const NonTriviallyCopyable NonCopy[N]{};
+  for (int I = 0; I < N; ++I) {
+printf("2 * %d = %d\n", NonCopy[I].X, NonCopy[I].X + NonCopy[I].X);
+  }
+  // CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based for loop instead
+  // CHECK-FIXES: for (const NonTriviallyCopyable & I : NonCopy)
+  // CHECK-FIXES-NEXT: printf("2 * %d = %d\n", I.X, I.X + I.X);
+
+  // TriviallyCopyableButBig has length 23 > 22
+  const TriviallyCopyableButBig Big[N]{};
+  for (int I = 0; I < N; ++I) {
+printf("2 * %d = %d\n", Big[I].X, Big[I].X + Big[I].X);
+  }
+  // CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based for loop instead
+  // CHECK-FIXES: for (const auto & I : Big)
+  // CHECK-FIXES-NEXT: printf("2 * %d = %d\n", I.X, I.X + I.X);
+}
Index: clang-tools-extra/docs/clang-tidy/checks/modernize-loop-convert.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/modernize-loop-convert.rst
+++ clang-tools-extra/docs/clang-tidy/checks/modernize-loop-convert.rst
@@ -121,23 +121,23 @@
 Reverse Iterator Support
 
 
-The converter is also capable of transforming iterator loops which use 
-``rbegin`` and ``rend`` for looping backwards over a container. Out of the box 
-this will automatically happen in C++20 mode using the ``ranges`` library, 
-however the check can be configured to work without C++20 by specifying a 
+The converter is also capable of transforming iterator loops which use
+``rbegin`` and ``rend`` for looping backwards over a container. Out of the box
+this will automatically happen in C++20 mode using the ``ranges`` library,
+however the check can be configured to work without C++20 by specifying a
 function to reverse a range and optionally the header file where that function
 lives.
 
 .. option:: UseCxx20ReverseRanges
-  
-   When set to true convert loops when in C++20 or later mode using 
+
+   When set to true convert loops when in C++20 or later mode using
``std::ranges::reverse_view``.
Default value is ``true``.
 
 .. option:: MakeReverseRangeFunction
 
-   Specify the function used to reverse an iterator pair, the function should 
-   accept a class with ``rbegin`` and ``rend`` methods and return a 
+   Specify the function used to reverse an iterator pair, the function should
+   accept a class with ``rbegin`` and ``rend`` methods and return a
class with ``begin`` and ``end`` methods methods that call the ``rbegin`` and
``rend`` methods respectively. Common examples are ``ranges::reverse_view``
and ``llvm::reverse``.
@@ -146,10 +146,10 @@
 .. option:: MakeReverseRangeHeader
 
Specifies the header file where :option:`MakeReverseRangeFunction` is
-   declared. For the previous examples this option would be set to 
+   declared. For the previous examples this option would be set to
``range/v3/view/reverse.hpp`` and ``llvm/ADT/STLExtras.h`` respectively.
-   If this is an empty string and :option:`MakeReverseRangeFunction` is set, 
-   the check will proceed on the assumption that the function is already 
+   If this is an empty string and :option:`MakeReverseRangeFunction` is set,
+   the check will proceed on the assumption that the function is already
available in the translation unit.
This can be wrapped in angle brackets to signify to add the include as a
system include.
@@ -160,6 +160,13 @@
A string specifying which include-style is used, `llvm` or `google`. Default
is `llvm`.
 
+AutoTypeNameLength option
+
+
+If TypeName length is strictly above this threshold, `auto` will be used.
+
+The default is 0, so `auto` will be use

[PATCH] D103188: [clang-tidy] modernize-loop-convert: limit use of auto

[Edward O via Phabricator via cfe-commits]

eddy-geek updated this revision to Diff 348038.
eddy-geek added a comment.

Fix test output


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103188

Files:
  clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp
  clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.h
  clang-tools-extra/docs/clang-tidy/checks/modernize-loop-convert.rst
  clang-tools-extra/test/clang-tidy/checkers/modernize-loop-convert-auto.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/modernize-loop-convert-auto.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/modernize-loop-convert-auto.cpp
@@ -0,0 +1,35 @@
+// RUN: %check_clang_tidy %s modernize-loop-convert %t -- \
+// RUN:   -config="{CheckOptions: [{key: modernize-loop-convert.AutoTypeNameLength, value: '22'}]}" \
+// RUN:   -- -I %S/Inputs/modernize-loop-convert
+
+#include "structures.h"
+
+const int N = 6;
+
+void autotype() {
+  Val Teas[N];
+  for (int I = 0; I < N; ++I) {
+Teas[I].g();
+  }
+  // CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based for loop instead
+  // CHECK-FIXES: for (Val & Tea : Teas)
+  // CHECK-FIXES-NEXT: Tea.g();
+
+  // NonTriviallyCopyable has length 21 < 22
+  const NonTriviallyCopyable NonCopy[N]{};
+  for (int I = 0; I < N; ++I) {
+printf("2 * %d = %d\n", NonCopy[I].X, NonCopy[I].X + NonCopy[I].X);
+  }
+  // CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based for loop instead
+  // CHECK-FIXES: for (const NonTriviallyCopyable & I : NonCopy)
+  // CHECK-FIXES-NEXT: printf("2 * %d = %d\n", I.X, I.X + I.X);
+
+  // TriviallyCopyableButBig has length 23 > 22
+  const TriviallyCopyableButBig Big[N]{};
+  for (int I = 0; I < N; ++I) {
+printf("2 * %d = %d\n", Big[I].X, Big[I].X + Big[I].X);
+  }
+  // CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based for loop instead
+  // CHECK-FIXES: for (const auto & I : Big)
+  // CHECK-FIXES-NEXT: printf("2 * %d = %d\n", I.X, I.X + I.X);
+}
Index: clang-tools-extra/docs/clang-tidy/checks/modernize-loop-convert.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/modernize-loop-convert.rst
+++ clang-tools-extra/docs/clang-tidy/checks/modernize-loop-convert.rst
@@ -121,23 +121,23 @@
 Reverse Iterator Support
 
 
-The converter is also capable of transforming iterator loops which use 
-``rbegin`` and ``rend`` for looping backwards over a container. Out of the box 
-this will automatically happen in C++20 mode using the ``ranges`` library, 
-however the check can be configured to work without C++20 by specifying a 
+The converter is also capable of transforming iterator loops which use
+``rbegin`` and ``rend`` for looping backwards over a container. Out of the box
+this will automatically happen in C++20 mode using the ``ranges`` library,
+however the check can be configured to work without C++20 by specifying a
 function to reverse a range and optionally the header file where that function
 lives.
 
 .. option:: UseCxx20ReverseRanges
-  
-   When set to true convert loops when in C++20 or later mode using 
+
+   When set to true convert loops when in C++20 or later mode using
``std::ranges::reverse_view``.
Default value is ``true``.
 
 .. option:: MakeReverseRangeFunction
 
-   Specify the function used to reverse an iterator pair, the function should 
-   accept a class with ``rbegin`` and ``rend`` methods and return a 
+   Specify the function used to reverse an iterator pair, the function should
+   accept a class with ``rbegin`` and ``rend`` methods and return a
class with ``begin`` and ``end`` methods methods that call the ``rbegin`` and
``rend`` methods respectively. Common examples are ``ranges::reverse_view``
and ``llvm::reverse``.
@@ -146,10 +146,10 @@
 .. option:: MakeReverseRangeHeader
 
Specifies the header file where :option:`MakeReverseRangeFunction` is
-   declared. For the previous examples this option would be set to 
+   declared. For the previous examples this option would be set to
``range/v3/view/reverse.hpp`` and ``llvm/ADT/STLExtras.h`` respectively.
-   If this is an empty string and :option:`MakeReverseRangeFunction` is set, 
-   the check will proceed on the assumption that the function is already 
+   If this is an empty string and :option:`MakeReverseRangeFunction` is set,
+   the check will proceed on the assumption that the function is already
available in the translation unit.
This can be wrapped in angle brackets to signify to add the include as a
system include.
@@ -160,6 +160,13 @@
A string specifying which include-style is used, `llvm` or `google`. Default
is `llvm`.
 
+AutoTypeNameLength option
+
+
+If TypeName length is strictly above this threshold, `auto` will be used.
+
+The default is 0, so `auto` will be us

[PATCH] D103188: [clang-tidy] modernize-loop-convert: limit use of auto

[Edward O via Phabricator via cfe-commits]

eddy-geek updated this revision to Diff 348047.
eddy-geek added a comment.

*Trigger rebuild (windows failed to git clone)*


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103188

Files:
  clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp
  clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.h
  clang-tools-extra/docs/clang-tidy/checks/modernize-loop-convert.rst
  clang-tools-extra/test/clang-tidy/checkers/modernize-loop-convert-auto.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/modernize-loop-convert-auto.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/modernize-loop-convert-auto.cpp
@@ -0,0 +1,35 @@
+// RUN: %check_clang_tidy %s modernize-loop-convert %t -- \
+// RUN:   -config="{CheckOptions: [{key: modernize-loop-convert.AutoTypeNameLength, value: '22'}]}" \
+// RUN:   -- -I %S/Inputs/modernize-loop-convert
+
+#include "structures.h"
+
+const int N = 6;
+
+void autotype() {
+  Val Teas[N];
+  for (int I = 0; I < N; ++I) {
+Teas[I].g();
+  }
+  // CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based for loop instead
+  // CHECK-FIXES: for (Val & Tea : Teas)
+  // CHECK-FIXES-NEXT: Tea.g();
+
+  // NonTriviallyCopyable has length 21 < 22
+  const NonTriviallyCopyable NonCopy[N]{};
+  for (int I = 0; I < N; ++I) {
+printf("2 * %d = %d\n", NonCopy[I].X, NonCopy[I].X + NonCopy[I].X);
+  }
+  // CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based for loop instead
+  // CHECK-FIXES: for (const NonTriviallyCopyable & I : NonCopy)
+  // CHECK-FIXES-NEXT: printf("2 * %d = %d\n", I.X, I.X + I.X);
+
+  // TriviallyCopyableButBig has length 23 > 22
+  const TriviallyCopyableButBig Big[N]{};
+  for (int I = 0; I < N; ++I) {
+printf("2 * %d = %d\n", Big[I].X, Big[I].X + Big[I].X);
+  }
+  // CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based for loop instead
+  // CHECK-FIXES: for (const auto & I : Big)
+  // CHECK-FIXES-NEXT: printf("2 * %d = %d\n", I.X, I.X + I.X);
+}
Index: clang-tools-extra/docs/clang-tidy/checks/modernize-loop-convert.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/modernize-loop-convert.rst
+++ clang-tools-extra/docs/clang-tidy/checks/modernize-loop-convert.rst
@@ -121,23 +121,23 @@
 Reverse Iterator Support
 
 
-The converter is also capable of transforming iterator loops which use 
-``rbegin`` and ``rend`` for looping backwards over a container. Out of the box 
-this will automatically happen in C++20 mode using the ``ranges`` library, 
-however the check can be configured to work without C++20 by specifying a 
+The converter is also capable of transforming iterator loops which use
+``rbegin`` and ``rend`` for looping backwards over a container. Out of the box
+this will automatically happen in C++20 mode using the ``ranges`` library,
+however the check can be configured to work without C++20 by specifying a
 function to reverse a range and optionally the header file where that function
 lives.
 
 .. option:: UseCxx20ReverseRanges
-  
-   When set to true convert loops when in C++20 or later mode using 
+
+   When set to true convert loops when in C++20 or later mode using
``std::ranges::reverse_view``.
Default value is ``true``.
 
 .. option:: MakeReverseRangeFunction
 
-   Specify the function used to reverse an iterator pair, the function should 
-   accept a class with ``rbegin`` and ``rend`` methods and return a 
+   Specify the function used to reverse an iterator pair, the function should
+   accept a class with ``rbegin`` and ``rend`` methods and return a
class with ``begin`` and ``end`` methods methods that call the ``rbegin`` and
``rend`` methods respectively. Common examples are ``ranges::reverse_view``
and ``llvm::reverse``.
@@ -146,10 +146,10 @@
 .. option:: MakeReverseRangeHeader
 
Specifies the header file where :option:`MakeReverseRangeFunction` is
-   declared. For the previous examples this option would be set to 
+   declared. For the previous examples this option would be set to
``range/v3/view/reverse.hpp`` and ``llvm/ADT/STLExtras.h`` respectively.
-   If this is an empty string and :option:`MakeReverseRangeFunction` is set, 
-   the check will proceed on the assumption that the function is already 
+   If this is an empty string and :option:`MakeReverseRangeFunction` is set,
+   the check will proceed on the assumption that the function is already
available in the translation unit.
This can be wrapped in angle brackets to signify to add the include as a
system include.
@@ -160,6 +160,13 @@
A string specifying which include-style is used, `llvm` or `google`. Default
is `llvm`.
 
+AutoTypeNameLength option
+
+
+If TypeName length is strictly above this threshold, `auto` will be used.
+
+The de

[PATCH] D103188: [clang-tidy] modernize-loop-convert: limit use of auto

[Edward O via Phabricator via cfe-commits]

eddy-geek updated this revision to Diff 348080.
eddy-geek added a comment.

*Trigger rebuild (windows failed to git clone)*


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103188

Files:
  
clang-tools-extra/clang-include-fixer/find-all-symbols/tool/run-find-all-symbols.py


Index: 
clang-tools-extra/clang-include-fixer/find-all-symbols/tool/run-find-all-symbols.py
===
--- 
clang-tools-extra/clang-include-fixer/find-all-symbols/tool/run-find-all-symbols.py
+++ 
clang-tools-extra/clang-include-fixer/find-all-symbols/tool/run-find-all-symbols.py
@@ -87,7 +87,7 @@
 
   # Load the database and extract all files.
   database = json.load(open(os.path.join(build_path, db_path)))
-  files = [entry['file'] for entry in database]
+  files = [os.path.join(entry['directory'], entry['file']) for entry in 
database]
 
   # Filter out .rc files on Windows. CMake includes them for some reason.
   files = [f for f in files if not f.endswith('.rc')]


Index: clang-tools-extra/clang-include-fixer/find-all-symbols/tool/run-find-all-symbols.py
===
--- clang-tools-extra/clang-include-fixer/find-all-symbols/tool/run-find-all-symbols.py
+++ clang-tools-extra/clang-include-fixer/find-all-symbols/tool/run-find-all-symbols.py
@@ -87,7 +87,7 @@
 
   # Load the database and extract all files.
   database = json.load(open(os.path.join(build_path, db_path)))
-  files = [entry['file'] for entry in database]
+  files = [os.path.join(entry['directory'], entry['file']) for entry in database]
 
   # Filter out .rc files on Windows. CMake includes them for some reason.
   files = [f for f in files if not f.endswith('.rc')]
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D103188: [clang-tidy] modernize-loop-convert: limit use of auto

[Edward O via Phabricator via cfe-commits]

eddy-geek updated this revision to Diff 348081.
eddy-geek added a comment.

*Trigger rebuild (windows failed to git clone)*


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103188

Files:
  clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp
  clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.h
  clang-tools-extra/docs/clang-tidy/checks/modernize-loop-convert.rst
  clang-tools-extra/test/clang-tidy/checkers/modernize-loop-convert-auto.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/modernize-loop-convert-auto.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/modernize-loop-convert-auto.cpp
@@ -0,0 +1,35 @@
+// RUN: %check_clang_tidy %s modernize-loop-convert %t -- \
+// RUN:   -config="{CheckOptions: [{key: modernize-loop-convert.AutoTypeNameLength, value: '22'}]}" \
+// RUN:   -- -I %S/Inputs/modernize-loop-convert
+
+#include "structures.h"
+
+const int N = 6;
+
+void autotype() {
+  Val Teas[N];
+  for (int I = 0; I < N; ++I) {
+Teas[I].g();
+  }
+  // CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based for loop instead
+  // CHECK-FIXES: for (Val & Tea : Teas)
+  // CHECK-FIXES-NEXT: Tea.g();
+
+  // NonTriviallyCopyable has length 21 < 22
+  const NonTriviallyCopyable NonCopy[N]{};
+  for (int I = 0; I < N; ++I) {
+printf("2 * %d = %d\n", NonCopy[I].X, NonCopy[I].X + NonCopy[I].X);
+  }
+  // CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based for loop instead
+  // CHECK-FIXES: for (const NonTriviallyCopyable & I : NonCopy)
+  // CHECK-FIXES-NEXT: printf("2 * %d = %d\n", I.X, I.X + I.X);
+
+  // TriviallyCopyableButBig has length 23 > 22
+  const TriviallyCopyableButBig Big[N]{};
+  for (int I = 0; I < N; ++I) {
+printf("2 * %d = %d\n", Big[I].X, Big[I].X + Big[I].X);
+  }
+  // CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based for loop instead
+  // CHECK-FIXES: for (const auto & I : Big)
+  // CHECK-FIXES-NEXT: printf("2 * %d = %d\n", I.X, I.X + I.X);
+}
Index: clang-tools-extra/docs/clang-tidy/checks/modernize-loop-convert.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/modernize-loop-convert.rst
+++ clang-tools-extra/docs/clang-tidy/checks/modernize-loop-convert.rst
@@ -121,23 +121,23 @@
 Reverse Iterator Support
 
 
-The converter is also capable of transforming iterator loops which use 
-``rbegin`` and ``rend`` for looping backwards over a container. Out of the box 
-this will automatically happen in C++20 mode using the ``ranges`` library, 
-however the check can be configured to work without C++20 by specifying a 
+The converter is also capable of transforming iterator loops which use
+``rbegin`` and ``rend`` for looping backwards over a container. Out of the box
+this will automatically happen in C++20 mode using the ``ranges`` library,
+however the check can be configured to work without C++20 by specifying a
 function to reverse a range and optionally the header file where that function
 lives.
 
 .. option:: UseCxx20ReverseRanges
-  
-   When set to true convert loops when in C++20 or later mode using 
+
+   When set to true convert loops when in C++20 or later mode using
``std::ranges::reverse_view``.
Default value is ``true``.
 
 .. option:: MakeReverseRangeFunction
 
-   Specify the function used to reverse an iterator pair, the function should 
-   accept a class with ``rbegin`` and ``rend`` methods and return a 
+   Specify the function used to reverse an iterator pair, the function should
+   accept a class with ``rbegin`` and ``rend`` methods and return a
class with ``begin`` and ``end`` methods methods that call the ``rbegin`` and
``rend`` methods respectively. Common examples are ``ranges::reverse_view``
and ``llvm::reverse``.
@@ -146,10 +146,10 @@
 .. option:: MakeReverseRangeHeader
 
Specifies the header file where :option:`MakeReverseRangeFunction` is
-   declared. For the previous examples this option would be set to 
+   declared. For the previous examples this option would be set to
``range/v3/view/reverse.hpp`` and ``llvm/ADT/STLExtras.h`` respectively.
-   If this is an empty string and :option:`MakeReverseRangeFunction` is set, 
-   the check will proceed on the assumption that the function is already 
+   If this is an empty string and :option:`MakeReverseRangeFunction` is set,
+   the check will proceed on the assumption that the function is already
available in the translation unit.
This can be wrapped in angle brackets to signify to add the include as a
system include.
@@ -160,6 +160,13 @@
A string specifying which include-style is used, `llvm` or `google`. Default
is `llvm`.
 
+AutoTypeNameLength option
+
+
+If TypeName length is strictly above this threshold, `auto` will be used.
+
+The de

[PATCH] D103188: [clang-tidy] modernize-loop-convert: limit use of auto

[Edward O via Phabricator via cfe-commits]

eddy-geek added a subscriber: sammccall.
eddy-geek added a comment.

Builds ok, ready for review. I can't edit reviewers now, can someone help? 
@sammccall ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103188

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

[PATCH] D103188: [clang-tidy] modernize-loop-convert: limit use of auto

[Edward O via Phabricator via cfe-commits]

eddy-geek added a subscriber: Eugene.Zelenko.
eddy-geek added a comment.

@Eugene.Zelenko wrote:

> Frankly, I don't think that length-based criteria is reasonable one. 
> Readability of code is much more important than line/file length.

It provides an added configuration flexibility without being more complex than 
a boolean. Arguably some people have found the similar 
`modernize-use-auto.MinTypeNameLength` useful (PR here 
)
 -- and I find its default of 5 makes sense as `MyTyp` can be argued as always 
better than `auto` even for people that like `auto`.

That said, I absolutely don't mind changing this PR to define a boolean option 
`UseAuto` instead, if requested.

//(otherwise I'll address review comments tomorrow. Thanks for the review!)//


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103188

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