njames93 added inline comments.
================
Comment at: clang/lib/ASTMatchers/ASTMatchFinder.cpp:796
+ MV.ActiveASTContext->getSourceManager());
+ } else if (const auto *T = Item.second.get<Type>()) {
+ OS << T->getTypeClassName() << "Type : ";
----------------
aaron.ballman wrote:
> njames93 wrote:
> > aaron.ballman wrote:
> > > Do we also need a match for `TypeLoc` matchers, or does the `else` cover
> > > that sufficiently well?
> > >
> > > (Actually, should we handle all of the various matchers at:
> > > https://github.com/llvm/llvm-project/blob/main/clang/include/clang/ASTMatchers/ASTMatchers.h#L141
> > > rather than leaving it to an `else`? Then the `else` can become an
> > > unreachable so we know to update this interface?)
> > The else should be sufficient for most general cases, the only reason some
> > are special cased is to improve the output, but I don't want there to be a
> > burden to update this interface if new nodes are added.
> I should verify: does this map hold arbitrary AST nodes, or does the map only
> hold the top-level classes in the AST matching hierarchy?
>
> If it's arbitrary AST nodes, then yeah, I definitely agree the `else` here is
> fine. If it's top-level classes instead, we don't add those all that often
> and so it doesn't seem like a major burden to keep those in sync.
It can hold any kind of node that can be stored in a DynTypedNode, so
essentially it's any arbitrary AST node.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D120185/new/
https://reviews.llvm.org/D120185
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
