[PATCH] D80961: Ignore template instantiations if not in AsIs mode

[Stephen Kelly via Phabricator via cfe-commits]

steveire added a comment.

In D80961#2079419 <https://reviews.llvm.org/D80961#2079419>, @aaron.ballman wrote:

> In D80961#2079044 <https://reviews.llvm.org/D80961#2079044>, @klimek wrote:
>
> > In D80961#2076242 <https://reviews.llvm.org/D80961#2076242>, @aaron.ballman wrote:
> >
> > > In D80961#2073049 <https://reviews.llvm.org/D80961#2073049>, @klimek wrote:
> > >
> > > > Without jumping into the discussion whether it should be the default, I think we should be able to control template instantiation visitation separately from other implicit nodes.
> > > >  Having to put AsIs on a matcher every time you need to match template instantiations is a rather big change (suddenly you have to change all the matchers you've written so far).
> > >
> > >
> > > I think that's the intended meaning of `AsIs` though. Template instantiations are not source code the user wrote, they're source code the compiler stamped out from code the user wrote. I hope `IgnoreUnlessSpelledInSource` isn't a misnomer.
> > >
> > > > I love the idea of being able to control visitation of template instantiation.
> > > >  I am somewhat torn on whether it should be the default, and would like to see more data.
> > > >  I feel more strongly about needing AsIs when I want to match template instantiations.
> > >
> > > FWIW, my experience in clang-tidy has been that template instantiations are ignored far more often than they're desired. In fact, instantiations tend to be a source of bugs for us because they're easy to forget about when writing matchers without keeping templates in mind. The times when template instantiations become important to *not* ignore within the checks is when the check is specific to template behavior, but that's a minority of the public checks thus far.
> >
> >
> > So I assume the idea is that this will work & be what we'll want people to write?
> >  traverse(AsIs, decl(traverse(IgnoreImplicit, hasFoo)))
>
>
> I believe so, yes. It's explicit about which traversal mode you want any given set of matchers to match with, so it is more flexible at the expense of being more verbose. Alternatively, you could be in `AsIs` mode for all of it and explicitly ignore the implicit nodes you wish to ignore (which may be even more verbose with the same results, depending on the matchers involved).


As far as I can tell, the people who had "standing objections" to fixing this bug have been convinced that it makes sense. At least that is implied.

The problem is they haven't explicitly reversed so progress is still blocked.

Please respond to either say you're no longer blocking this or say why you continue to block it.

There is no reason not to fix this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80961