[ASTMatchers] Add matchers available through casting to derived

[steveire]

These commits are available on github if it's convenient to see it all together there:

https://github.com/steveire/clang/commits/matcher-output

Here is context for the changes I'm making in case it is useful:

https://steveire.wordpress.com/2018/11/11/future-developments-in-clang-query/

Note that the commits pushed for review so far don't implement everything I wrote about there.

[steveire]

Update

[aaron.ballman]

lib/ASTMatchers/Dynamic/Registry.cpp

654

Don't use auto here (and if you did. the const would go on the other side).

656–660

I think this can be replaced with llvm::find_if().

667

I'd prefer this to not be a default argument (the callers can pass it in). It might also be nice to use an enum here rather than a bool, but alternative to that, you should stick in comments in the callers that describe the parameter name. e.g. getMatchingMatchersImpl(yada, /*ExactOnly*/true);

682

Don't use auto.

687–689

Why do these need their own inner block scopes?

690

auto.

691–692

if (TypeForMatcherOpt && StaticType.isBaseOf(...)) {

693–694

auto, here and everywhere in this file.

734

Can remove whitespace around here.

746–751

It seems like getMatchingMatchersImpl() should just be renamed to getMatchingMatchers().

[steveire]

Some updates

[lebedev.ri]

Differential lacks description.

[aaron.ballman]

lib/ASTMatchers/Dynamic/Registry.cpp

651

targetCtor -> TargetCtor

653

it -> It

654

ctor -> Ctor

657–659

Elide braces

682

Can combine with the previous if statement, I believe.

688

Don't use auto here please.

691

Don't use auto here please.

704

Don't use auto here please.

710

item -> Item

[steveire]

Case

[Abpostelnicu]

@aaron.ballman
I think the auto usage improves and simplifies the code, however, as I would really like to see this code landed, I would be ok to help Stephen to finish this work and replace auto by the "old" way. Do you think we can have this approved if we make the changes mentioned earlier?

[aaron.ballman]

In D54408#1659301, @Abpostelnicu wrote:

@aaron.ballman
I think the auto usage improves and simplifies the code, however, as I would really like to see this code landed, I would be ok to help Stephen to finish this work and replace auto by the "old" way. Do you think we can have this approved if we make the changes mentioned earlier?

Nothing jumps out at me as blocking this, aside from completing the usual review activities. Have at it!

[steveire]

@aaron.ballman I think we agreed in Belfast in November (after the most recent comment) to get this in as it is and not be as draconian about auto. Is anything blocking this?

[aaron.ballman]

In D54408#2056251, @steveire wrote:

@aaron.ballman I think we agreed in Belfast in November (after the most recent comment) to get this in as it is and not be as draconian about auto. Is anything blocking this?

Nothing is blocking this, but I think we may have slightly different understandings of what we agreed to. Reviewers asking for code to follow the coding guidelines is not draconian; it's expected for code to follow the coding guidelines and for patch authors to field requests for changes like these. I re-reviewed the things I was asking for changes on and have added a comment where I definitely insist on changes. LGTM with that nit fixed though.

lib/ASTMatchers/Dynamic/Registry.cpp

704

Please change this use of auto -- I have no idea what the type is for NestedResult without hunting for it and so I have no idea what the type is for Item and whether using a const reference is reasonable or not (after doing the hunting, it looks fine though).

lib/ASTMatchers/Dynamic/Registry.cpp

for (const auto &M : RegistryData->constructors()) {
 
     if (!RetKinds.empty() && MaxSpecificity > 0) {
       std::forward<Callable>(Func)(Name, Matcher, RetKinds, ArgsKinds,
                                    MaxSpecificity);
     }
   }
 }
 
+llvm::Optional<std::pair<ASTNodeKind, std::string>>
+getNodeConstructorType(MatcherCtor TargetCtor) {
+  const auto &Ctors = RegistryData->nodeConstructors();
+  auto It = llvm::find_if(
+      Ctors, [TargetCtor](const NodeConstructorMap::value_type &Ctor) {
+        return Ctor.second.second == TargetCtor;
+      });
+  if (It == Ctors.end())
+    return llvm::None;
+  return std::make_pair(It->first, It->second.first);
+}
+
 std::vector<MatchingMatcher>
