[PATCH] D87527: [ASTMatchers] Fix `hasBody` for the descendants of `FunctionDecl`

Mon Sep 14 06:30:28 PDT 2020

baloghadamsoftware added a comment.

> Thank you for clarifying. I might nitpick that it's not "obviously wrong": I'm actually writing code now that depends on exactly that behavior -- I don't care where the method decl body is, as long as it's visible in the current TU.  That said, I think you're right in wanting `FunctionDecl`'s behavior to align with that of its derived classes and I think it better to behave that way than how it currently behaves.  But, we should have an alternative available in case this change breaks anyone else depending on the current behavior.  Is there a different matcher with the semantics I've described?
>
> Also, please clarify the comments in the header for this matcher. The current description is quite unclear for functions: "Matches a 'for', 'while', 'do while' statement or a function
> definition that has a given body."

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.

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.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87527