[PATCH] D73270: [clang-tidy] Fix false positive in bugprone-infinite-loop

2020-01-27 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr2 added a comment. Please talk to Hans Wennborg about cherry-picking this change into the release. I think it is a safe change, if Hans needs that sort of review from someone. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73270/new/ htt

[PATCH] D73270: [clang-tidy] Fix false positive in bugprone-infinite-loop

2020-01-27 Thread Balogh, Ádám via Phabricator via cfe-commits
baloghadamsoftware added a comment. Should I also commit it to the release 10.0 branch? This is a bugfix so it should be fixed in the release, I think. I added a dependency between this bug and the 10.0.0 release blockers. Is that enough? Repository: rG LLVM Github Monorepo CHANGES SINCE LA

[PATCH] D73270: [clang-tidy] Fix false positive in bugprone-infinite-loop

2020-01-27 Thread Balogh, Ádám via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG70f4c6e7b14f: [clan-tidy] Fix false positive in bugprone-infinite-loop (authored by baloghadamsoftware). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73270/

[PATCH] D73270: [clang-tidy] Fix false positive in bugprone-infinite-loop

2020-01-24 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. In D73270#1838956 , @gribozavr2 wrote: > In D73270#1838883 , @njames93 wrote: > > > May not be one for this patch, but how does this check handle volatile loop > > variables and cases wher

[PATCH] D73270: [clang-tidy] Fix false positive in bugprone-infinite-loop

2020-01-24 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr2 added a comment. In D73270#1838883 , @njames93 wrote: > May not be one for this patch, but how does this check handle volatile loop > variables and cases where modification isn't visible in the context e.g. Should be OK, see "if (Var->getType

[PATCH] D73270: [clang-tidy] Fix false positive in bugprone-infinite-loop

2020-01-24 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. May not be one for this patch, but how does this check handle volatile loop variables and cases where modification isn't visible in the context e.g. extern void mutate(bool &); volatile bool* LoopCond; void foo() { for (bool Cond = true; Cond; mutate(Cond))

[PATCH] D73270: [clang-tidy] Fix false positive in bugprone-infinite-loop

2020-01-24 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr2 accepted this revision. gribozavr2 added a comment. This revision is now accepted and ready to land. LGTM! Please commit if you have commit access, or let me know if I should push it for you. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73270/new/ https://reviews.llvm.org/D

[PATCH] D73270: [clang-tidy] Fix false positive in bugprone-infinite-loop

2020-01-24 Thread Balogh, Ádám via Phabricator via cfe-commits
baloghadamsoftware updated this revision to Diff 240195. baloghadamsoftware added a comment. Minor fix. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73270/new/ https://reviews.llvm.org/D73270 Files: clang-tools-extra/clang-tidy/bugprone/InfiniteLoopCheck.cpp clang-tools-extra/test

[PATCH] D73270: [clang-tidy] Fix false positive in bugprone-infinite-loop

2020-01-24 Thread Balogh, Ádám via Phabricator via cfe-commits
baloghadamsoftware marked 3 inline comments as done. baloghadamsoftware added inline comments. Comment at: clang-tools-extra/clang-tidy/bugprone/InfiniteLoopCheck.cpp:186 +} + } + gribozavr2 wrote: > baloghadamsoftware wrote: > > gribozavr2 wrote: > > > I th

[PATCH] D73270: [clang-tidy] Fix false positive in bugprone-infinite-loop

2020-01-24 Thread Balogh, Ádám via Phabricator via cfe-commits
baloghadamsoftware updated this revision to Diff 240192. baloghadamsoftware added a comment. Updated according to the comments. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73270/new/ https://reviews.llvm.org/D73270 Files: clang-tools-extra/clang-tidy/bugprone/InfiniteLoopCheck.cpp

[PATCH] D73270: [clang-tidy] Fix false positive in bugprone-infinite-loop

2020-01-23 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr2 added inline comments. Comment at: clang-tools-extra/clang-tidy/bugprone/InfiniteLoopCheck.cpp:186 +} + } + baloghadamsoftware wrote: > gribozavr2 wrote: > > I think this logic should go into `isChanged()`, similarly to how it > > handles for loo

[PATCH] D73270: [clang-tidy] Fix false positive in bugprone-infinite-loop

2020-01-23 Thread Balogh, Ádám via Phabricator via cfe-commits
baloghadamsoftware updated this revision to Diff 239927. baloghadamsoftware added a comment. Updated according to the comments. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73270/new/ https://reviews.llvm.org/D73270 Files: clang-tools-extra/clang-tidy/bugprone/InfiniteLoopCheck.cpp

[PATCH] D73270: [clang-tidy] Fix false positive in bugprone-infinite-loop

2020-01-23 Thread Balogh, Ádám via Phabricator via cfe-commits
baloghadamsoftware marked 2 inline comments as done. baloghadamsoftware added inline comments. Comment at: clang-tools-extra/clang-tidy/bugprone/InfiniteLoopCheck.cpp:186 +} + } + gribozavr2 wrote: > I think this logic should go into `isChanged()`, similarly

[PATCH] D73270: [clang-tidy] Fix false positive in bugprone-infinite-loop

2020-01-23 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr2 added inline comments. Comment at: clang-tools-extra/clang-tidy/bugprone/InfiniteLoopCheck.cpp:186 +} + } + I think this logic should go into `isChanged()`, similarly to how it handles for loops today. Comment at: clang-tools-e

[PATCH] D73270: [clang-tidy] Fix false positive in bugprone-infinite-loop

2020-01-23 Thread Nathan James via Phabricator via cfe-commits
njames93 added inline comments. Comment at: clang-tools-extra/clang-tidy/bugprone/InfiniteLoopCheck.cpp:132 } + return false; Don't change formatting of code this patch doesn't need to touch Comment at: clang-tools-extra/clang-tidy/bugpro

[PATCH] D73270: [clang-tidy] Fix false positive in bugprone-infinite-loop

2020-01-23 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added inline comments. Comment at: clang-tools-extra/clang-tidy/bugprone/InfiniteLoopCheck.cpp:179 + if (const auto *While = dyn_cast(LoopStmt)) { +if (const auto *LoopVarDecl = While->getConditionVariable()) { + if (const Expr *Init = LoopVarDecl->getIni