-Registry::getMatchingMatchers(ast_type_traits::ASTNodeKind StaticType) {
+getDerivedResults(ast_type_traits::ASTNodeKind StaticType, StringRef Name);
+
+enum MatchScope { ExactOnly, IncludeDerived };
+
+std::vector<MatchingMatcher>
+getMatchingMatchersImpl(ast_type_traits::ASTNodeKind StaticType,
+                        MatchScope Scope) {
+
   std::vector<MatchingMatcher> Result;
 
   std::vector<ArgKind> AcceptedTypes{StaticType};
 
   processAcceptableMatchers(
-      AcceptedTypes,
-      [&Result](StringRef Name, const MatcherDescriptor &,
-                std::set<ASTNodeKind> &, std::vector<std::vector<ArgKind>>,
-                unsigned) { Result.emplace_back((Name + "()").str()); });
+      AcceptedTypes, [&](StringRef Name, const MatcherDescriptor &Matcher,
+                         std::set<ASTNodeKind> &,
+                         std::vector<std::vector<ArgKind>>, unsigned) {
+        unsigned Specificity;
+        ASTNodeKind LeastDerivedKind;
+        if (Scope == ExactOnly &&
+            Matcher.isConvertibleTo(StaticType, &Specificity,
+                                    &LeastDerivedKind) &&
+            !LeastDerivedKind.isSame(StaticType))
+          return;
+
+        auto TypeForMatcherOpt = getNodeConstructorType(&Matcher);
+        if (TypeForMatcherOpt &&
+            StaticType.isBaseOf(TypeForMatcherOpt->first)) {
+          auto Derived = getDerivedResults(TypeForMatcherOpt->first, Name);
+          Result.insert(Result.end(), Derived.begin(), Derived.end());
+          return;
+        }
+
+        Result.emplace_back((Name + "()").str());
+      });
+
+  return Result;
+}
+
+std::vector<MatchingMatcher>
+getDerivedResults(ast_type_traits::ASTNodeKind StaticType, StringRef Name) {
+  auto NestedResult = getMatchingMatchersImpl(StaticType, ExactOnly);
+
+  std::vector<MatchingMatcher> Result;
+
+  Diagnostics DiagnosticIgnorer;
+
+  for (const auto &Item : NestedResult) {
+    Result.emplace_back((Name + "(" + Item.MatcherString + ")").str());
+  }
+
+  return Result;
+}
+
+std::vector<MatchingMatcher>
+Registry::getMatchingMatchers(ast_type_traits::ASTNodeKind StaticType) {
+
+  std::vector<MatchingMatcher> Result =
+      getMatchingMatchersImpl(StaticType, IncludeDerived);
 
   return Result;
 }
 
 std::vector<MatcherCompletion>
 Registry::getMatcherCompletions(ArrayRef<ArgKind> AcceptedTypes) {
   std::vector<MatcherCompletion> Completions;
 
   processAcceptableMatchers(
       AcceptedTypes,
       [&Completions](StringRef Name, const MatcherDescriptor &Matcher,
                      std::set<ASTNodeKind> &RetKinds,
                      std::vector<std::vector<ArgKind>> ArgsKinds,
                      unsigned MaxSpecificity) {
         std::string Decl;
         llvm::raw_string_ostream OS(Decl);
 
         if (Matcher.isPolymorphic()) {
           OS << "Matcher<T> " << Name << "(Matcher<T>";
         } else {
           OS << "Matcher<" << RetKinds << "> " << Name << "(";
           for (const std::vector<ArgKind> &Arg : ArgsKinds) {
             if (&Arg != &ArgsKinds[0])
               OS << ", ";
 
             bool FirstArgKind = true;
             std::set<ASTNodeKind> MatcherKinds;
             // Two steps. First all non-matchers, then matchers only.
             for (const ArgKind &AK : Arg) {
               if (AK.getArgKind() == ArgKind::AK_Matcher) {
                 MatcherKinds.insert(AK.getMatcherKind());
               } else {
                 if (!FirstArgKind)
                   OS << "|";
                 FirstArgKind = false;
                 OS << AK.asString();
               }
             }
             if (!MatcherKinds.empty()) {

unittests/ASTMatchers/Dynamic/RegistryTest.cpp

TEST_F(RegistryTest, Completion) {
   EXPECT_TRUE(hasCompletion(Comps, "whileStmt(",
                             "Matcher<Stmt> whileStmt(Matcher<WhileStmt>...)"));
   // Polymorphic.
   EXPECT_TRUE(hasCompletion(
       Comps, "hasDescendant(",
       "Matcher<NestedNameSpecifierLoc|QualType|TypeLoc|...> "
       "hasDescendant(Matcher<NestedNameSpecifierLoc|QualType|TypeLoc|...>)"));
 
+  CompVector FuncDeclComps = getCompletions("functionDecl", 0);
+
+  EXPECT_TRUE(!FuncDeclComps.empty());
+
+  // Exclude bases
+  EXPECT_TRUE(!hasCompletion(FuncDeclComps, "namedDecl("));
+  EXPECT_TRUE(!hasCompletion(FuncDeclComps, "valueDecl("));
+  EXPECT_TRUE(!hasCompletion(FuncDeclComps, "declaratorDecl("));
+  EXPECT_TRUE(!hasCompletion(FuncDeclComps, "functionDecl("));
+
   CompVector WhileComps = getCompletions("whileStmt", 0);
 
   EXPECT_TRUE(hasCompletion(WhileComps, "hasBody(",
                             "Matcher<WhileStmt> hasBody(Matcher<Stmt>)"));
   EXPECT_TRUE(hasCompletion(
       WhileComps, "hasParent(",
       "Matcher<Stmt> "
       "hasParent(Matcher<NestedNameSpecifierLoc|TypeLoc|Decl|...>)"));
auto Contains = [](const std::vector<MatchingMatcher> &C, StringRef Name) {
     return llvm::find_if(C, [Name](const MatchingMatcher &M) {
              return M.MatcherString == Name;
            }) != C.end();
   };
 
   EXPECT_TRUE(Contains(Matchers, "isPublic()"));
   EXPECT_TRUE(Contains(Matchers, "hasName()"));
   EXPECT_TRUE(Contains(Matchers, "parameterCountIs()"));
+  EXPECT_TRUE(Contains(Matchers, "cxxMethodDecl(isOverride())"));
 
   EXPECT_FALSE(Contains(Matchers, "namedDecl()"));
   EXPECT_FALSE(Contains(Matchers, "valueDecl()"));
   EXPECT_FALSE(Contains(Matchers, "declaratorDecl()"));
   EXPECT_FALSE(Contains(Matchers, "functionDecl()"));
-
-  EXPECT_TRUE(Contains(Matchers, "cxxMethodDecl()"));
+  EXPECT_FALSE(Contains(Matchers, "cxxMethodDecl()"));
 }
 
 } // end anonymous namespace
 } // end namespace dynamic
 } // end namespace ast_matchers
 } // end namespace clang