[Review, -cxx-abi microsoft] Implement proper handling of virtual destructors (single-inheritance case)

Timur Iskhodzhanov timurrrr at google.com
Fri Feb 1 09:18:32 PST 2013


Ah, I see.
I should probably look at the ItaniumMangler more often :)

Attached is the patch that resolves both issuew Charles has asked me to fix.
Anything else I should change?

2013/1/31 John McCall <rjmccall at apple.com>:
> On Jan 29, 2013, at 11:15 AM, Timur Iskhodzhanov <timurrrr at google.com> wrote:
>> 2013/1/29 Charles Davis <cdavis5x at gmail.com>:
>>>
>>> On Jan 29, 2013, at 9:58 AM, Timur Iskhodzhanov wrote:
>>>
>>>> 2013/1/28 Timur Iskhodzhanov <timurrrr at google.com>:
>>>>> Two problems:
>>>>> 1) I was optimistically running only the new tests and didn't notice
>>>>> I've broken one of my own earlier tests in the Clang sources. I'll fix
>>>>> that soon
>>>>>
>>>>> 2) CGM code has changed over the weekend, I'll have to rebase.
>>>>>
>>>>> I'll ping this thread with a new patch when I resolve these.
>>>> Here it is attached.
>>>> Sorry for the delay, the Windows build was broken a few times in the
>>>> interim and also my VS checkout was borked for some reason.
>>>>
>>>> Please note that I had to change the test/CodeGenCXX/arm.cpp test
>>>> because I believe it is wrong :)
>>>> See the other e-mail thread for the details.
>>>
>>>> diff --git lib/AST/MicrosoftMangle.cpp lib/AST/MicrosoftMangle.cpp
>>>> index 0b77ac8..5a68704 100644
>>>> --- lib/AST/MicrosoftMangle.cpp
>>>> +++ lib/AST/MicrosoftMangle.cpp
>>>> @@ -47,10 +47,11 @@ class MicrosoftCXXNameMangler {
>>>>
>>>> public:
>>>>   MicrosoftCXXNameMangler(MangleContext &C, raw_ostream &Out_)
>>>> -  : Context(C), Out(Out_), UseNameBackReferences(true) { }
>>>> +    : Context(C), Out(Out_), UseNameBackReferences(true), DtorType(Dtor_Complete) { }
>>>>
>>>>   raw_ostream &getStream() const { return Out; }
>>>>
>>>> +  CXXDtorType DtorType;  // FIXME: pass as arg?
>>> Yes, it should be an argument. It seems like a simple enough change.
>> Can you please suggest me a simple way to do this?
>> Currently, MicrosoftMangleContext::mangleCXXDtor calls the
>> MicrosoftCXXNameMangler::mangle(const NamedDecl *D).
>> I don't think adding a CXXDtorType parameter to a generic
>> mangle(NamedDecl) is appropriate.
>> I can probably add ::mangle(const CXXDestructorDecl*,CXXDtorType)
>> though but this might make things harder, e.g. what happens when
>> declaring a static variable inside a dtor?..
>
> Then the call to mangle the context of the variable recognizes that the
> context is a destructor and decides which to use.  The Itanium
> implementation does this.  Making a member variable does not seem
> appropriate.
>
> John.



-- 
Timur Iskhodzhanov,
Google Russia
-------------- next part --------------
A non-text attachment was scrubbed...
Name: bug_15058_3.patch
Type: application/octet-stream
Size: 33624 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130201/c02b1283/attachment.obj>


More information about the cfe-commits mailing list