[cfe-commits] [PATCH] Abstract v-table generation into the CGCXXABI interface

Timur Iskhodzhanov timurrrr at google.com
Thu Oct 4 06:09:29 PDT 2012


[and now to the new Charles's address]

On Thu, Oct 4, 2012 at 5:07 PM, Timur Iskhodzhanov <timurrrr at google.com> wrote:
> Actually, looking at the patch more closely now I don't get its idea at all.
>
> Charles have added a EmitVTables virtual function to the CGCXXABI
> interface which does sound like a reasonable move at first.
>
> However, the ItaniumCXXABI implementation:
>   void ItaniumCXXABI::EmitVTables(const CXXRecordDecl *Class) {
>     CGM.getVTables().GenerateClassData(CGM.getVTableLinkage(Class), Class);
>   }
> calls non-abstract CGM member's method.
> I'm pretty sure the MicrosoftCXXABI::EmitVTables implementation should
> be the same (then why do we need virtual EmitVTables at all?)
> and that the abstraction should be done at a different level -
> VTableBuilder? VTableContext? [see a new thread in your inbox soon]
>
> What was the idea?
>
> Timur Iskhodzhanov,
> Google Russia
>
>
>
> On Fri, Jun 29, 2012 at 4:14 AM, Timur Iskhodzhanov <timurrrr at google.com> wrote:
>> On Thu, Jun 28, 2012 at 5:11 PM, John McCall <rjmccall at apple.com> wrote:
>>> On Jun 28, 2012, at 4:08 PM, Timur Iskhodzhanov wrote:
>>>> FTR, this patch has regressed:
>>>> http://llvm.org/bugs/show_bug.cgi?id=13231
>>>>
>>>> Now I can't build non-trivial C++ code anymore.
>>>> [I had local patches to work OK on some files and was planning to send
>>>> a few things for review but now I'm literally stuck at ~r158985]
>>>
>>> There are going to be growing pains like this — I do understand your use
>>> case of "I'd just like as much to work as possible, even if it's not using the
>>> right logic for MSVC at all", but as long as we have multiple people working
>>> on the ABI, that's not really supportable.
>>>
>>> I think Charles is working on patches to actually implement this, though,
>>> so it might not be too long.  In the meantime, I think you should either
>>> bite the bullet and propose a patch to add a -cc1 flag to opt in to
>>> inconsistent ABI generation (understanding that that means you'll get
>>> miscompiled from time to time), or just locally make the Microsoft
>>> implementation call the Itanium implementation.
>> I totally understand this.
>> It would be good if refactoring didn't break already-almost-working
>> stuff though or at least there was a clear estimate of when it's going
>> to be fixed :)




More information about the cfe-commits mailing list