LegalizeAdulthood added a comment. In D116328#3223299 <https://reviews.llvm.org/D116328#3223299>, @aaron.ballman wrote:
> In D116328#3223268 <https://reviews.llvm.org/D116328#3223268>, > @LegalizeAdulthood wrote: > >> In D116328#3221995 <https://reviews.llvm.org/D116328#3221995>, >> @aaron.ballman wrote: >> >>> > > `has()` matches the direct descendant AST node, whereas `hasDescendant()` > matches *any* descendant AST node. > [...] I don't know of any performance concerns with `has()` though. Thanks for the explanation! >> Honestly, at this point, I become lost in manually tracing the code through >> "go to definition" in my IDE. > > I'm impressed you got this far -- the AST matching machinery under the hood > is really quite complex! :-) Having CMake generate a VS project and drilling in with ReSharper for C++ is pretty powerful :), but you have to manually keep track of how the template and function arguments are being resolved in the dynamic execution. So it's pretty easy to get lost in template oriented code, which is understandably pretty common in AST land. > My understanding (and my recollection here may be wrong) is that `has()` can > memoize the results > [...] but you can't get the same behavior from `hasDescendant()` or > `hasAncestor()` OK, that makes sense. > I'm adding @sammccall in case he has information about the performance > characteristics or thoughts about > the proposed matcher in general. So let me summarize what we've learned so far: - `has` only descends one level, is memoizable, and is less expensive than `hasDescendant` - `has` uses the Visitor pattern to compute the result that is memoized - @sammccall might be aware of any performance related issues with `has` My takeaway: - if `has` isn't expensive, I can either ditch this public matcher or move it to be a private matcher used in my check CHANGES SINCE LAST ACTION https://reviews.llvm.org/D116328/new/ https://reviews.llvm.org/D116328 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
