[PATCH] D100092: [clang-tidy] cppcoreguidelines-declare-loop-variable-in-the-initializer: a new check
Fabian Thurnheer via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Fri Apr 30 07:13:05 PDT 2021
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
More information about the cfe-commits
mailing list