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



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to