[llvm] r216170 - Make format_object_base's destructor protected and non-virtual.

[David Blaikie]

On Fri, Aug 22, 2014 at 10:38 AM, Owen Anderson <resistor at mac.com> wrote:
>
> On Aug 21, 2014, at 2:14 PM, David Blaikie <dblaikie at gmail.com> wrote:
>
> On Thu, Aug 21, 2014 at 1:34 PM, Justin Bogner <mail at justinbogner.com>
> wrote:
>
> Benjamin Kramer <benny.kra at googlemail.com> writes:
>
> Author: d0k
> Date: Thu Aug 21 06:22:05 2014
> New Revision: 216170
>
> URL: http://llvm.org/viewvc/llvm-project?rev=216170&view=rev
> Log:
> Make format_object_base's destructor protected and non-virtual.
>
> It's not meant to be used with operator delete and this avoids
> emitting virtual dtors for every derived format object.
>
>
> This change is obviously for the better, but it triggers a warning when
> building with clang 3.4 (and probably every clang back to 2.9 or so),
> which is pretty unfortunate.
>
> The problem is, until r208449, we didn't know not to warn about this on
> final classes or when the destructor was protected. I'm not entirely
> sure what we should do here, but this is likely to break anyone who
> builds with -Werror for a while.
>
>
> Only if they don't update their compiler - I usually just roll my
> clang release compiler (disabling -Werror) when this sort of thing
> comes up.
>
> For those who want to use old/pre-installed/official release clangs
> rather than more recent iterations, they can just disable -Werror
> (it's not the default anyway).
>
>
> This isn’t a workable solution for some of us who are tied to specific
> toolchain releases and can’t upgrade on a moment’s notice.  And even if I’m
> not building with -Werror, this change generates hundreds or thousands of
> warnings across an LLVM build, which makes it practically impossible to do
> any development work because of my own work getting lost in the sea of
> noise.

You can disable the warning - as with bad MSVC/GCC warnings, we
usually rely on buildbots/other developers to catch the bugs that slip
through from users on compilers with buggy diagnostics.

If there are enough such users that it's annoying for each of them to
disable the warning individually, it would be possible to add some
clang version detection to auto-disable the warning.

(that said, I'm not going to adamantly rail against anyone if the
prevailing opinion is that it's worth just adding unnecessary virtual
dtors to such hierarchies while we wait for those users to catch up -
put a big fixme comment in and move on... )