[PATCH] D126891: [clang-tidy] The check should ignore final classes

This revision was automatically updated to reflect the committed changes. steakhal marked an inline comment as done. Closed by commit rGd9afb8c3e8fd: [clang-tidy] cppcoreguidelines-virtual-class-destructor should ignore final… (authored by steakhal). Changed prior to commit: https://reviews.ll

[PATCH] D126891: [clang-tidy] The check should ignore final classes

whisperity added a comment. (Please ensure a more appropriate commit message that actually mentions the check when committing, @steakhal.) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126891/new/ https://reviews.llvm.org/D126891

[PATCH] D126891: [clang-tidy] The check should ignore final classes

whisperity accepted this revision. whisperity added a comment. This revision is now accepted and ready to land. Thank you! Comment at: clang-tools-extra/docs/ReleaseNotes.rst:170 + ` involving + ``final`` classes. The check will not diagnose ``final`` marked classes, since +

[PATCH] D126891: [clang-tidy] The check should ignore final classes

steakhal added a comment. @aaron.ballman @njames93 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126891/new/ https://reviews.llvm.org/D126891 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https

[PATCH] D126891: [clang-tidy] The check should ignore final classes

steakhal added a comment. ping again; @whisperity Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126891/new/ https://reviews.llvm.org/D126891 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https:

[PATCH] D126891: [clang-tidy] The check should ignore final classes

steakhal updated this revision to Diff 435054. steakhal marked an inline comment as done. steakhal added a comment. - rebase - use double-backticks for the keyword `final` in the Release Notes. polite ping Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.o

[PATCH] D126891: [clang-tidy] The check should ignore final classes

steakhal added inline comments. Comment at: clang-tools-extra/docs/ReleaseNotes.rst:170 + ` involving + `final` classes. The check will not diagnose `final` marked classes, since + those cannot be used as base classes, consequently they can not violate the car

[PATCH] D126891: [clang-tidy] The check should ignore final classes

carlosgalvezp added inline comments. Comment at: clang-tools-extra/docs/ReleaseNotes.rst:170 + ` involving + `final` classes. The check will not diagnose `final` marked classes, since + those cannot be used as base classes, consequently they can not violate the ---

[PATCH] D126891: [clang-tidy] The check should ignore final classes

carlosgalvezp added a comment. Oh, I see the unit test now, indeed `Base` does not have a virtual destructor. LGTM then! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126891/new/ https://reviews.llvm.org/D126891 __

[PATCH] D126891: [clang-tidy] The check should ignore final classes

carlosgalvezp added a comment. In D126891#3555456 , @steakhal wrote: > In D126891#3554039 , @carlosgalvezp > wrote: > >> Hmm, `MostDerived` **does** have a public virtual destructor in your example >> already -

[PATCH] D126891: [clang-tidy] The check should ignore final classes

steakhal added a comment. In D126891#3554039 , @carlosgalvezp wrote: > Hmm, `MostDerived` **does** have a public virtual destructor in your example > already - if the base class destructor is virtual, the child class destructor > is virtual. In that se

[PATCH] D126891: [clang-tidy] The check should ignore final classes

carlosgalvezp added a comment. Hmm, `MostDerived` **does** have a public virtual destructor in your example already - if the base class destructor is virtual, the child class destructor is virtual. In that sense the check should not warn. Seems like there's some deeper problem in the check? R

[PATCH] D126891: [clang-tidy] The check should ignore final classes

steakhal created this revision. steakhal added reviewers: aaron.ballman, njames93, LegalizeAdulthood, whisperity. Herald added subscribers: carlosgalvezp, martong, rnkovacs, kbarton, xazax.hun, nemanjai. Herald added a project: All. steakhal requested review of this revision. Herald added a proje