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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
