llvm / llvm-project

The LLVM Project is a collection of modular and reusable compiler and toolchain technologies.
http://llvm.org
Other
29.3k stars 12.11k forks source link

[AST-Matchers] Inconsistency between the dynamic AST matchers used via clang-query and the C++ AST matchers used via clang-tidy #106656

Open MichelleCDjunaidi opened 2 months ago

MichelleCDjunaidi commented 2 months ago

As per https://clang.llvm.org/extra/clang-tidy/Contributing.html, clang-query is used for interactive prototyping of AST matchers that will eventually be used via clang-tidy. However, there are differences between two matchers such that a matcher that works in clang-query will not work as intended or doesn't compile in clang-tidy.

See the following discussions for examples/code-samples of this issue cropping up: https://discourse.llvm.org/t/matcher-working-in-clang-query-but-not-in-libtooling/56947 (clang-query matcher works, clang (and clang-tidy) doesn't compile) https://discourse.llvm.org/t/inconsistency-between-hasdescendant-in-clang-query-and-clang-libtooling-matchers/80799 (matcher compiles but doesn't work as intended)

Code samples (extracted from the first discussion, tested to be present at least up to clang 17.0.6):

//clang-query, will work
match cxxRecordDecl(has(unless(cxxRecordDecl())))
//will not compile:
#include “clang/ASTMatchers/ASTMatchers.h”

using namespace clang::ast_matchers;

int main(int argc, char** argv){
auto m = cxxRecordDecl(has(unless(cxxRecordDecl())));
return 0;

}

//this is the outcome:
../src/clang/Example.cpp:6:24: error: no matching function for call to object of type 'const internal::ArgumentAdaptingMatcherFunc<clang::ast_matchers::internal::HasMatcher>'
    6 | auto m = cxxRecordDecl(has(unless(cxxRecordDecl())));
      |                        ^~~
/usr/include/clang/ASTMatchers/ASTMatchersInternal.h:1491:3: note: candidate template ignored: could not match 'Matcher' against 'VariadicOperatorMatcher'
 1491 |   operator()(const Matcher<T> &InnerMatcher) const {
      |   ^
/usr/include/clang/ASTMatchers/ASTMatchersInternal.h:1498:3: note: candidate template ignored: could not match 'MapAnyOfHelper' against 'VariadicOperatorMatcher'
 1498 |   operator()(const MapAnyOfHelper<T...> &InnerMatcher) const {
      |   ^
1 error generated.

The way to make the same behaviour as clang-query's matcher is to add a has in the middle:

#include “clang/ASTMatchers/ASTMatchers.h”

using namespace clang::ast_matchers;

int main(int argc, char** argv){
auto m = cxxRecordDecl(has(decl(unless(cxxRecordDecl()))));
return 0;

}

Of course, this probably isn't ideal.

As it stands the quickest way to make people be at least aware of this difference is to edit the Contributing guide.

Ideally the differences can be resolved or reduced by refining the dynamic matchers (https://github.com/llvm/llvm-project/blob/main/clang/lib/ASTMatchers/Dynamic/Marshallers.cpp, https://github.com/llvm/llvm-project/blob/main/clang/lib/ASTMatchers/Dynamic/Parser.cpp), and the AST matchers (https://github.com/llvm/llvm-project/blob/main/clang/lib/ASTMatchers/ASTMatchersInternal.cpp, https://github.com/llvm/llvm-project/blob/main/clang/include/clang/ASTMatchers/ASTMatchersInternal.h).

MichelleCDjunaidi commented 2 months ago

I'm working on a documentation update regarding contributing to clang-tidy (went through the process very recently and encountered some issues with the written guide), so I can handle the documentation update side, but I don't think I have the knowledge base to deal with the matchers yet.