[PATCH] Re-enabling another warning
aaron at aaronballman.com
Sat Jul 27 12:50:14 PDT 2013
On Sat, Jul 27, 2013 at 3:40 PM, David Blaikie <dblaikie at gmail.com> wrote:
> On Sat, Jul 27, 2013 at 12:27 PM, Aaron Ballman <aaron at aaronballman.com> wrote:
>> On Sat, Jul 27, 2013 at 2:59 PM, David Blaikie <dblaikie at gmail.com> wrote:
>>> On Sat, Jul 27, 2013 at 11:48 AM, Aaron Ballman <aaron at aaronballman.com> wrote:
>>>> This patch fixes a few cases where a default destructor would not be
>>>> generated due to a member variable having an inaccessible default
>>>> destructor. While the original code was correct, the updated code is
>>>> a bit more clear. Assuming that this new code is acceptable, this
>>>> also re-enables the C4624 MSVC warning (''derived class' : destructor
>>>> could not be generated because a base class destructor is
>>> That code change does seem a bit strange/questionable/unnecessary.
>> This is why I was passing it by the lists. ;-)
>>> While I'm OK enabling warnings in MSVC that have some substantial
>>> overlap with Clang warnings to help developers working with MSVC keep
>>> their work Clang-clean, generally we've taken the line that if a
>>> warning isn't viable for Clang, it's not worth turning on in some
>>> other compiler for LLVM. Does this warning catch useful bugs? (Why
>>> would you need a warning for cases when a dtor couldn't be generated
>>> when you'd get an error if/when you actually call that dtor? (because
>>> that latter warning is hard to understand? That would be unfortunate))
>> I think the benefit is that it points out when destructors are not
>> generated, which isn't always obvious.
> Why would it matter if the dtor is never called? If it is called, I
> imagine you get an actual error, no?
You would, but the error isn't likely to be as obvious as it is with
an explicit destructor (at least in MSVC).
>>> I wonder why making an explicit dtor works? If the base class dtor is
>>> inaccessible, wouldn't it still be inaccessible even in an explicit
>> The warning is about generating the implicit destructor for the
>> derived class. By providing an explicit destructor for the derived
>> class, the implicit one cannot be generated so the warning doesn't
> But either explicit or implicit, the dtor has to call the base dtor -
> why would it be accessible when explicit but inaccessible when
The base class destructor would remain inaccessible.
> OK, now I've looked at the code I'm even more confused -
> AlignmentCalcImpl has no base classes nor do any classes derive from
> it. So which base/derived classes is this diagnostic referring to?
The warning isn't particularly well-worded IMO. The problem with
AlignmentCalcImpl is the T -- for some values of T, the destructor is
inaccessible. Since there's a value type of T in the class with an
inaccessible destructor, AlignmentCalcImpl does not get an implicit
I don't feel incredibly strongly about this particular warning, but I
think we should strive to have as few warnings turned off by default
as possible (regardless of compiler). This one isn't as egregious as
some others I've re-enabled recently, but I do still think it can make
the code slightly more clear when being explicit.
More information about the cfe-commits