njames93 added inline comments.

================
Comment at: clang/lib/ASTMatchers/ASTMatchFinder.cpp:789
+          else
+            OS << "<anonymous> ";
+          D->getSourceRange().print(OS,
----------------
aaron.ballman wrote:
> Should this be `"<anonymous> : "` instead?
Good catch


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


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

Reply via email to