[cfe-commits] r76776 - /cfe/trunk/lib/AST/DeclCXX.cpp

[Fariborz Jahanian]

Done in r77030

- Thanks Fariborz

On Jul 24, 2009, at 5:04 PM, Daniel Dunbar wrote:

> On Wed, Jul 22, 2009 at 1:25 PM, Fariborz  
> Jahanian<fjahanian at apple.com> wrote:
>> Author: fjahanian
>> Date: Wed Jul 22 15:25:00 2009
>> New Revision: 76776
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=76776&view=rev
>> Log:
>> Improved on performance of the algorithm for proper ordering of
>> ctor's initialization of bases and fields.
>
> Cool
>
>> +  llvm::DenseMap<const void *, CXXBaseOrMemberInitializer*>  
>> AllBaseFields;
>> +
>> +  for (unsigned i = 0; i < NumInitializers; i++) {
>> +    CXXBaseOrMemberInitializer *Member = Initializers[i];
>> +    const void * Key = Member->isBaseInitializer() ?
>> +      reinterpret_cast<const void *>(
>> +                                   Member->getBaseClass()- 
>> >getAsRecordType()) :
>> +      reinterpret_cast<const void *>(Member->getMember());
>> +    AllBaseFields[Key] = Member;
>> +  }
>
> I think this is more readable as:
> --
> for ... {
>    CXXBaseOrMemberInitializer *Member = Initializers[i];
>    if (Member->isBaseInitializer())
>      AllBaseFields[Member->getBaseClass()->getAsRecordType()] =  
> Member;
>    else
>      AllBaseFields[Member->getMember()] = Member;
> }
> --
>
>> +
>>   // Push virtual bases before others.
>>   for (CXXRecordDecl::base_class_iterator VBase =
>>        ClassDecl->vbases_begin(),
>>        E = ClassDecl->vbases_end(); VBase != E; ++VBase) {
>> -    const Type * T = VBase->getType()->getAsRecordType();
>> -    unsigned int i = 0;
>> -    for (i = 0; i < NumInitializers; i++) {
>> -      CXXBaseOrMemberInitializer *Member = Initializers[i];
>> -      if (Member->isBaseInitializer() &&
>> -          Member->getBaseClass()->getAsRecordType() == T) {
>> -        AllToInit.push_back(Member);
>> -        break;
>> -      }
>> -    }
>> -    if (i == NumInitializers) {
>> +    const void *Key = reinterpret_cast<const void *>(
>> +                                          VBase->getType()- 
>> >getAsRecordType());
>
> I don't think this case is necessary?
>
>> +    if (AllBaseFields[Key])
>> +      AllToInit.push_back(AllBaseFields[Key]);
>
> This should be
> --
> if (CXXBaseOrMemberInitializer *Value = AllBaseFields.lookup(Key))
>  AllToInit.push_back(Value)
> --
> Otherwise it looks up the value twice, and inserts a null binding  
> into the map.
>
>> +    const void *Key = reinterpret_cast<const void *>(
>> +                                          Base->getType()- 
>> >getAsRecordType());
>> +    if (AllBaseFields[Key])
>> +      AllToInit.push_back(AllBaseFields[Key]);
>
> Same two comments as before.
>
>> +    const void * Key = reinterpret_cast<const void *>(*Field);
>> +    if (AllBaseFields[Key]) {
>> +      AllToInit.push_back(AllBaseFields[Key]);
>> +      continue;
>
> Ditto.
>
> - Daniel