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

[Torbjörn Klatt via Phabricator via cfe-commits]

torbjoernk added a comment.

In D61827#1499184 <https://reviews.llvm.org/D61827#1499184>, @lebedev.ri wrote:

> In D61827#1499183 <https://reviews.llvm.org/D61827#1499183>, @hintonda wrote:
>
> > In D61827#1499160 <https://reviews.llvm.org/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 range-for loop is not available until OpenMP 5.
> > >
> > > I don't think this false-positive can be avoided though, if building without
> > >  `-fopenmp` there won't be anything about OpenMP in AST,
> > >  and thus no way to detect this case..
> >
> >
> > Could you suggest a simple test case that could be added to the test?  That way, instead of just removing the `if else` block, @torbjoernk could try to handle it.  Or perhaps exclude it from the match altogether.
>
>
> As i said, i don't see how this can be avoided in general.


I have to admit that I have very little experience with OpenMP and haven't thought of this at all. Thank you very much for bringing this up.

Would it help to extend the exclusion AST matcher for iterator-based loops by an exclusion for loops with an ancestor of `ompExecutableDirective`?:

  return forStmt(
               unless(anyOf(isInTemplateInstantiation(),
                            hasAncestor(ompExecutableDirective()))),


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D61827