[PATCH] Re-enabling another warning

[David Blaikie]

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
>>> inaccessible').
>>
>> 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?

>
>> 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
>> dtor?
>
> 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
> apply.

But either explicit or implicit, the dtor has to call the base dtor -
why would it be accessible when explicit but inaccessible when
implicit?

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?

>
>> (& can we workaround this some other way? What happens if the default
>> ctor uses LLVM_DELETED_FUNCTION instead of an empty definition, does
>> that suppress the dtor warning? I doubt it, but figured I'd throw that
>> out there...)
>
> We might be able to, but I think the benefit is in being explicit
> about whether the destructor is accessible or not from within the
> derived class.
>
> ~Aaron