[PATCH] D87527: [ASTMatchers] Fix `hasBody` for the descendants of `FunctionDecl`
Yitzhak Mandelbaum via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Mon Sep 14 07:47:05 PDT 2020
ymandel added a comment.
In D87527#2271059 <https://reviews.llvm.org/D87527#2271059>, @baloghadamsoftware wrote:
> We must decide about the namings. If we want to be in sync with the methods in `FunctionDecl`, then we keep `hasBody()` as is, but remove the template specialization, and create a new `doesThisDeclarationHaveABody()`. However, this involves changing existing checks. The other solution is to fix `hasBody()` like in this patch, and find a new name for the other behavior, such as e.g. `hasBodySomewhere()` or something similar.
Agreed. I think we should stick with your approach, since it involves fewer changes and is consistent with the AST API itself. As for naming, `hasBodySomewhere()` works, but other options: `hasSomeBody`, `hasAnyBody`, `hasBodyForSomeDecl`.
FWIW, I checked our codebase and we definitely have a handful of uses that rely on current behavior. So, I think its important to add the new matcher before we commit this change.
> The comments in the header are really poorly written, but there is the word //definition// which means that the matcher matches function definitions and not declarations. This is surely to be rephrased.
+1
Thanks!
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D87527/new/
https://reviews.llvm.org/D87527
More information about the cfe-commits
mailing list