[PATCH] D113917: Add infrastructure to support matcher names.

[James King via Phabricator via cfe-commits]

jcking1034 added inline comments.


================
Comment at: clang/include/clang/ASTMatchers/ASTMatchersInternal.h:1021
 
+  std::string getName() const override {
+    return "HasOverloadedOperatorNameMatcher";
----------------
Here and below, we have the option to use the name of the matcher (for this, we could use "hasOverloadedOperatorName" instead). However, some of these classes are used in multiple matchers. For example, `HasOverloadedOperatorNameMatcher` is used in both `hasOverloadedOperatorName` and `hasAnyOverloadedOperatorName`, which I'm a bit concerned about.


================
Comment at: clang/include/clang/ASTMatchers/ASTMatchersInternal.h:1409
   return BindableMatcher<T>(
-      makeAllOfComposite(InnerMatchers).template dynCastTo<T>());
+      makeAllOfComposite(InnerMatchers).template dynCastTo<T>(),
+      makeMatcherNameFromType<InnerT>());
----------------
@ymandel suggested an alternate implementation where we instead pass the matcher name to `makeAllOfComposite`, which then passes the name to `constructVariadic`, to avoid making changes to `BindableMatcher`. I may look into this again, but my previous attempts to try this approach seemed messy due to the fact that these functions are used in a few different places, and because `makeAllOfComposite` handles cases with 0 or 1 inner matchers without constructing a variadic matcher.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D113917/new/

https://reviews.llvm.org/D113917