llvm / llvm-project

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

clang-query: matchesName AST matcher insists on "::" prefix #47223

Open f06211aa-4eb1-40d0-bf34-1204f85843f3 opened 3 years ago

f06211aa-4eb1-40d0-bf34-1204f85843f3 commented 3 years ago
Bugzilla Link 47879
Version unspecified
OS All
CC @AaronBallman

Extended Description

$ echo "void foo(void) { }" > foo.c

$ clang-query foo.c -c 'match functionDecl(matchesName("^foo"))' 0 matches.

$ clang-query foo.c -c 'match functionDecl(matchesName("^::foo"))'

Match #​1:

/tmp/foo.c:1:1: note: "root" binds here void foo(void) { } ^~~~~~ 1 match.

This is clear from the code: https://github.com/llvm/llvm-project/blob/32d565b4618d31511e79dbe77aae8d456f2bf466/clang/include/clang/ASTMatchers/ASTMatchers.h#L2790

But it's at least unexpected, in particular for C code.

The matcher seems to be correctly used in a few places within the LLVM codebase. If this is the intended behavior, then the documentation is wrong.

AaronBallman commented 3 years ago

But it's at least unexpected, in particular for C code.

I agree that this is especially strange in C, where it's already hard enough to remember that '::Foo' is supported.

The matcher seems to be correctly used in a few places within the LLVM codebase. If this is the intended behavior, then the documentation is wrong.

This has been the behavior of the matcher since it was introduced 8 years ago: https://github.com/llvm/llvm-project/commit/1dad183b38efedfb5a0248842caaa088268106c7#diff-fbafa44b28989eafe5795802cfefbad77b8cb6f6557a75ae2a8592afdb449147R794

I don't think this behavior was ever intended for C, but even for C++ it's a bit questionable given that "::foo" is a valid fully-qualified name that means "foo at the root namespace" (I would have expected the user would have to write "[A-Za-z0-9_]*::foo" in order to get the default behavior the matcher gives you). However, I'm not certain we want to invalidate 8 years' worth of user inertia behind the matcher and perhaps the most reasonable approach is to change the documentation to make these points more clear.

Manuel, what are your thoughts?