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

[John McCall]

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.