[AST matchers] making findAll work with VariantMatcher interface

Hello, I need the findAll matcher to work with the dynamic/VariantMatcher interface for Reasons, but I’m running into problems writing the marshaller (I don’t fully understand the marshallers). I’ve gotten it to the point where clang-query no longer says it doesn’t recognize findAll, but it gives me this error now:

4:24: Error parsing matcher. Found token <(> while looking for ','.

I’m assuming I’m doing something wrong with getArgKinds() or getNumArgs() in the marshaller/MatcherDescriptor.

Here’s what I’ve added to Marshallers.h:

class FindAllBuilderDescriptor : public MatcherDescriptor {
public:
  VariantMatcher create(SourceRange, ArrayRef<ParserValue>,
                        Diagnostics *) const override {
    return {};
  }

  bool isBuilderMatcher() const override { return true; }

  bool isVariadic() const override { return false; }

  unsigned getNumArgs() const override { return 1; }

  void getArgKinds(ASTNodeKind ThisKind, unsigned,
                   std::vector<ArgKind> &ArgKinds) const override {
    ArgKinds.push_back(ArgKind::MakeMatcherArg(ThisKind));
  }

  bool isConvertibleTo(ASTNodeKind Kind, unsigned *Specificity = nullptr,
                       ASTNodeKind *LeastDerivedKind = nullptr) const override {
    if (Specificity)
      *Specificity = 1;
    if (LeastDerivedKind)
      *LeastDerivedKind = Kind;
    return true;
  }

  bool isPolymorphic() const override { return true; }
};

And here’s what I put in RegistryMaps::RegistryMaps() in Registry.cpp:

registerMatcher("findAll",
                  std::make_unique<internal::FindAllBuilderDescriptor>());

I’ve found a workaround by using the definition of findAll(matcher) = eachOf(matcher, forEachDescendant(matcher)) but would still like to get this working. It seems the error is not coming from my code but from the matcher parser that clang-query uses (which I understand even less than marshallers). If anyone could provide some guidance on debugging why the parser seems to be expecting a comma instead of a matcher, I’d very much appreciate it.

Yeah, that diagnostic comes from the dynamic matcher parser: either at llvm-project/Parser.cpp at main · llvm/llvm-project · GitHub or at llvm-project/Parser.cpp at main · llvm/llvm-project · GitHub

What I’m confused by (and I’ve not hooked up a debugger to see what’s going on) is why you’d expect a comma in the first place. I suspect the problem is that we’ve found something when parsing the findAll token, but we’re not realizing that’s the name of a matcher and thus not trying to read the ( token, but instead deciding findAll is part of an argument list for some reason.

I think you’ll have to dig into it with a debugger to get more of an idea as to what’s gone wrong. I think your implementation of getNumArgs() and getArgKinds() looks like what I’d expect, but I do wonder if your call to registerMatcher() is correct. findAll() has argument overloading and IIRC, those are special.

CC @sam-mccall, @pcc, and @r4nt in case any of them have more of these details paged into memory than I do.