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

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D102213#2755064 <https://reviews.llvm.org/D102213#2755064>, @NoQ wrote:

> I'm also mildly worried that Function is not the technically correct term. Maybe we should mark the old matcher as deprecated instead?

Thank you for the explanations as to why there's a separate matcher. I think using a separate matcher is the safer approach to introduce the functionality. I'd be fine if we marked the old matcher as deprecated in the documentation for it. (I don't believe that we've added any louder deprecation markings yet aside from comments.)



================
Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:7601
+  llvm::SmallVector<DynTypedNode, 8> Stack(Parents.begin(), Parents.end());
+  while(!Stack.empty()) {
+    const auto &CurNode = Stack.back();
----------------
Might as well fix this clang-format warning.


================
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);
----------------
NoQ wrote:
> 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?)
I don't think we have a checklist, but the mental checklist I use is:

* If there's a change to doc comments in ASTMatchers.h, did the HTML file get regenerated?
* If there's a new matcher added, did Registry.cpp get updated for it?
* If there are changes to the list in Registry.cpp, is the list still alphabetical?
* Testcases?


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

https://reviews.llvm.org/D102213