[PATCH] D61827: [clang-tidy] modernize-loop-convert: impl const cast iter

lebedev.ri added a comment. By the way, much to my surprise, this didn't start diagnosing the loop i expected it to start diagnosing: https://godbolt.org/z/lsJTSS This is expected? Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61827/new/

[PATCH] D61827: [clang-tidy] modernize-loop-convert: impl const cast iter

This revision was automatically updated to reflect the committed changes. Closed by commit rCTE360785: [clang-tidy] modernize-loop-convert: impl const cast iter (authored by dhinton, committed by ). Changed prior to commit: https://reviews.llvm.org/D61827?vs=199496=199631#toc Repository:

[PATCH] D61827: [clang-tidy] modernize-loop-convert: impl const cast iter

torbjoernk updated this revision to Diff 199496. torbjoernk added a comment. minor rewording CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61827/new/ https://reviews.llvm.org/D61827 Files: clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp

[PATCH] D61827: [clang-tidy] modernize-loop-convert: impl const cast iter

hintonda added a comment. I'll be happy to commit for you, but will give it till tomorrow just to make sure no one else has any additional comments. Thanks again! Comment at: clang-tools-extra/docs/clang-tidy/checks/modernize-loop-convert.rst:262 +not been used on code with

[PATCH] D61827: [clang-tidy] modernize-loop-convert: impl const cast iter

torbjoernk updated this revision to Diff 199483. torbjoernk edited the summary of this revision. torbjoernk added a comment. Herald added a subscriber: arphaman. Addressed Roman Lebedev's comment regarding reference to `NOLINT` in the docs. As I don't have commit rights, I'd be grateful someone

[PATCH] D61827: [clang-tidy] modernize-loop-convert: impl const cast iter

hintonda accepted this revision. hintonda added a comment. This revision is now accepted and ready to land. Awesome, thanks... LGTM, but you may want to give @alexfh or @aaron.ballman a day to comment before you commit. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61827/new/

[PATCH] D61827: [clang-tidy] modernize-loop-convert: impl const cast iter

lebedev.ri added a comment. Sounds good, thank you! Comment at: clang-tools-extra/docs/clang-tidy/checks/modernize-loop-convert.rst:262 +not been used on code with a compatibility requirements of OpenMP prior to +version 5. I would add a cross-reference to

[PATCH] D61827: [clang-tidy] modernize-loop-convert: impl const cast iter

torbjoernk updated this revision to Diff 199326. torbjoernk edited the summary of this revision. torbjoernk added a comment. Herald added subscribers: jdoerfert, jfb. Fixed the issues pointed out by Don Hinton and added note on OpenMP to the check's docs as suggested by Roman Lebedev. Also

[PATCH] D61827: [clang-tidy] modernize-loop-convert: impl const cast iter

hintonda added a comment. In D61827#1499368 , @lebedev.ri wrote: > In D61827#1499347 , @hintonda wrote: > > > > >> Are you saying this patch is a regression? > > Not in general, no. This is most certainly an

[PATCH] D61827: [clang-tidy] modernize-loop-convert: impl const cast iter

hintonda added a comment. I pulled down you patch, compiled and ran it. Once I fixed the two problems I mentioned, it ran clean, e.g.: $ bin/llvm-lit -vv ../../llvm-project/clang-tools-extra/test/clang-tidy/modernize-loop-convert-[be]*.cpp -- Testing: 2 tests, 2 threads -- PASS: Clang

[PATCH] D61827: [clang-tidy] modernize-loop-convert: impl const cast iter

lebedev.ri added a comment. In D61827#1499347 , @hintonda wrote: > In D61827#1499335 , @lebedev.ri > wrote: > > > In D61827#1499333 , @hintonda > > wrote: > > > > > When I

[PATCH] D61827: [clang-tidy] modernize-loop-convert: impl const cast iter

hintonda added a comment. In D61827#1499335 , @lebedev.ri wrote: > In D61827#1499333 , @hintonda wrote: > > > When I asked for a test above, I understood you to say you couldn't provide > > one, but If I

