[cfe-commits] Clang change to detect and warn about unused private members

[Richard Smith]

On Tue, Jun 5, 2012 at 2:45 AM, Daniel Jasper <djasper at google.com> wrote:

> On Mon, Jun 4, 2012 at 10:42 PM, Richard Smith <richard at metafoo.co.uk>wrote:
>
>> I mean that we should not consider a class to be completely defined if
>> it has a pure virtual destructor with no definition in this TU.
>>
>
> Done and test added.
>
[...]

> >> (See clang::DependentFunctionTempateSpecializationInfo for a
>>
> >> real-world example of this.) It looks like we won't diagnose that case
>> >> (because Aligner is technically public, despite being effectively a
>> >> private member of S). But I suspect we will diagnose this (and
>> >> probably shouldn't):
>> >>
>> >> union S {
>> >>  // ... no use of Aligner ...
>> >> private:
>> >>  void *Aligner;
>> >>  unsigned char Data[8];
>> >> };
>> >>
>> >> Perhaps we should conservatively disable the diagnostic for union
>> members?
>> >
>> >
>> > Why should we not warn in this case? I think in general we might want to
>> > exempt char-arrays as they are commonly use for alignment, but other
>> than
>> > that?
>>
>> Aligner is 'unused', but has a semantic effect. Note that in the
>> clang::DependentFunctionTempateSpecializationInfo case, there is no
>> char array. I'm concerned that a warning for cases like this would
>> have a high false positive rate. (I have the same concern for padding
>> members in structs, but I would expect those to generally be public,
>> since such structs are usually intended to be PODs.)
>>
>
> Ah, (things-I-know-about-C++)++. :-)
> I know exclude unions from this analysis.
>
> New patch attached.
>

Thanks! The new patch looks good to me, other than some redundant braces
(in particular in IsRecordFullyDefined and CollectFieldInitializer). I'll
leave final approval on this to Jordy.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20120605/89143a47/attachment.html>