[PATCH] D113943: Add `withIntrospection` matcher.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D113943#3184076 <https://reviews.llvm.org/D113943#3184076>, @avogelsgesang wrote:

> Nit:
> Personally, I would prefer the name `withDebugOutput` over `withIntrospection`, as it more clearly describes the intent of this matcher.
> At least to me, "introspection" is usually something done programatically, i.e. the matcher somehow reflects about its own state, but in this case all we are doing is printing some debug output to stderr.

FWIW, I tend to agree that "introspection" is a bit of an odd choice for naming here. `withDebugOutput` would be an improvement, but `withPrefixAndSuffix` or something along those lines would also work.



================
Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:3000-3001
+                  const internal::Matcher<T> &InnerMatcher) {
+  return internal::Matcher<T>(new internal::WithIntrospectionMatcher<T>(
+      BeforeTag, AfterTag, InnerMatcher));
+}
----------------
avogelsgesang wrote:
> std::move(BeforeTag), std::move(AfterTag)
> 
> No need to create additional copies of those strings. Here and other places
Alternatively, it'd be great if we can switch these to use `StringRef` to avoid copies.


================
Comment at: clang/include/clang/ASTMatchers/ASTMatchersInternal.h:1064-1066
+  explicit WithIntrospectionMatcher(std::string _BeforeTag,
+                                    std::string _AfterTag,
+                                    internal::Matcher<T> _InnerMatcher)
----------------
All of those identifiers introduce UB because they're reserved, dropping the underscore solves the issue (and, oddly, matches our terrible coding style where we let ctor parameters shadow the names of member variables).


================
Comment at: clang/include/clang/ASTMatchers/ASTMatchersInternal.h:1784
+      internal::PolymorphicMatcher<MatcherT, ReturnTypesF, ParamTypes...>
+          _InnerMatcher)
+      : InnerMatcher(_InnerMatcher) {}
----------------



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113943