[cfe-commits] r60993 - in /cfe/trunk: include/clang/AST/DeclObjC.h lib/AST/DeclObjC.cpp lib/Sema/SemaDecl.cpp lib/Sema/SemaDeclObjC.cpp

[Fariborz Jahanian]

On Dec 15, 2008, at 8:43 AM, Douglas Gregor wrote:

> On Dec 13, 2008, at 12:28 PM, Fariborz Jahanian wrote:
>
>> Author: fjahanian
>> Date: Sat Dec 13 14:28:25 2008
>> New Revision: 60993
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=60993&view=rev
>> Log:
>> Add storage layout to ObjC classes.
>
> Cool.
>
>>
>> Modified:
>>   cfe/trunk/include/clang/AST/DeclObjC.h
>>   cfe/trunk/lib/AST/DeclObjC.cpp
>>   cfe/trunk/lib/Sema/SemaDecl.cpp
>>   cfe/trunk/lib/Sema/SemaDeclObjC.cpp
>>
>> Modified: cfe/trunk/include/clang/AST/DeclObjC.h
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/DeclObjC.h?rev=60993&r1=60992&r2=60993&view=diff
>>
>> =
>> =
>> =
>> =
>> =
>> =
>> =
>> =
>> =
>> =====================================================================
>> --- cfe/trunk/include/clang/AST/DeclObjC.h (original)
>> +++ cfe/trunk/include/clang/AST/DeclObjC.h Sat Dec 13 14:28:25 2008
>> @@ -23,6 +23,7 @@
>> class Stmt;
>> class FunctionDecl;
>> class AttributeList;
>> +class RecordDecl;
>> class ObjCIvarDecl;
>> class ObjCMethodDecl;
>> class ObjCProtocolDecl;
>> @@ -276,6 +277,8 @@
>>  Type *TypeForDecl;
>>  friend class ASTContext;
>>
>> +  RecordDecl *RecordForDecl;
>> +
>>  /// Class's super class.
>>  ObjCInterfaceDecl *SuperClass;
>>
>> @@ -312,7 +315,7 @@
>>  ObjCInterfaceDecl(SourceLocation atLoc, IdentifierInfo *Id,
>>                    SourceLocation CLoc, bool FD, bool isInternal)
>>    : NamedDecl(ObjCInterface, atLoc, Id), DeclContext(ObjCInterface),
>> -      TypeForDecl(0), SuperClass(0),
>> +      TypeForDecl(0), RecordForDecl(0), SuperClass(0),
>>      Ivars(0), NumIvars(0),
>>      InstanceMethods(0), NumInstanceMethods(0),
>>      ClassMethods(0), NumClassMethods(0),
>> @@ -347,6 +350,10 @@
>>  protocol_iterator protocol_begin() const {return  
>> ReferencedProtocols.begin();}
>>  protocol_iterator protocol_end() const { return  
>> ReferencedProtocols.end(); }
>>
>> +  void CollectObjCIvars(std::vector<FieldDecl*> &Fields);
>> +  void setRecordForDecl(RecordDecl *Decl) { RecordForDecl = Decl; }
>> +  RecordDecl *getRecordForDecl() const { return RecordForDecl; }
>> +
>
> Should setRecordForDecl be private? It looks like addLayoutToClass  
> will be the only caller.

Yes. But removed it as really not needed.

>
>
>> = 
>> = 
>> = 
>> = 
>> = 
>> = 
>> = 
>> = 
>> = 
>> =====================================================================
>> --- cfe/trunk/lib/AST/DeclObjC.cpp (original)
>> +++ cfe/trunk/lib/AST/DeclObjC.cpp Sat Dec 13 14:28:25 2008
>> @@ -338,6 +338,17 @@
>>  return 0;
>> }
>>
>> +void ObjCInterfaceDecl::CollectObjCIvars(std::vector<FieldDecl*>  
>> &Fields) {
>> +  ObjCInterfaceDecl *SuperClass = getSuperClass();
>> +  if (SuperClass)
>> +    SuperClass->CollectObjCIvars(Fields);
>> +  for (ObjCInterfaceDecl::ivar_iterator I = ivar_begin(),
>> +       E = ivar_end(); I != E; ++I) {
>> +    ObjCIvarDecl *IVDecl = (*I);
>> +    Fields.push_back(cast<FieldDecl>(IVDecl));
>> +  }
>> +}
>> +
>
> There's a nearly identical routine called CollectIvars in lib/Sema/ 
> SemaDecl.cpp, except that it creates ObjCAtDefsFieldDecls rather  
> than collecting FieldDecls. Is CollectObjCIvars meant to do the same  
> thing? And, is it part of semantic analysis (handling @defs) such  
> that it should live in Sema, or is it a core operation on Objective- 
> C AST's (and therefore should live in the AST)?

It is part of the ASTs and is needed by the clients (code gen) in  
this  case. I am collecting FieldDecl.  CollectIvars is also  
collecting the fields
for $defs AST but of a different node kind. These two can probably be  
consolidated into one. I will add a FIXME for consideration.

>
>
>> = 
>> = 
>> = 
>> = 
>> = 
>> = 
>> = 
>> = 
>> = 
>> =====================================================================
>> --- cfe/trunk/lib/Sema/SemaDecl.cpp (original)
>> +++ cfe/trunk/lib/Sema/SemaDecl.cpp Sat Dec 13 14:28:25 2008
>> @@ -3075,8 +3075,10 @@
>>      Consumer.HandleTagDeclDefinition(Record);
>>  } else {
>>    ObjCIvarDecl **ClsFields =  
>> reinterpret_cast<ObjCIvarDecl**>(&RecFields[0]);
>> -    if (ObjCInterfaceDecl *ID =  
>> dyn_cast<ObjCInterfaceDecl>(EnclosingDecl))
>> +    if (ObjCInterfaceDecl *ID =  
>> dyn_cast<ObjCInterfaceDecl>(EnclosingDecl)) {
>>      ID->addInstanceVariablesToClass(ClsFields, RecFields.size(),  
>> RBrac);
>> +      ID->addLayoutToClass(Context);
>> +    }
>>    else if (ObjCImplementationDecl *IMPDecl =
>>               dyn_cast<ObjCImplementationDecl>(EnclosingDecl)) {
>>      assert(IMPDecl && "ActOnFields - missing  
>> ObjCImplementationDecl");
>
> Will we need to have the RecordDecl layout for most or all Objective- 
> C interfaces? If most programs only need the layout for a small  
> number of interfaces, it might be beneficial to compute the layouts  
> lazily (as we do with the layout of RecordDecls).
Lay out is needed when a field is referenced. I will look at this  
later when I am done with code gen.

- Fariborz

>
>
> 	- Doug