[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 <https://reviews.llvm.org/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<VarDecl>(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