[PATCH] Re-enabling another warning

[Aaron Ballman]

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.

> 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.

> (& 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