[PATCH] D68912: Adds -Wrange-loop-analysis to -Wall

2020-01-19 Thread Mark de Wever via Phabricator via cfe-commits
Mordante added a comment. I proposed D73007 as fix. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68912/new/ https://reviews.llvm.org/D68912 ___ cfe-commits mailing

[PATCH] D68912: Adds -Wrange-loop-analysis to -Wall

2020-01-18 Thread Mark de Wever via Phabricator via cfe-commits
Mordante added a comment. Thanks for your report. I think it's similar to https://bugs.llvm.org/show_bug.cgi?id=44556. I'll look for a fix. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68912/new/ https://reviews.llvm.org/D68912

[PATCH] D68912: Adds -Wrange-loop-analysis to -Wall

2020-01-17 Thread Denis Nikitin via Phabricator via cfe-commits
denik added a comment. We are hitting the warning in template code with bool and loop like this: "for (const auto& element : value)" where element is bool from template. But not always. When it is bool the warning triggers: error: loop variable 'element' is always a copy because the range of

[PATCH] D68912: Adds -Wrange-loop-analysis to -Wall

2020-01-01 Thread Mark de Wever via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGd8117542ac57: Adds -Wrange-loop-analysis to -Wall (authored by Mordante). Changed prior to commit: https://reviews.llvm.org/D68912?vs=226539=235783#toc Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D68912: Adds -Wrange-loop-analysis to -Wall

2019-12-22 Thread Mark de Wever via Phabricator via cfe-commits
Mordante added a comment. In D68912#1794138 , @xbolva00 wrote: > All is fixed now? Not yet :-( I ran into a false positive with rvalue references, will post a code fix for it and then I'll look into posting more error fixes. CHANGES SINCE LAST ACTION

[PATCH] D68912: Adds -Wrange-loop-analysis to -Wall

2019-12-22 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 added a comment. All is fixed now? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68912/new/ https://reviews.llvm.org/D68912 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D68912: Adds -Wrange-loop-analysis to -Wall

2019-11-24 Thread Mark de Wever via Phabricator via cfe-commits
Mordante added a comment. In D68912#1757850 , @xbolva00 wrote: > In D68912#1754660 , @xbolva00 wrote: > > > I think you can commit it, you already fixed some warnings. > > > Oh, I hit this (-Werror bots) hard :D

[PATCH] D68912: Adds -Wrange-loop-analysis to -Wall

2019-11-23 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 added a comment. In D68912#1754660 , @xbolva00 wrote: > I think you can commit it, you already fixed some warnings. Oh, I hit this (-Werror bots) hard :D Some bots use -Werror. So when build is warning-free, then please commit this patch.

[PATCH] D68912: Adds -Wrange-loop-analysis to -Wall

2019-11-21 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 added a comment. I think you can commit it, you already fixed some warnings. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68912/new/ https://reviews.llvm.org/D68912 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D68912: Adds -Wrange-loop-analysis to -Wall

2019-11-09 Thread Mark de Wever via Phabricator via cfe-commits
Mordante added a comment. In D68912#1737559 , @xbolva00 wrote: > Formally unblocking this Thanks. I'll first want to reduce the number of warnings before committing this patch. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68912/new/

[PATCH] D68912: Adds -Wrange-loop-analysis to -Wall

2019-11-07 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 accepted this revision. xbolva00 added a comment. This revision is now accepted and ready to land. Formally unblocking this CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68912/new/ https://reviews.llvm.org/D68912 ___ cfe-commits

[PATCH] D68912: Adds -Wrange-loop-analysis to -Wall

2019-11-07 Thread Mark de Wever via Phabricator via cfe-commits
Mordante added a comment. Thanks for the review. I also looked at some of the new warnings and I'm preparing patches to fix them. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68912/new/ https://reviews.llvm.org/D68912 ___ cfe-commits

[PATCH] D68912: Adds -Wrange-loop-analysis to -Wall

2019-11-07 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman accepted this revision. aaron.ballman added a comment. I spot-checked the list and definitely saw some true positives in there. I did not exhaustively check the list, however. The timing numbers look reasonable enough as well. Unless someone finds an issue with the fp rate that

[PATCH] D68912: Adds -Wrange-loop-analysis to -Wall

2019-10-29 Thread Mark de Wever via Phabricator via cfe-commits
Mordante added a comment. I haven't reviewed the list [1] properly so I don't know the number of false positives yet. [1] http://paste.debian.net/806/ CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68912/new/ https://reviews.llvm.org/D68912

[PATCH] D68912: Adds -Wrange-loop-analysis to -Wall

2019-10-29 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 added a comment. > I compiled LLVM and Clang and it adds 145 unique warnings. I wouldn't mind to > fix those and the other subprojects, so I can test whether the tests have > false positives. > Am I correct to assume these patches all need to be reviewed? Can you upload the list with

[PATCH] D68912: Adds -Wrange-loop-analysis to -Wall

2019-10-29 Thread Mark de Wever via Phabricator via cfe-commits
Mordante added a comment. In D68912#1723722 , @aaron.ballman wrote: > In D68912#1723691 , @xbolva00 wrote: > > > >> Does this analysis require CFG support > > > > https://reviews.llvm.org/D69292 also requires CFG

[PATCH] D68912: Adds -Wrange-loop-analysis to -Wall

2019-10-28 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. In D68912#1723691 , @xbolva00 wrote: > >> Does this analysis require CFG support > > https://reviews.llvm.org/D69292 also requires CFG support. Yes, it does. > Generally, is CFG support ok for -Wall if the warning has few

[PATCH] D68912: Adds -Wrange-loop-analysis to -Wall

2019-10-28 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 requested changes to this revision. xbolva00 added a comment. This revision now requires changes to proceed. >> Does this analysis require CFG support https://reviews.llvm.org/D69292 also requires CFG support. Generally, is CFG support ok for -Wall if the warning has few false

[PATCH] D68912: Adds -Wrange-loop-analysis to -Wall

2019-10-28 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. Does this analysis require CFG support or have a high false positive rate? We typically keep those out of `-Wall` because of performance concerns, so I am wondering if the diagnostic was kept out of `-Wall` for a reason. CHANGES SINCE LAST ACTION

[PATCH] D68912: Adds -Wrange-loop-analysis to -Wall

2019-10-26 Thread Mark de Wever via Phabricator via cfe-commits
Mordante updated this revision to Diff 226539. Mordante added a comment. Adds a test as requested by @xbolva00 . CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68912/new/ https://reviews.llvm.org/D68912 Files: clang/include/clang/Basic/DiagnosticGroups.td

[PATCH] D68912: Adds -Wrange-loop-analysis to -Wall

2019-10-23 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 added a comment. Needs a test Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68912/new/ https://reviews.llvm.org/D68912 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D68912: Adds -Wrange-loop-analysis to -Wall

2019-10-12 Thread Mark de Wever via Phabricator via cfe-commits
Mordante created this revision. Mordante added reviewers: rsmith, rtrieu, xbolva00. Mordante added a project: clang. This makes the range loop warnings part of -Wall. This 'fixes' https://bugs.llvm.org/show_bug.cgi?id=32823 Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D68912