ymandel added a comment. In D87527#2271016 <https://reviews.llvm.org/D87527#2271016>, @baloghadamsoftware wrote:
> In D87527#2270939 <https://reviews.llvm.org/D87527#2270939>, @ymandel wrote: > >> Can you expand on what is wrong currently for `FunctionDecl` descendants? >> Would the new test `FindsBodyOfFunctionChildren` fail with the current >> implementation? > > Yes, exactly. They return 2 findings instead of 1 because the matcher matches > both the forward declaration and the definition. The cause for this is that > template specializations do not work for the descendants of the class for > which is template specialized. Instead, the generic version is used. Thus for > the children of `FunctionDecl` the original `GetBodyMatcher` is instantiated > which does not check whether `doesThisDeclarationHaveABody()` but returns a > body instead which may be bound to another declaration of the same function > in the declaration chain. This is obviously wrong. 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." 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
