new AST matcher that checks the actual number of arguments given to a function call, *excluding* defaults not present; works like argumentCountIs(n) when isTraversalIgnoringImplicitNodes() is true
Details
Details
Diff Detail
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
Comment Actions
This seems somewhat more generally applicable as an AST matcher, but I'm still curious what the in-tree use will be for it. (Same suggestion for test coverage and regenerating the documentation as before.)
| clang/include/clang/ASTMatchers/ASTMatchers.h | ||
|---|---|---|
| 4432 | I have a preference for argumentCountAsWrittenIs() -- we often use "as written" to distinguish syntax from semantics, and in the case of default arguments, the semantics are that the default arguments are treated as if they were given at the call site, so I would expect argumentsGivenCountIs(2) to match both calls to f(). | |
Comment Actions
This is the same use case as my other patches :) https://github.com/ajohnson-uoregon/llvm-project/blob/feature-ajohnson/clang-tools-extra/clang-rewrite/ConstructMatchers.cpp#L545
If there's a function with default arguments, and our user doesn't specify some of those defaults when they're writing code for our tool, we want to match only the calls in their code that give the same number of arguments, but argumentCountIs() matches the total *including* default arguments, and we need the total *excluding* non-provided default arguments. For example, our user might write
void func(int a = 0, int b = 0);
[[clang::matcher("test")]]
auto foo(int x) {
func(x);
}and we don't want that to match func() or func(x, y). When we're doing matcher generation, we aren't able to look up the FunctionDecl and see if there are defaults (because the function name itself might be a parameter for us), and even if we could, adding cxxDefaultArgExpr() for all of the missing ones is a lot of work that we'd rather not do.
| clang/include/clang/ASTMatchers/ASTMatchers.h | ||
|---|---|---|
| 4432 | Ah that's a good point, and I like that name much better, I will update that. | |
Comment Actions
Thanks, this is pretty compelling rationale for why this is a generally useful matcher.
Be sure to also add test coverage for the matcher, as well as regenerate the documentation from docs/tools/dump_ast_matchers.py.
| clang/include/clang/ASTMatchers/ASTMatchers.h | ||
|---|---|---|
| 4432 | I'm not 100% sold on the new name being argumentsGivenCountIs -- I get where it's coming from, but it feels less grammatical than argumentCountAsWrittenIs(). Also, we have an AsWritten matcher already (isVirtualAsWritten()), but none with Given. | |
I have a preference for argumentCountAsWrittenIs() -- we often use "as written" to distinguish syntax from semantics, and in the case of default arguments, the semantics are that the default arguments are treated as if they were given at the call site, so I would expect argumentsGivenCountIs(2) to match both calls to f().