[PATCH] D104112: [clang-tidy] cppcoreguidelines-avoid-init-default-constructors: a new check

[Fabian Thurnheer via Phabricator via cfe-commits]

DNS320 created this revision.
DNS320 added reviewers: aaron.ballman, njames93, alexfh.
DNS320 added a project: clang-tools-extra.
Herald added subscribers: shchenz, kbarton, xazax.hun, mgorny, nemanjai.
DNS320 requested review of this revision.
Herald added a subscriber: cfe-commits.

This check tries to implement C.45 

 of the C++ Core Guidelines.
During the implementation of the current state of this check, it was found that 
multiple functionalities, which the check also could use, already exists in 
other clang-tidy checks. These are:

- Lookup if the body of a constructor (only) initialize class members 
(cppcoreguidelines-prefer-member-initializer)
- Set a constructor `=default` if possible (hicpp-use-equals-default)
- If possible, move/change constructor list initializers to class member 
initializers (modernize-use-default-member-init)

I would like to ask how I could proceed, as reimplementing already existing 
functionality of other checks might not be preferred, because of reasons like 
duplication and maintainability. But I think it would be beneficial if a check 
exists that combines the mentioned functionalities under the 
`cppcoreguidelines` module.

Generally, do you see a need for this check at all or is the activation of the 
three checks mentioned above the way to go to implicitly enforce C.45?


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D104112

Files:
  
clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidInitDefaultConstructorsCheck.cpp
  
clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidInitDefaultConstructorsCheck.h
  clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt
  clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-avoid-init-default-constructors.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-avoid-init-default-constructors.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-avoid-init-default-constructors.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-avoid-init-default-constructors.cpp
