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

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

In D61827#1499347 <https://reviews.llvm.org/D61827#1499347>, @hintonda wrote:

> In D61827#1499335 <https://reviews.llvm.org/D61827#1499335>, @lebedev.ri wrote:
>
> > In D61827#1499333 <https://reviews.llvm.org/D61827#1499333>, @hintonda wrote:
> >
> > > When I asked for a test above, I understood you to say you couldn't provide one, but If I misunderstood, by all means, please add the test.
> >
> >
> > Please do note that i have provided a testcase (godbolt link) in my very first comment, and quoted that line when replying the previous time.
> >  (Granted, that loop is not in a correct form for openmp, but the point being, the current check does not diagnose it either)
>
>
> I'm really not sure I understand what you are trying to say.




> Are you saying this patch is a regression?

Not in general, no. This is most certainly an improvement for normal C++ code.

> By "false positive", do you mean that diagnoses something it shouldn't change and creates an erroneous fixit?

For normal C++ - not that i know of, can't comment.

For loops that are OpenMP loops - yes, this will be a false-positive with erroneous, compilation-breaking, fixit.
(To be noted, the existing fix-its in this check already are that way.)

But as i said, this is only "for your information", only to be added to docs (if it isn't there already).
As i have said, this can not be reliably avoided, and i don't believe a partial avoidance will be good.

> Of that it ignores something it should fix?

That is called false-negative. No, this isn't the case here.


Repository:
  rCTE Clang Tools Extra

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D61827/new/

https://reviews.llvm.org/D61827