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