[cfe-dev] Review Request: Plain Old Data (POD) property of classes, and type traits basics

[Sebastian Redl]

Douglas Gregor wrote:
> Hi Sebastian,
>
> On Jan 2, 2009, at 6:57 PM, Sebastian Redl wrote:
>> However, I'm not quite sure about all aspects of the patch, so I'm
>> posting it for review first. In particular, I have an enum called
>> UnaryTypeTrait that represents the various type trait keywords. Since it
>> is needed in the parser, the sema and the AST, I saw no other choice but
>> to make it part of Basic. Is this the right thing to do?
>
> Yes, I think so. AccessSpecifier, which currently lives in Parse,
> should also be moved back to Basic (but that's my problem, not yours).
OK.
>
> +/// UnaryTypeTraitExpr - A GCC or MS unary type trait, as used in the
> +/// implementation of TR1/C++0x type trait templates.
>
> Please add an example to the documentation for this class.
Done.
>
> +  // FIXME: Some of the type traits have requirements. Interestingly,
> only the
> +  // __is_base_of requirement is explicitly stated to be diagnosed.
> Indeed,
> +  // G++ accepts __is_pod(Incomplete) without complaints, and claims
> that the
> +  // type is indeed a POD.
>
> Interesting. The documentation for __is_pod says:
>
>     Requires: type shall be a complete type, an array type of unknown
> bound, or is a void type.
>
The candidate draft says the same thing, word for word, about
std::is_pod<T>.
> So GCC should be diagnosing this kind of error. We should report the
> problem to them. However, we can address this FIXME later. The point
> of the patch is PODs, right? :)
>
> In Type::isPODType, could you add a FIXME for pointer-to-member types?
> (I almost want to just add basic pointer-to-member support now, since
> we seem to have lots of FIXMEs for them).
Done. The FIXME, not member pointers. :-(
>
> Regarding the FIXME at the end of
> CXXRecordDecl::hasConstCopyAssignment, I think we should assert that
> the CXXRecordDecl itself is marked "invalid". We shouldn't get here in
> any well-formed program.
OK.
>
> This patch looks really good to me. Thanks!
Thanks for the review.

Sebastian