[llvm-commits] [llvm] r147651 - in /llvm/trunk/lib/CodeGen/AsmPrinter: DwarfAccelTable.cpp DwarfAccelTable.h DwarfCompileUnit.cpp DwarfCompileUnit.h DwarfDebug.cpp

[Eric Christopher]

> 
> I'd push this in DebugInfo and simply use DICompositeType.isImplementation() here.
> Please add comment clarifying how you're treating C++ here (:)) and why.
> 

I've added the comment, I wanted to keep all of the flag values out of the table code if possible in case we want more flags some day.

>> +    
>> +    addAccelType(Ty.getName(),
>> +                 std::make_pair(TyDIE,
>> +                                (IsImplementation ?
>> +                               DwarfAccelTable::eTypeFlagClassIsImplementation :
>> +                                 0)));
>> +  }
> 
> For sake of readers eye, I'd just hoist ? and std::make_pair outside the call and trust optimizer.
> 

Something like this done :)

>> 
>>  addToContextOwner(TyDIE, Ty.getContext());
>>  return TyDIE;
>> 
>> Modified: llvm/trunk/lib/CodeGen/AsmPrinter/DwarfCompileUnit.h
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/AsmPrinter/DwarfCompileUnit.h?rev=147651&r1=147650&r2=147651&view=diff
>> ==============================================================================
>> --- llvm/trunk/lib/CodeGen/AsmPrinter/DwarfCompileUnit.h (original)
>> +++ llvm/trunk/lib/CodeGen/AsmPrinter/DwarfCompileUnit.h Thu Jan  5 22:35:23 2012
>> @@ -65,7 +65,7 @@
>>  StringMap<std::vector<DIE*> > AccelNames;
>>  StringMap<std::vector<DIE*> > AccelObjC;
>>  StringMap<std::vector<DIE*> > AccelNamespace;
>> -  StringMap<std::vector<DIE*> > AccelTypes;
>> +  StringMap<std::vector<std::pair<DIE*, unsigned> > > AccelTypes;
> 
> It is a good idea to have typedef here. I see vector<pair<> > multiple times.
> 

Haven't done this yet. Good idea though :)

>> 
>> -      std::vector<DIE *> Entities = GI->second;
>> -      for (std::vector<DIE *>::const_iterator DI = Entities.begin(),
>> -             DE= Entities.end(); DI !=DE; ++DI)
>> -        AT.AddName(Name, (*DI));
>> +      std::vector<std::pair<DIE *, unsigned> > Entities = GI->second;
> 
> Do you really want to copy the entire vector or just get a reference to iterate over ?
> 

Reference to iterate over, nice catch.

Thanks!

-eric