[PATCH] D31780: Add a virtual destructor to a class with virtual methods.

David Blaikie via llvm-commits llvm-commits at lists.llvm.org
Mon Apr 10 10:23:24 PDT 2017


On Thu, Apr 6, 2017 at 11:42 AM Ivan Krasin via Phabricator via
llvm-commits <llvm-commits at lists.llvm.org> wrote:

> krasin created this revision.
> Herald added a subscriber: kubamracek.
>
> Recently, Clang enabled the check for virtual destructors
> in the presence of virtual methods.


Which check exactly? A warning?

If the type is never actually destroyed via a base pointer - I'd suggest
not adding a virtual dtor unnecessarily.

Instead mark the dtor protected in the base class and mark derived classes
as 'final'. (that way they'll have public non-virtual dtors accessible from
the derived objects/pointers - and no chance of accidental polymorphic
destruction)


> That broke the bootstrap
> build. Fixing it.
>
> This is a second attempt to fix this. Now, with out-of-line definition.
>
>
> https://reviews.llvm.org/D31780
>
> Files:
>   lib/sanitizer_common/sanitizer_flag_parser.cc
>   lib/sanitizer_common/sanitizer_flag_parser.h
>
>
> Index: lib/sanitizer_common/sanitizer_flag_parser.h
> ===================================================================
> --- lib/sanitizer_common/sanitizer_flag_parser.h
> +++ lib/sanitizer_common/sanitizer_flag_parser.h
> @@ -22,6 +22,7 @@
>
>  class FlagHandlerBase {
>   public:
> +  virtual ~FlagHandlerBase();
>    virtual bool Parse(const char *value) { return false; }
>  };
>
> Index: lib/sanitizer_common/sanitizer_flag_parser.cc
> ===================================================================
> --- lib/sanitizer_common/sanitizer_flag_parser.cc
> +++ lib/sanitizer_common/sanitizer_flag_parser.cc
> @@ -20,6 +20,8 @@
>
>  namespace __sanitizer {
>
> +FlagHandlerBase::~FlagHandlerBase() {}
> +
>  LowLevelAllocator FlagParser::Alloc;
>
>  class UnknownFlags {
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170410/52e1e215/attachment.html>


More information about the llvm-commits mailing list