[cfe-commits] r43268 - in /cfe/trunk: CodeGen/CodeGenTypes.cpp CodeGen/CodeGenTypes.h test/CodeGen/struct.c

[Devang Patel]

On Oct 23, 2007, at 5:04 PM, Chris Lattner wrote:

>
> On Oct 23, 2007, at 4:26 PM, Devang Patel wrote:
>
>> Author: dpatel
>> Date: Tue Oct 23 18:26:46 2007
>> New Revision: 43268
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=43268&view=rev
>> Log:
>> Handle nested structs.
>> typdef struct A { int i; struct A *next; } A
>
> What, no more infinite looping? :)
>
>> =
>> =
>> =
>> =
>> =
>> =
>> =
>> =
>> =
>> =====================================================================
>> --- cfe/trunk/CodeGen/CodeGenTypes.cpp (original)
>> +++ cfe/trunk/CodeGen/CodeGenTypes.cpp Tue Oct 23 18:26:46 2007
>> @@ -167,13 +167,36 @@
>>       return ConvertType(cast<EnumDecl>(TD)->getIntegerType());
>>     } else if (TD->getKind() == Decl::Struct) {
>>       const RecordDecl *RD = cast<const RecordDecl>(TD);
>> +
>> +      // If this is nested record and this RecordDecl is already  
>> under
>> +      // process then return associated OpaqueType for now.
>> +      llvm::DenseMap<const RecordDecl *, llvm::Type *>::iterator
>> +	OpaqueI = RecordTypesToResolve.find(RD);
>
> Tabs tabs tabs, oh my!

It is all Emacs' fault. It should figure out settings based on source  
file name and its relative location inside a project. C'mon this is 21  
century. :) :)

> Should there be a helper method that does the RecordTypesToResolve  
> lookup?  It could return a recorddecl* or null pointer if not in the  
> map.

This is only one lookup. But helper method is fine with me. ok.

>> +      if (OpaqueI != RecordTypesToResolve.end())
>> +	return OpaqueI->second;
>> +
>> +      // Create new OpaqueType now for later use.
>> +      llvm::OpaqueType *OpaqueTy =  llvm::OpaqueType::get();
>> +      RecordTypesToResolve[RD] = OpaqueTy;
>
> Ah, I see what you mean about needing to always create an opaque  
> type.  Yeah, this is fine for now, but please add a "// FIXME: this  
> creates a lot of opaque types, most of which are not needed.   
> Reevaluate this when we are farther along" or something.

ok

>> +      // Layout fields.
>>       RecordOrganizer *RO = new RecordOrganizer();
>>       for (unsigned i = 0, e = RD->getNumMembers(); i != e; ++i)
>> 	RO->addField(RD->getMember(i));
>>       RO->layoutFields(*this);
>> +
>> +      // Get llvm::StructType.
>>       RecordLayoutInfo *RLI = new RecordLayoutInfo(RO);
>>       ResultType = RLI->getLLVMType();
>>       RecordLayouts[ResultType] = RLI;
>> +
>> +      // Refine any OpaqueType associated with this RecordDecl.
>> +      OpaqueTy->refineAbstractTypeTo(ResultType);
>> +      OpaqueI = RecordTypesToResolve.find(RD);
>
> This is good in spirit, but needs some changes to actually work.   
> The issue is that resolving types can cause types to merge and fold  
> away, causing dangling pointer problems.  We'll have to adopt the  
> llvm-types.cpp approach of using a map with PATypeHolder in them.   
> PATypeHolder is automatically updated as types are merged.


Well, I completely missed this part. I'll read how llvm-types.cpp  
handles this.
Thanks,
-
Devang