[PATCH] D90984: Update matchers to be traverse-aware

[Stephen Kelly via Phabricator via cfe-commits]

steveire added a comment.

I added the release note to D90982 <https://reviews.llvm.org/D90982>, which this change follows.



================
Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:3115
+
+  if (Finder->getASTContext().getParentMapContext().getTraversalKind() !=
+          TK_AsIs &&
----------------
aaron.ballman wrote:
> Given how often this gets repeated (and what a mouthful it is), how about adding a static helper function `ClassifyTraversal()` (or some such) that returns the traversal kind given a `Finder`? (Alternatively, `isTraversalAsIs()` or some specific traversal behavior.)
Done in D91144.


================
Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:5466
+            TK_AsIs &&
+        FD->isDefaulted())
+      return false;
----------------
aaron.ballman wrote:
> In general (not specific to this matcher), I'd like to see some tests for explicitly defaulted functions vs implicitly defaulted ones. e.g., I think we should handle these differently, shouldn't we?
> ```
> struct S {}; // Has implicitly defaulted constructor
> struct T {
>   T() = default; // Has explicitly defaulted constructor
> };
> ```
> Specific to this matcher, I am a bit concerned about the behavior of answering "isDefinition" based on whether a function is defaulted or not. The user has already gotten some `FunctionDecl` object, so why would the implicit vs explicit nature of the declaration matter for deciding whether the node is a definition? The behavior with `hasBody` makes sense to me because the body may be implicitly provided, but whether something is or is not a definition is a different kind of question.
> 
I agree for both `isDefinition` and `isInline`. `hasBody` is the one to use for these cases.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90984