[PATCH] D69218: [ASTMatchers] Add `cxxBaseSpecifier` matcher (non-top-level)

[Stephen Kelly via Phabricator via cfe-commits]

steveire added a comment.

In D69218#2654638 <https://reviews.llvm.org/D69218#2654638>, @nick wrote:

> In D69218#2654614 <https://reviews.llvm.org/D69218#2654614>, @steveire wrote:
>
>> @nick Sorry that getting these changes merged takes so long.
>>
>> @njames93 If you have an alternative way forward, please let us know what it is.
>>
>> Otherwise, this LGTM too and we should merge it soon unless there are objections which haven't been addressed.
>
> I had a PR in Boost that took 4 years to merge, so it is nothing new to me :D

Yes, I'm a few years trying to get https://steveire.wordpress.com/2019/04/30/the-future-of-ast-matching-refactoring-tools-eurollvm-and-accu/ merged, but I've made some progress :D.

> Rebased, even though there was no conflicts.

I think `arc patch` might be a bit fickle, so it wasn't applying cleanly for me before.

This LGTM and there have been no further objections, and the objection from @njames93 seems to be addressed.

What name/email should I use for the commit?



================
Comment at: clang/unittests/ASTMatchers/Dynamic/RegistryTest.cpp:301
+TEST_F(RegistryTest, CXXBaseSpecifier) {
+  // TODO: rewrite with top-level cxxBaseSpecifier matcher when available
+  DeclarationMatcher ClassHasAnyDirectBase =
----------------
nick wrote:
> steveire wrote:
> > @nick Is this implemented in another MR? I don't see anything in your list of revisions. I think this is reasonable as is, but wondering if you intend to implement the top-level support too.
> The patch had some of the top-level matcher parts, but it cut them off when rebased. I have no need in it myself and I am not sure there is any kind of use for it.
If you look at the overloads of `MatchFinder::addMatcher` and compare to, say, the `using` declarations near the top of `ASTMatchers.h` or the types supported in `ASTNodeKind` or `DynTypedNode::BaseConverter`, this one is certainly "missing" as a supported top level node in `MatchFinder::addMatcher` (though it's not the only one in any case). 

That said I don't think merging this MR should depend on that feature.

Hopefully someone will submit it for review. I have the same difficulty with getting changes reviewed, but I can review them and get them towards approval and merge more easily than I can find a reviewer :).


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

https://reviews.llvm.org/D69218