[cfe-commits] r49565 - in /cfe/trunk: Driver/ASTConsumers.cpp include/clang/AST/DeclObjC.h include/clang/Parse/Action.h lib/AST/DeclObjC.cpp lib/Parse/ParseObjc.cpp lib/Sema/Sema.h lib/Sema/SemaDeclObjC.cpp

[Chris Lattner]

On Apr 14, 2008, at 10:19 AM, Fariborz Jahanian wrote:
>>> +    for (unsigned i = 0; i < NumPropertyDeclarators; i++) {
>>> +      Declarator &D = propertyDeclarators[i].D;
>>> +      propertyName[i] = new NamedDecl(Decl::ObjCProperty,
>>> +                                      D.getIdentifierLoc(),  
>>> D.getIdentifier());
>>
>> You should not 'new' NamedDecl, it is supposed to be an abstract  
>> class.  After this gets fixed, we should mark its ctor 'protected'.
>
> This is news to me. See below.

Right, this is a relatively recent change to use foo::Create instead  
of 'new foo'.

>> Finally, one *big* question: you seem to model:
>>
>> @property int x, y, z;
>>
>> as one PropertyDecl with three "NamedDecl"'s hanging off it.  Why  
>> not model this as three PropertyDecls?  This way PropertyDecl could  
>> really be a NamedDecl, and this more closely matches how we handle  
>> ivars and struct fields.
>
> My goal was to preserve the syntax of a property declaration in AST.  
> So, we can rewrite it exactly as is written. I ASTed property for  
> what they are; list of attributes, a type, and list of names.  But  
> now that you foresee a NamedDecl to be an abstract class in the  
> future, I will change them as you suggested. Assuming that we don;t  
> care about preserving the syntactic information exactly.

Right, we don't preserve this info for struct fields, so I think  
properties should be handled the same way.  When/if we ever care to do  
this, we can fix both cases at the same time.

Thanks Fariborz!

-Chris