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