[cfe-commits] [Windows] Use thiscall as the default calling convention for class methods

Timur Iskhodzhanov timurrrr at google.com
Wed Jul 11 05:34:14 PDT 2012


With the attached patch the cdecl test passes now.

However, I'm not very sure what I'm doing here and why it doesn't work
in the first place - can you please take a look?

On Wed, Jul 11, 2012 at 3:53 PM, Timur Iskhodzhanov <timurrrr at google.com> wrote:
> On Mon, Jul 9, 2012 at 9:46 PM, John McCall <rjmccall at apple.com> wrote:
>> On Jul 9, 2012, at 6:52 AM, Timur Iskhodzhanov wrote:
>>> On Sat, Jul 7, 2012 at 10:44 AM, John McCall <rjmccall at apple.com> wrote:
>>>> On Jun 28, 2012, at 1:39 PM, Timur Iskhodzhanov wrote:
>>>>> Part one is attached - it propagates the "isInsance" boolean to the
>>>>> two places where it should be read afterwards [in part 2].
>>>>
>>>> I didn't really like this approach, so I committed a different one as r159893.
>>>> Let me know if this helps.
>>> You've meant r159894.
>>> Yeah, I like your approach much more than mine :)
>>>
>>>>> The only logic change is around lib/CodeGen/CGCall.cpp:150.
>>>>> Without this change, the definition of `void __cdecl cdecl_method() {}`
>>>>> ends up using "thiscall" (and the call remains "cdecl" - ouch!).
>>>>> Probably it was a bug in the first place.
>>>>
>>>> Let me know if this is still a problem arising in your second patch.
>>> It still is!
>>> See the FIXME in the attached patch.
>>> Other than that, the tests pass OK and it was easy to achieve this.
>>
>> Aha, that's interesting.  Okay, I think the isVariadic bit should probably be
>> passed down to getDefaultMethodCallConv().  While you're at it, could
>> you rename that to getDefaultCXXMethodCallConv()?
> Done.
> Actually, I've found and fixed a mangler bug when adding this argument :)
> See the attached patch
>
>> Otherwise, this looks good.
> I'll try to fix the "FIXME" in the test first, otherwise this might
> break some C++ which compile and work now.
>
>> John.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: bug_12785p3.patch
Type: application/octet-stream
Size: 2696 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20120711/4ecce07d/attachment.obj>


More information about the cfe-commits mailing list