[PATCH] D87527: [ASTMatchers] Fix `hasBody` for the descendants of `FunctionDecl`

[Gabor Marton via Phabricator via cfe-commits]

martong accepted this revision.
martong added a comment.
This revision is now accepted and ready to land.

> Unfortunately template specializations do not catch the descendants of the class for which the template is specialized. Therefore it does not work correcly for the descendants of FunctionDecl, such as CXXMethodDecl, CXXConstructorDecl, CXXDestructorDecl etc. This patch fixes this issue by using a template metaprogram.

Yes, implicit type conversions are not considered during template argument deduction.

First I was thinking about why don't we just simply specialize for all subclasses of FunctionDecl (CXXMethodDecl, CXXConstructorDecl, CXXDestructorDecl, etc) ?
But then I realized, your solution with enable_if is more generic and will work even if a new subclass is added. Looks good to me.



================
Comment at: clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp:1468
+
+TEST(HasBody, FindsBodyOfFunctionChildren) {
+  EXPECT_TRUE(matchAndVerifyResultTrue(
----------------
FindsBodyOfFunctionDeclSubclasses ?


================
Comment at: clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp:1471
+      "class C { void f(); }; void C::f() {}",
+      cxxMethodDecl(hasBody(compoundStmt())).bind("met"),
+      std::make_unique<VerifyIdIsBoundTo<CXXMethodDecl>>("met", 1)));
----------------
"met" -> "method" ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87527