[PATCH] D102213: [ASTMatchers] Add forCallable(), a generalization of forFunction().

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

In D102213#2750050 <https://reviews.llvm.org/D102213#2750050>, @steveire wrote:

> I'm not certain what strong opinions I've voiced on the mailing list about that. Was it just that I think `hasDescendant` can lead to unexpected matches and a more-specific matcher should always be used when available (such as `hasArgument`)?

Yeah probably :D So, anyway, you sound like the right person to ask: do you have any immediate thoughts on a variant of `hasDescendant` that doesn't lead to unexpected matches because it stops exploring insides of nested declarations? Would that be useful in your endeavors?

> I do think `has`, `hasDescendant`, `hasAncestor` etc have their uses and they get more useful with utilities like `forFunction`.
>
> I don't know anything about objc. Is it possible to have callable decls nested within each other, like in c++? Is it common? `forFunction` is mostly useful for the very common case of lambdas within functions.

ObjC class/interface declarations or method implementations cannot be nested into functions. Blocks, however, are like lambdas, they can appear anywhere (in fact, blocks and lambdas are implicitly convertible into each other). Additionally, in Objective-C++ you can have lambdas nested anywhere including methods or blocks and you can still have nested C++ classes everywhere.

> Does it make sense to add an overload for `LambdaExpr` so that you can write `forCallable(lambdaExpr())`?

Dunno, maybe but I've never encountered such use-cases. If at all, the same problem applies to `forFunction()` as well.

> Is there a missing test for `forFunction(functionDecl())`?

Uhm yeah, I should add them. There's some code duplication so it's worth it to at least duplicate the tests.



================
Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:7597
+///   but does not match 'int y = 2'.
+AST_MATCHER_P(Stmt, forCallable, internal::Matcher<Decl>, InnerMatcher) {
+  const auto &Parents = Finder->getASTContext().getParents(Node);
----------------
aaron.ballman wrote:
> You should also update Registry.cpp to expose this via the dynamic matchers.
Uh-oh, i knew i was forgetting something!

Do we have a checklist? (Do we want to?)


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D102213/new/

https://reviews.llvm.org/D102213