[cfe-dev] [clang-tidy][Sema][AST] Different behavior Sema and AST matcher

[David Blaikie via cfe-dev]

On Sun, Oct 10, 2021 at 6:46 AM Carlos Galvez via cfe-dev <
cfe-dev at lists.llvm.org> wrote:

> Hi!
>
> As proposed by @whisperity I'm posting here to ask about a strange
> situation we found in the following bug fix:
>
> https://reviews.llvm.org/D110614
>
> We had a matcher to detect if a class has a public virtual destructor:
>
> has(cxxDestructorDecl(isPublic(), isVirtual())),
>
> However this didn't work for class templates, see details:
>
> https://reviews.llvm.org/D110614#3032760
>
> The workaround was to create a custom matcher that seems to do the same
> thing as the original matcher, but it actually works for templates because
> it calls cxxDestructorDecl::isVirtual() (from Sema), which correctly
> propagates the virtual specifier from the base class.
>

What does the original (buggy) implementation use instead of
cxxDestructorDecl::isVirtual? If the original's just using the wrong query
for testing virtuality that seems like a reasonable thing to fix as a bug?


>
> Is this a concern, and should someone look into Sema/AST? (I'm
> unfortunately a n00b here). Or should we just merge the clang-tidy fix and
> move on?
>
> Thanks!
> _______________________________________________
> cfe-dev mailing list
> cfe-dev at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20211025/39195d58/attachment.html>