On Tue, Jun 5, 2012 at 2:45 AM, Daniel Jasper <[email protected]> wrote:
> On Mon, Jun 4, 2012 at 10:42 PM, Richard Smith <[email protected]>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.
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
