steveire added a comment.

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.


Hmm, `IgnoreUnlessSpelledInSource` is designed and named to fill that role. 
After this patch is in, I intend to make it also control whether the implicit 
node here is dumped/matchable: https://godbolt.org/z/V_KCFm

> 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 don't think that's true. You have to change the matchers you've written which 
deliberately match the instantiation, but you can also (optionally) simplify 
the others to remove `unless(isInTemplateInstantiation())` from them. The third 
category of matchers are the ones where you haven't used 
`unless(isInTemplateInstantiation())` even though you should have and you have 
a bug that you don't know about yet. This change fixes those ones.

> 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.

I feel strongly that the default should not change code in a known-wrong way, 
as the unit test demonstrates. It's not a novice-friendly default.


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