@@ -0,0 +1,66 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-avoid-init-default-constructors %t
+
+class NoDefaultCtor {
+  int z;
+
+  NoDefaultCtor(int a) : z{1} {}
+};
+
+class HasBody {
+  int z;
+
+  HasBody() : z{1} { ; }
+};
+
+class OneParameterOneInitList {
+  int z;
+  int y;
+
+  OneParameterOneInitList(int a = 1) : z{a}, y{1} {}
+};
+
+// Test needed because class member initializations are listed as ``cxxCtorInitializer`` in clang's AST
+class OnlyInClassMemberInit {
+  int z{1};
+  int y = 1;
+
+  OnlyInClassMemberInit() {}
+};
+
+// User may wants explicitly overwrite the class member value in the constructor
+class ExplicitOverwrite {
+  int z{1};
+
+  ExplicitOverwrite() : z{2} {}
+};
+
+class HasBodyInit {
+  int z;
+
+  HasBodyInit() {
+z = 1;
+  }
+};
+
+class OneInitList {
+  int z;
+
+  OneInitList() : z{1} {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: Default constructors like 'OneInitList' can be omitted. Use in-class member initializers instead [cppcoreguidelines-avoid-init-default-constructors]
+};
+
+class OneDirectInit {
+  int z;
+
+  OneDirectInit() : z(1) {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: Default constructors like 'OneDirectInit' can be omitted. Use in-class member initializers instead [cppcoreguidelines-avoid-init-default-constructors]
+};
+
+class OneInitListOneDirectInit {
+  int z;
+  int y;
+  int w;
+
+  OneInitListOneDirectInit() : z{1}, y(2) {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: Default constructors like 'OneInitListOneDirectInit' can be omitted. Use in-class member initializers instead [cppcoreguidelines-avoid-init-default-constructors]
+};
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
@@ -145,6 +145,7 @@
`concurrency-mt-unsafe `_,
`concurrency-thread-canceltype-asynchronous `_,
`cppcoreguidelines-avoid-goto `_,
+   `cppcoreguidelines-avoid-init-default-constructors `_,
`cppcoreguidelines-avoid-non-const-global-variables `_,
`cppcoreguidelines-init-variables `_, "Yes"
`cppcoreguidelines-interfaces-global-init `_,
Index: clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-avoid-init-default-constructors.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-avoid-init-default-constructors.rst
@@ -0,0 +1,26 @@
+.. title:: clang-tidy - 

[PATCH] D102576: [clang-tidy] cppcoreguidelines-avoid-do-while: a new check

[Fabian Thurnheer via Phabricator via cfe-commits]

DNS320 added a comment.

Friendly Ping.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102576

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

[PATCH] D100092: [clang-tidy] cppcoreguidelines-declare-loop-variable-in-the-initializer: a new check

[Fabian Thurnheer via Phabricator via cfe-commits]

DNS320 added a comment.

Dear reviewers,

Since this work was conducted as part of a bachelor's thesis, which has to be
handed in on the 18th of June 2021, we wanted to add a friendly ping to this
patch.

It would make us, the thesis team, proud to state that some of our work was
accepted and included into the LLVM project.

Therefore, we wanted to ask if we can further improve anything in this patch
for it to get a LGTM?

Thanks a lot in advance!
Yours faithfully,
Marco & Fabian


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100092

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

[PATCH] D102576: [clang-tidy] cppcoreguidelines-avoid-do-while: a new check

[Fabian Thurnheer via Phabricator via cfe-commits]

DNS320 updated this revision to Diff 346360.
DNS320 added a comment.

I updated the check according to the last review.

It ignores now do-while statements which are inside a macro and have a false 
condition. (false condition part was taken from bugprone-terminating-continue 
)

Tests were added and the check documentation has now a hint about ignoring 
do-while statements in macros.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102576

Files:
  clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidDoWhileCheck.cpp
  clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidDoWhileCheck.h
  clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt
  clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-avoid-do-while.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-avoid-do-while.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-avoid-do-while.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-avoid-do-while.cpp
@@ -0,0 +1,87 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-avoid-do-while %t
+
+void func1() {
+  int I{0};
+  const int Limit{10};
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: Try to avoid using do-while loops in terms of readability. Consider using a while loop instead. [cppcoreguidelines-avoid-do-while]
+  do {
+I++;
+  } while (I <= Limit);
+}
+
+void func2() {
+  int I{0};
+  const int Limit{10};
+
+  // OK
+  while (I <= Limit) {
+I++;
+  }
+}
+
+void func3() {
+#define MACRO \
+  do {\
+  } while (true)
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: Try to avoid using do-while loops in terms of readability. Consider using a while loop instead. [cppcoreguidelines-avoid-do-while]
+  MACRO;
+
+#undef MACRO
+}
+
+void func4() {
+#define MACRO \
+  do {\
+  } while (1)
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: Try to avoid using do-while loops in terms of readability. Consider using a while loop instead. [cppcoreguidelines-avoid-do-while]
+  MACRO;
+
+#undef MACRO
+}
+
+void func5() {
+#define MACRO \
+  do {\
+  } while (false)
+
+  // OK
+  MACRO;
+
+#undef MACRO
+}
+
+void func6() {
+#define MACRO \
+  do {\
+  } while (0)
+
+  // OK
+  MACRO;
+
+#undef MACRO
+}
+
+void func7() {
+#define MACRO \
+  do {\
+  } while (nullptr)
+
+  // OK
+  MACRO;
+
+#undef MACRO
+}
+
+void func8() {
+#define MACRO \
+  do {\
+  } while (__null)
+
+  // OK
+  MACRO;
+
+#undef MACRO
+}
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
@@ -144,6 +144,7 @@
`clang-analyzer-valist.Unterminated `_,
`concurrency-mt-unsafe `_,
`concurrency-thread-canceltype-asynchronous `_,
+   `cppcoreguidelines-avoid-do-while `_,
`cppcoreguidelines-avoid-goto `_,
`cppcoreguidelines-avoid-non-const-global-variables `_,
`cppcoreguidelines-init-variables `_, "Yes"
Index: clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-avoid-do-while.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-avoid-do-while.rst
@@ -0,0 +1,36 @@
+.. title:: clang-tidy - cppcoreguidelines-avoid-do-while
+
+cppcoreguidelines-avoid-do-while
+
+
+Checks if a ``do-while`` loop exists and flags it.
+
+Using a ``while`` loop instead of a ``do-while`` could improve readability and
+prevents overlooking the condition at the end.
+
+Usages of ``do-while`` loops inside a macro definition are not flagged by this
+check if the condition is either ``false``, ``0``, ``nullptr`` or ``__null``.
+
+.. code-block:: c++
+
+  void func1() {
+int I{0};
+const int Limit{10};
+
+// Consider using a while loop
+do {
+  I++;
+} while (I <= Limit);
+  }
+
+  void func2() {
+int I{0};
+const int Limit{10};
+
+// Better
+while (I <= Limit) {
+  I++;
+}
+  }
+
+This check implements rule `ES.75 `_ of the C++ Core Guidelines.
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -108,6 +108,11 @@
   Finds inner loops that have not been unrolled, as well as fully unrolled
   loops with unknown loops 

[PATCH] D102576: [clang-tidy] cppcoreguidelines-avoid-do-while: a new check

[Fabian Thurnheer via Phabricator via cfe-commits]

DNS320 created this revision.
DNS320 added reviewers: aaron.ballman, njames93, alexfh.
DNS320 added a project: clang-tools-extra.
Herald added subscribers: shchenz, kbarton, xazax.hun, mgorny, nemanjai.
DNS320 requested review of this revision.

This simple check flags do-while statements according to rule ES.75 

 of the C++ Core Guidelines.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D102576

Files:
  clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidDoWhileCheck.cpp
  clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidDoWhileCheck.h
  clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt
  clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-avoid-do-while.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-avoid-do-while.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-avoid-do-while.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-avoid-do-while.cpp
@@ -0,0 +1,21 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-avoid-do-while %t
+
+void func1() {
+  int I{0};
+  const int Limit{10};
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: Try to avoid using do-while loops in terms of readability. Consider using a while loop instead. [cppcoreguidelines-avoid-do-while]
+  do {
+I++;
+  } while (I <= Limit);
+}
+
+void func2() {
+  int I{0};
+  const int Limit{10};
+
+  // OK
+  while (I <= Limit) {
+I++;
+  }
+}
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
@@ -144,6 +144,7 @@
`clang-analyzer-valist.Unterminated `_,
`concurrency-mt-unsafe `_,
`concurrency-thread-canceltype-asynchronous `_,
+   `cppcoreguidelines-avoid-do-while `_,
`cppcoreguidelines-avoid-goto `_,
`cppcoreguidelines-avoid-non-const-global-variables `_,
`cppcoreguidelines-init-variables `_, "Yes"
Index: clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-avoid-do-while.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-avoid-do-while.rst
@@ -0,0 +1,32 @@
+.. title:: clang-tidy - cppcoreguidelines-avoid-do-while
+
+cppcoreguidelines-avoid-do-while
+
+
+Checks if a do-while loop exists and flags it.
+
+Using a while loop instead of a do-while could improve readability and prevents overlooking the condition at the end.
+
+.. code-block:: c++
+
+  void func1() {
+int I{0};
+const int Limit{10};
+
+// Consider using a while loop
+do {
+  I++;
+} while (I <= Limit);
+  }
+
+  void func2() {
+int I{0};
+const int Limit{10};
+
+// Better
+while (I <= Limit) {
+  I++;
+}
+  }
+
+This check implements rule `ES.75 `_ of the C++ Core Guidelines.
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -108,6 +108,11 @@
   Finds inner loops that have not been unrolled, as well as fully unrolled
   loops with unknown loops bounds or a large number of iterations.
 
+- New :doc:`cppcoreguidelines-avoid-do-while
+  ` check.
+
+  Checks if a do-while loop exists and flags it.
+
 - New :doc:`cppcoreguidelines-prefer-member-initializer
   ` check.
 
Index: clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
===
--- clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
+++ clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
@@ -14,6 +14,7 @@
 #include "../modernize/AvoidCArraysCheck.h"
 #include "../modernize/UseOverrideCheck.h"
 #include "../readability/MagicNumbersCheck.h"
+#include "AvoidDoWhileCheck.h"
 #include "AvoidGotoCheck.h"
 #include "AvoidNonConstGlobalVariablesCheck.h"
 #include "InitVariablesCheck.h"
@@ -46,6 +47,8 @@
   void addCheckFactories(ClangTidyCheckFactories ) override {
 CheckFactories.registerCheck(
 "cppcoreguidelines-avoid-c-arrays");
+CheckFactories.registerCheck(
+"cppcoreguidelines-avoid-do-while");
 CheckFactories.registerCheck(
 "cppcoreguidelines-avoid-goto");
 CheckFactories.registerCheck(
Index: clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt

[PATCH] D100092: [clang-tidy] cppcoreguidelines-declare-loop-variable-in-the-initializer: a new check

[Fabian Thurnheer via Phabricator via cfe-commits]

DNS320 updated this revision to Diff 341929.
DNS320 added a comment.

Removed a link to the ES.26 C++ Core Guideline in the documentation part.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100092

Files:
  clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt
  clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
  
clang-tools-extra/clang-tidy/cppcoreguidelines/DeclareLoopVariableInTheInitializerCheck.cpp
  
clang-tools-extra/clang-tidy/cppcoreguidelines/DeclareLoopVariableInTheInitializerCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-declare-loop-variable-in-the-initializer.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-declare-loop-variable-in-the-initializer.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-declare-loop-variable-in-the-initializer.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-declare-loop-variable-in-the-initializer.cpp
@@ -0,0 +1,79 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-declare-loop-variable-in-the-initializer %t
+
+const int Limit{10};
+
+void func1() {
+  // CHECK-MESSAGES: :[[@LINE+2]]:7: warning: Variable 'A' is only modified in a for statement and not used elsewhere. Consider declaring it inside the for statement. [cppcoreguidelines-declare-loop-variable-in-the-initializer]
+  // CHECK-MESSAGES: :11:5: note: Variable gets modified here
+  int A{0};
+
+  for (int I{0}; I < Limit; I++) {
+A = 3;
+  }
+}
+
+void func2() {
+  // CHECK-MESSAGES: :[[@LINE+2]]:7: warning: Variable 'A' is only modified in a for statement and not used elsewhere. Consider declaring it inside the for statement. [cppcoreguidelines-declare-loop-variable-in-the-initializer]
+  // CHECK-MESSAGES: :21:5: note: Variable gets modified here
+  int A{0};
+
+  for (int I{0}; I < Limit; I++) {
+A += 3;
+  }
+}
+
+void func3() {
+  // CHECK-MESSAGES: :[[@LINE+2]]:7: warning: Variable 'I' is only modified in a for statement and not used elsewhere. Consider declaring it inside the for statement. [cppcoreguidelines-declare-loop-variable-in-the-initializer]
+  // CHECK-MESSAGES: :30:8: note: Variable gets modified here
+  int I{0};
+
+  for (I = 1; I < Limit; I++) {
+  }
+}
+
+void func4() {
+  // CHECK-MESSAGES: :[[@LINE+2]]:7: warning: Variable 'I' is only modified in a for statement and not used elsewhere. Consider declaring it inside the for statement. [cppcoreguidelines-declare-loop-variable-in-the-initializer]
+  // CHECK-MESSAGES: :39:21: note: Variable gets modified here
+  int I{0};
+
+  for (; I < Limit; I++) {
+  }
+}
+
+void func5() {
+  // CHECK-MESSAGES: :[[@LINE+2]]:7: warning: Variable 'I' is only modified in a for statement and not used elsewhere. Consider declaring it inside the for statement. [cppcoreguidelines-declare-loop-variable-in-the-initializer]
+  // CHECK-MESSAGES: :48:34: note: Variable gets modified here
+  int I{0};
+
+  for (int Unused{0}; I < Limit; I++) {
+  }
+}
+
+void func6() {
+  // CHECK-MESSAGES: :[[@LINE+2]]:7: warning: Variable 'I' is only modified in a for statement and not used elsewhere. Consider declaring it inside the for statement. [cppcoreguidelines-declare-loop-variable-in-the-initializer]
+  // CHECK-MESSAGES: :58:8: note: Variable gets modified here
+  int I{0};
+  int Z{0};
+
+  for (I = 3; I < Limit; I++) {
+  }
+  Z = 3;
+}
+
+void func7() {
+  // OK
+  int A{0};
+
+  for (int I{0}; I < Limit; I++) {
+const int B{A};
+  }
+}
+
+void func8() {
+  // OK
+  int I{0};
+
+  for (I = 0; I < Limit; I++) {
+  }
+  const int A{I};
+}
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
@@ -143,6 +143,7 @@
`concurrency-thread-canceltype-asynchronous `_,
`cppcoreguidelines-avoid-goto `_,
`cppcoreguidelines-avoid-non-const-global-variables `_,
+   `cppcoreguidelines-declare-loop-variable-in-the-initializer `_,
`cppcoreguidelines-init-variables `_, "Yes"
`cppcoreguidelines-interfaces-global-init `_,
`cppcoreguidelines-macro-usage `_,
Index: clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-declare-loop-variable-in-the-initializer.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-declare-loop-variable-in-the-initializer.rst
@@ -0,0 +1,49 @@
+.. title:: clang-tidy - cppcoreguidelines-declare-loop-variable-in-the-initializer
+
+cppcoreguidelines-declare-loop-variable-in-the-initializer
+==
+
+Finds variables that are 

[PATCH] D100092: [clang-tidy] cppcoreguidelines-declare-loop-variable-in-the-initializer: a new check

[Fabian Thurnheer via Phabricator via cfe-commits]

DNS320 marked an inline comment as done.
DNS320 added a comment.

In D100092#2726808 , @steveire wrote:

> I implemented something like this recently too. The code I was trying to 
> refactor was something like
>
>   auto ii = 0;
>   for (ii = 0; ii < thing.size(); ++ii)
>   {
>   // code
>   }
>   // code
>   for (ii = 0; ii < thing.size(); ++ii)
>   {
>   // code
>   }
>   // code
>   for (ii = 0; ii < thing.size(); ++ii)
>   {
>   // code
>   }
>
> ie, the variable was used repeatedly, but always initialized in the loop.
>
> I also had code like
>
>   auto ii = 0;
>   for (ii = 0; ii < thing.size(); ++ii)
>   {
>   // code
>   }
>   // code
>   ii = getNumber();
>   doSomething(ii);
>
> ie, the next statement referring to `ii` outside the loop was an assignment, 
> so I could just change it to
>
>   for (auto ii = 0; ii < thing.size(); ++ii)
>   {
>   // code
>   }
>   // code
>   auto ii = getNumber();
>   doSomething(ii);
>
> You don't necessarily have to handle these cases in the initial version of 
> this check, but you could consider them in the future.

Thank you for your comment!
I agree with you and I can see the need to cover such code with a style check.
The initial check was contributed to implement the ES.74 C++ Core Guideline. As 
I think, your code examples should be covered by a check that implements the 
ES.26 rule of the C++ Core Guidelines, which could be implemented in the future.
Due to your comment, I noticed the link to the ES.26 rule in the description of 
this check is not accurate, so I will remove it.




Comment at: 
clang-tools-extra/clang-tidy/cppcoreguidelines/DeclareLoopVariableInTheInitializerCheck.cpp:37
+  bool VisitDeclRefExpr(DeclRefExpr *D) {
+if (const VarDecl *To = dyn_cast(D->getDecl())) {
+  if (To == MatchedDecl &&

Eugene.Zelenko wrote:
> `const auto*` could be used because type is spelled in same statement.
Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100092

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

[PATCH] D100092: [clang-tidy] cppcoreguidelines-declare-loop-variable-in-the-initializer: a new check

[Fabian Thurnheer via Phabricator via cfe-commits]

DNS320 updated this revision to Diff 341784.
DNS320 added a comment.

Renamed the IsInsideMatchedForStmt() function according to a comment from the 
build system.
Changed a data type to "auto" according to a comment from Eugene.Zelenko.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100092

Files:
  clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt
  clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
  
clang-tools-extra/clang-tidy/cppcoreguidelines/DeclareLoopVariableInTheInitializerCheck.cpp
  
clang-tools-extra/clang-tidy/cppcoreguidelines/DeclareLoopVariableInTheInitializerCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-declare-loop-variable-in-the-initializer.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-declare-loop-variable-in-the-initializer.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-declare-loop-variable-in-the-initializer.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-declare-loop-variable-in-the-initializer.cpp
@@ -0,0 +1,79 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-declare-loop-variable-in-the-initializer %t
+
+const int Limit{10};
+
+void func1() {
+  // CHECK-MESSAGES: :[[@LINE+2]]:7: warning: Variable 'A' is only modified in a for statement and not used elsewhere. Consider declaring it inside the for statement. [cppcoreguidelines-declare-loop-variable-in-the-initializer]
+  // CHECK-MESSAGES: :11:5: note: Variable gets modified here
+  int A{0};
+
+  for (int I{0}; I < Limit; I++) {
+A = 3;
+  }
+}
+
+void func2() {
+  // CHECK-MESSAGES: :[[@LINE+2]]:7: warning: Variable 'A' is only modified in a for statement and not used elsewhere. Consider declaring it inside the for statement. [cppcoreguidelines-declare-loop-variable-in-the-initializer]
+  // CHECK-MESSAGES: :21:5: note: Variable gets modified here
+  int A{0};
+
+  for (int I{0}; I < Limit; I++) {
+A += 3;
+  }
+}
+
+void func3() {
+  // CHECK-MESSAGES: :[[@LINE+2]]:7: warning: Variable 'I' is only modified in a for statement and not used elsewhere. Consider declaring it inside the for statement. [cppcoreguidelines-declare-loop-variable-in-the-initializer]
+  // CHECK-MESSAGES: :30:8: note: Variable gets modified here
+  int I{0};
+
+  for (I = 1; I < Limit; I++) {
+  }
+}
+
+void func4() {
+  // CHECK-MESSAGES: :[[@LINE+2]]:7: warning: Variable 'I' is only modified in a for statement and not used elsewhere. Consider declaring it inside the for statement. [cppcoreguidelines-declare-loop-variable-in-the-initializer]
+  // CHECK-MESSAGES: :39:21: note: Variable gets modified here
+  int I{0};
+
+  for (; I < Limit; I++) {
+  }
+}
+
+void func5() {
+  // CHECK-MESSAGES: :[[@LINE+2]]:7: warning: Variable 'I' is only modified in a for statement and not used elsewhere. Consider declaring it inside the for statement. [cppcoreguidelines-declare-loop-variable-in-the-initializer]
+  // CHECK-MESSAGES: :48:34: note: Variable gets modified here
+  int I{0};
+
+  for (int Unused{0}; I < Limit; I++) {
+  }
+}
+
+void func6() {
+  // CHECK-MESSAGES: :[[@LINE+2]]:7: warning: Variable 'I' is only modified in a for statement and not used elsewhere. Consider declaring it inside the for statement. [cppcoreguidelines-declare-loop-variable-in-the-initializer]
+  // CHECK-MESSAGES: :58:8: note: Variable gets modified here
+  int I{0};
+  int Z{0};
+
+  for (I = 3; I < Limit; I++) {
+  }
+  Z = 3;
+}
+
+void func7() {
+  // OK
+  int A{0};
+
+  for (int I{0}; I < Limit; I++) {
+const int B{A};
+  }
+}
+
+void func8() {
+  // OK
+  int I{0};
+
+  for (I = 0; I < Limit; I++) {
+  }
+  const int A{I};
+}
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
@@ -143,6 +143,7 @@
`concurrency-thread-canceltype-asynchronous `_,
`cppcoreguidelines-avoid-goto `_,
`cppcoreguidelines-avoid-non-const-global-variables `_,
+   `cppcoreguidelines-declare-loop-variable-in-the-initializer `_,
`cppcoreguidelines-init-variables `_, "Yes"
`cppcoreguidelines-interfaces-global-init `_,
`cppcoreguidelines-macro-usage `_,
Index: clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-declare-loop-variable-in-the-initializer.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-declare-loop-variable-in-the-initializer.rst
@@ -0,0 +1,50 @@
+.. title:: clang-tidy - cppcoreguidelines-declare-loop-variable-in-the-initializer
+

[PATCH] D100092: [clang-tidy] cppcoreguidelines-declare-loop-variable-in-the-initializer: a new check

[Fabian Thurnheer via Phabricator via cfe-commits]

DNS320 added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/cppcoreguidelines/DeclareLoopVariableInTheInitializerCheck.cpp:28
+
+  diag(MatchedForStmt->getBeginLoc(),
+   "Prefer to declare a loop variable in the initializer part of a "

Eugene.Zelenko wrote:
> It'll be reasonable to add add note with actual variable declaration.
The hole check was refactored. A diagnostic hint points now to the statement 
which modifies the variable.




Comment at: 
clang-tools-extra/clang-tidy/cppcoreguidelines/DeclareLoopVariableInTheInitializerCheck.h:1
+//===--- DeclareLoopVariableInTheInitializerCheck.h - clang-tidy *- C++ 
-*-===//
+//

Eugene.Zelenko wrote:
> I may be mistaken, but proper code is `-*- C++ -*-`. Please remove  dash at 
> left side.
Thanks. I checked other header files with long names. I correct it this way, to 
match the other ones:
```
//===--- DeclareLoopVariableInTheInitializerCheck.h - clang-tidy-*- C++ -*-===//
//===--- ProBoundsArrayToPointerDecayCheck.h - clang-tidy*- C++ -*-===//
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100092

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

[PATCH] D100092: [clang-tidy] cppcoreguidelines-declare-loop-variable-in-the-initializer: a new check

[Fabian Thurnheer via Phabricator via cfe-commits]

DNS320 updated this revision to Diff 341492.
DNS320 marked 7 inline comments as done.
DNS320 added a comment.

I updated the check according to the last review.
The check should now fully implement the defined enforcement chapter from the 
ES.74 C++ Core Guideline.

About searching for declRefExpr outside the for statement:
I limited the search to the next ancestor compoundStmt of the varDecl, so the 
Visitor must not traverse the whole translation unit.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100092

Files:
  clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt
  clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
  
clang-tools-extra/clang-tidy/cppcoreguidelines/DeclareLoopVariableInTheInitializerCheck.cpp
  
clang-tools-extra/clang-tidy/cppcoreguidelines/DeclareLoopVariableInTheInitializerCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-declare-loop-variable-in-the-initializer.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-declare-loop-variable-in-the-initializer.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-declare-loop-variable-in-the-initializer.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-declare-loop-variable-in-the-initializer.cpp
@@ -0,0 +1,79 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-declare-loop-variable-in-the-initializer %t
+
+const int Limit{10};
+
+void func1() {
+  // CHECK-MESSAGES: :[[@LINE+2]]:7: warning: Variable 'A' is only modified in a for statement and not used elsewhere. Consider declaring it inside the for statement. [cppcoreguidelines-declare-loop-variable-in-the-initializer]
+  // CHECK-MESSAGES: :11:5: note: Variable gets modified here
+  int A{0};
+
+  for (int I{0}; I < Limit; I++) {
+A = 3;
+  }
+}
+
+void func2() {
+  // CHECK-MESSAGES: :[[@LINE+2]]:7: warning: Variable 'A' is only modified in a for statement and not used elsewhere. Consider declaring it inside the for statement. [cppcoreguidelines-declare-loop-variable-in-the-initializer]
+  // CHECK-MESSAGES: :21:5: note: Variable gets modified here
+  int A{0};
+
+  for (int I{0}; I < Limit; I++) {
+A += 3;
+  }
+}
+
+void func3() {
+  // CHECK-MESSAGES: :[[@LINE+2]]:7: warning: Variable 'I' is only modified in a for statement and not used elsewhere. Consider declaring it inside the for statement. [cppcoreguidelines-declare-loop-variable-in-the-initializer]
+  // CHECK-MESSAGES: :30:8: note: Variable gets modified here
+  int I{0};
+
+  for (I = 1; I < Limit; I++) {
+  }
+}
+
+void func4() {
+  // CHECK-MESSAGES: :[[@LINE+2]]:7: warning: Variable 'I' is only modified in a for statement and not used elsewhere. Consider declaring it inside the for statement. [cppcoreguidelines-declare-loop-variable-in-the-initializer]
+  // CHECK-MESSAGES: :39:21: note: Variable gets modified here
+  int I{0};
+
+  for (; I < Limit; I++) {
+  }
+}
+
+void func5() {
+  // CHECK-MESSAGES: :[[@LINE+2]]:7: warning: Variable 'I' is only modified in a for statement and not used elsewhere. Consider declaring it inside the for statement. [cppcoreguidelines-declare-loop-variable-in-the-initializer]
+  // CHECK-MESSAGES: :48:34: note: Variable gets modified here
+  int I{0};
+
+  for (int Unused{0}; I < Limit; I++) {
+  }
+}
+
+void func6() {
+  // CHECK-MESSAGES: :[[@LINE+2]]:7: warning: Variable 'I' is only modified in a for statement and not used elsewhere. Consider declaring it inside the for statement. [cppcoreguidelines-declare-loop-variable-in-the-initializer]
+  // CHECK-MESSAGES: :58:8: note: Variable gets modified here
+  int I{0};
+  int Z{0};
+
+  for (I = 3; I < Limit; I++) {
+  }
+  Z = 3;
+}
+
+void func7() {
+  // OK
+  int A{0};
+
+  for (int I{0}; I < Limit; I++) {
+const int B{A};
+  }
+}
+
+void func8() {
+  // OK
+  int I{0};
+
+  for (I = 0; I < Limit; I++) {
+  }
+  const int A{I};
+}
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
@@ -143,6 +143,7 @@
`concurrency-thread-canceltype-asynchronous `_,
`cppcoreguidelines-avoid-goto `_,
`cppcoreguidelines-avoid-non-const-global-variables `_,
+   `cppcoreguidelines-declare-loop-variable-in-the-initializer `_,
`cppcoreguidelines-init-variables `_, "Yes"
`cppcoreguidelines-interfaces-global-init `_,
`cppcoreguidelines-macro-usage `_,
Index: clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-declare-loop-variable-in-the-initializer.rst
===
--- /dev/null
+++ 

[PATCH] D100092: [clang-tidy] cppcoreguidelines-declare-loop-variable-in-the-initializer: a new check

[Fabian Thurnheer via Phabricator via cfe-commits]

DNS320 added a comment.

Thank you for your reviews.
I will work on your comments and write back soon.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100092

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

[PATCH] D100092: [clang-tidy] cppcoreguidelines-declare-loop-variable-in-the-initializer: a new check

[Fabian Thurnheer via Phabricator via cfe-commits]

DNS320 created this revision.
DNS320 added reviewers: aaron.ballman, njames93, alexfh.
DNS320 added a project: clang-tools-extra.
Herald added subscribers: shchenz, kbarton, xazax.hun, mgorny, nemanjai.
DNS320 requested review of this revision.

This check implements the ES.74 
 
rule of the C++ Core Guidelines.

The goal of this check is to find for-statements which do not declare their 
loop variable in the initializer part.
This limits the loop variables visibility to the scope of the loop and prevents 
using the loop variable for other purposes after the loop.

Thank you for the review.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D100092

Files:
  clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt
  clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
  
clang-tools-extra/clang-tidy/cppcoreguidelines/DeclareLoopVariableInTheInitializerCheck.cpp
  
clang-tools-extra/clang-tidy/cppcoreguidelines/DeclareLoopVariableInTheInitializerCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-declare-loop-variable-in-the-initializer.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-declare-loop-variable-in-the-initializer.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-declare-loop-variable-in-the-initializer.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-declare-loop-variable-in-the-initializer.cpp
@@ -0,0 +1,17 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-declare-loop-variable-in-the-initializer %t
+
+void forLoopFunction() {
+  const int Limit{10};
+  int I{0};
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: Prefer to declare a loop variable in the initializer part of a for-statement [cppcoreguidelines-declare-loop-variable-in-the-initializer]
+  for (; I < Limit; I++) {
+  }
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: Prefer to declare a loop variable in the initializer part of a for-statement [cppcoreguidelines-declare-loop-variable-in-the-initializer]
+  for (I = 0; I < Limit; I++) {
+  }
+
+  for (int I{0}; I < Limit; I++) { // OK
+  }
+}
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
@@ -143,6 +143,7 @@
`concurrency-thread-canceltype-asynchronous `_,
`cppcoreguidelines-avoid-goto `_,
`cppcoreguidelines-avoid-non-const-global-variables `_,
+   `cppcoreguidelines-declare-loop-variable-in-the-initializer `_,
`cppcoreguidelines-init-variables `_, "Yes"
`cppcoreguidelines-interfaces-global-init `_,
`cppcoreguidelines-macro-usage `_,
Index: clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-declare-loop-variable-in-the-initializer.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-declare-loop-variable-in-the-initializer.rst
@@ -0,0 +1,33 @@
+.. title:: clang-tidy - cppcoreguidelines-declare-loop-variable-in-the-initializer
+
+cppcoreguidelines-declare-loop-variable-in-the-initializer
+==
+
+Checks if a loop variable is declared in the initializer part of a for-statement.
+
+This check implements the rule `ES.74 `_ of the C++ Core Guidelines.
+
+It does also cover parts of:
+- `ES.26 `_
+- `ES.5 `_
+- `ES.6 `_
+
+
+.. code-block:: c++
+
+void forLoopFunction() {
+const int Limit{10};
+int I{0};
+
+// Should print a warning
+for (; I < Limit; I++) {
+}
+
+// Should print a warning
+for (I = 0; I < Limit; I++) {
+}
+
+// Good
+for (int I{0}; I < Limit; I++) {
+}
+}
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -89,6 +89,11 @@
   Finds inner loops that have not been unrolled, as well as fully unrolled
   loops with unknown loops bounds or a large number of