Better support for constructors with -cxx-abi microsoft

[Timur Iskhodzhanov]

That's a great suggestion!

Attached is a patch that does as you've described and also addresses
all the TODOs I've left for myself in the previous patch.

One questionable change is the removal of "static" for GetVTTParameter.
I didn't want to make it a CGF method as it should be used anywhere
except ItaniumCXXABI.cpp but we have to use it in CGClass.cpp until
the full abstraction is finished.
Please tell me if I cut a corner too much there and how to do it better.

2013/2/26 John McCall <rjmccall at apple.com>:
> On Feb 25, 2013, at 9:02 AM, Timur Iskhodzhanov <timurrrr at google.com> wrote:
>> Attached is a preliminary patch for ctors in -cxx-abi microsoft.
>>
>> It adds:
>> - support for out-of-class constructor definitions (PR12784),
>> - reading and writing proper ctor arguments when dealing with virtual bases.
>> I also tried this patch on some "build, run and verify" tests for
>> virtual inheritance vs ctors and it works great.
>>
>> Can you please take a look at the implementation and tell me if there
>> are any logical mistakes?
>>
>> Also, I've put two TODOs in the code with questions.
>> It would be nice if you could take a look at the logic first and
>> suggest me if my CGCXXABI abstraction plans are sane - before I copy
>> any code between files.
>>
>> If the logic of the patch and the abstraction suggestions are fine,
>> I'll move the code around as described and prepare a patch ready for
>> review.
>
> I think the overall structure here is fine.
>
> +      CXXCtorInitializer *Member = *B;
> +      if (!Member->isBaseInitializer())
> +        continue;
>
> The member and base initializers should already be in the correct
> order, with the virtual base initializers coming first.  That's something
> we can take advantage of by breaking up the loop like so:
>
>   CXXConstructorDecl::init_const_iterator B = CD->init_begin(), E = CD->init_end();
>
>   // Branch on implicit parameter here if necessary.
>   // You can abstract this with some sort of method like this:
>   //   llvm::BasicBlock *emitCtorCompleteObjectCheck();
>   // The rule would be that it only gets implemented for ABIs without
>   // constructor variants, and that it returns the continuation block if a
>   // branch was required.
>   llvm::BasicBlock *baseCtorContBB = 0;
>   if (CD->getParent()->hasVBases() && !ABI.hasConstructorVariants()) {
>     baseCtorContBB = CGM.getCXXABI().emitCtorCompleteObjectCheck();
>   }
>
>   // Emit all the virtual base initializers.
>   while (B != E && (*B)->isBaseInitializer() && (*B)->isBaseVirtual()) {
>     if (CtorType == Ctor_Base) continue;
>     EmitBaseInitializer...
>   }
>
>   // Enter continuation block for conditional branch
>   if (baseCtorContBB) EmitBlock(baseCtorContBB);
>
>   // Emit all the non-virtual base initializers.
>   while (B != E && (*B)->isBaseInitializer()) {
>     assert(!(*B)->isBaseVirtual());
>     EmitBaseInitializer...
>   }
>
>   // Okay, everything else.
>   InitializeVTablePointers
>   ConstructorMemcpyizer CM(...);
>   while (B != E) {
>     assert((*B)->isMemberInitializer());
>     CM.addMember...
>   }
>   CM.finish();
> }
>
> John.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: ctors_2.patch
Type: application/octet-stream
Size: 29356 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130226/8f585796/attachment.obj>