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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
