[cfe-commits] r61746 - in /cfe/trunk: include/clang/AST/ include/clang/Basic/ include/clang/Parse/ lib/AST/ lib/Parse/ lib/Sema/ test/SemaCXX/

[Douglas Gregor]

On Jan 5, 2009, at 6:20 PM, Chris Lattner wrote:

> On Jan 5, 2009, at 12:52 PM, Sebastian Redl wrote:
>>
>>
>> +
>> +  ArgType = ArgType.getUnqualifiedType();
>> +  QualType ClassType =
>> Context.getCanonicalType(Context.getTypeDeclType(
>> +    const_cast<CXXRecordDecl*>(this)));
>> +
>> +  if (ClassType != Context.getCanonicalType(ArgType))
>> +    return;
>
> Doing this comparison with types seems awkward, can you just do
> something like:
>
> if (const RecordType *RT = ArgType->getAsRecordType())
>   if (RT->getDecl() == this) ...
>
> instead?  likewise in hasConstCopyAssignment.

That won't work, since there can be multiple RecordDecls for a given  
record type.

>> +  if (getLangOptions().CPlusPlus) {
>>    CheckExtraCXXDefaultArguments(D);
>> +    if (!T->isPODType())
>> +      cast<CXXRecordDecl>(Record)->setPOD(false);
>
> I see that you basically clear the 'ispod' bit whenever something is
> parsed that prevents a class from being a POD.  Would it be better to
> make it a predicate that walks the member list to determine podness?
> The advantage of doing this is that it would put all the "podness"
> checks together into one method instead of scattered throughout sema.
> What do you think?

Most declarations of a class type will need to perform the check for a  
POD type, which is recursive (it's only a POD if all of its non-static  
data members are PODs, and so on), so it might be expensive to compute  
this property on the fly. Personally, I prefer the way Sebastian's  
doing it: there will be quite a few other class properties like this,  
too.

>
>> +
>> +    //   An implicitly-declared copy assignment operator is an
>> inline public
>> +    //   member of its class.
>> +    DeclarationName Name =
>> +      Context.DeclarationNames.getCXXOperatorName(OO_Equal);
>> +    CXXMethodDecl *CopyAssignment =
>> +      CXXMethodDecl::Create(Context, ClassDecl, ClassDecl-
>>> getLocation(), Name,
>> +                            Context.getFunctionType(RetType,
>> &ArgType, 1,
>> +                                                    false, 0),
>> +                            /*isStatic=*/false, /*isInline=*/true,
>> 0);
>> +    CopyAssignment->setAccess(AS_public);
>
> My reading of this code is that it creates the decl eagerly when the
> class is defined if it doesn't already have one.  Does this mean that
> simple things like:
>
> struct foo { int x, y; };
>
> will get these decls?

Yes. It's the same for the default constructor, copy constructor, and  
destructor, too.

>  Would it be possible to synthesize these lazily
> like G++ does?


Yes, but let's implement the semantics of these operations first, so  
that we have known, working regression tests when we go and optimize  
away these implicitly-declared special member functions.

	- Doug