[PATCH] D61827: [clang-tidy] modernize-loop-convert: impl const cast iter

lebedev.ri added a comment. In D61827#1499333 , @hintonda wrote: > In D61827#1499309 , @lebedev.ri > wrote: > > > In D61827#1499306 , @hintonda > > wrote: > > > > > In

[PATCH] D61827: [clang-tidy] modernize-loop-convert: impl const cast iter

hintonda added a comment. In D61827#1499309 , @lebedev.ri wrote: > In D61827#1499306 , @hintonda wrote: > > > In D61827#1499303 , @torbjoernk > > wrote: > > > > > In

[PATCH] D61827: [clang-tidy] modernize-loop-convert: impl const cast iter

lebedev.ri added a comment. In D61827#1499306 , @hintonda wrote: > In D61827#1499303 , @torbjoernk > wrote: > > > In D61827#1499184 , @lebedev.ri > > wrote: > > > > > In

[PATCH] D61827: [clang-tidy] modernize-loop-convert: impl const cast iter

hintonda added a comment. In D61827#1499303 , @torbjoernk wrote: > In D61827#1499184 , @lebedev.ri > wrote: > > > In D61827#1499183 , @hintonda > > wrote: > > > > > In

[PATCH] D61827: [clang-tidy] modernize-loop-convert: impl const cast iter

lebedev.ri added a comment. In D61827#1499303 , @torbjoernk wrote: > In D61827#1499184 , @lebedev.ri > wrote: > > > In D61827#1499183 , @hintonda > > wrote: > > > > > In

[PATCH] D61827: [clang-tidy] modernize-loop-convert: impl const cast iter

torbjoernk added a comment. In D61827#1499184 , @lebedev.ri wrote: > In D61827#1499183 , @hintonda wrote: > > > In D61827#1499160 , @lebedev.ri > > wrote: > > > > > This

[PATCH] D61827: [clang-tidy] modernize-loop-convert: impl const cast iter

torbjoernk added inline comments. Comment at: clang-tools-extra/test/clang-tidy/modernize-loop-convert-basic.cpp:273 + for (S::const_iterator It = Ss.begin(), E = Ss.end(); It != E; ++It) { +printf("s has value %d\n", (*It).X); hintonda wrote: > I think

[PATCH] D61827: [clang-tidy] modernize-loop-convert: impl const cast iter

hintonda added a comment. In D61827#1499160 , @lebedev.ri wrote: > This will now trigger on https://godbolt.org/z/9oFMcB right? > Just want to point out that this will then have "false-positives" when that > loop > is an OpenMP for loop, since

[PATCH] D61827: [clang-tidy] modernize-loop-convert: impl const cast iter

lebedev.ri added a comment. In D61827#1499183 , @hintonda wrote: > In D61827#1499160 , @lebedev.ri > wrote: > > > This will now trigger on https://godbolt.org/z/9oFMcB right? > > Just want to point out that this

[PATCH] D61827: [clang-tidy] modernize-loop-convert: impl const cast iter

lebedev.ri added a comment. This will now trigger on https://godbolt.org/z/9oFMcB right? Just want to point out that this will then have "false-positives" when that loop is an OpenMP for loop, since range-for loop is not available until OpenMP 5. I don't think this false-positive can be avoided

[PATCH] D61827: [clang-tidy] modernize-loop-convert: impl const cast iter

hintonda added inline comments. Comment at: clang-tools-extra/test/clang-tidy/modernize-loop-convert-basic.cpp:273 + for (S::const_iterator It = Ss.begin(), E = Ss.end(); It != E; ++It) { +printf("s has value %d\n", (*It).X); I think you need to fix the

[PATCH] D61827: [clang-tidy] modernize-loop-convert: impl const cast iter

torbjoernk created this revision. torbjoernk added reviewers: dergachev.a, hintonda, alexfh. torbjoernk added a project: clang-tools-extra. Herald added subscribers: cfe-commits, xazax.hun. Herald added a project: clang. modernize-loop-convert was not detecting implicit casts to const_iterator as