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

[John McCall]

On May 15, 2012, at 11:09 AM, Timur Iskhodzhanov wrote:
> On Fri, May 11, 2012 at 9:55 PM, John McCall <rjmccall at apple.com> wrote:
>> On May 11, 2012, at 6:58 AM, Timur Iskhodzhanov wrote:
>>> Can you please review the attached patch?
>>> 
>>> In two words what it does is
>>> it overrides the CC_Default calling convention with
>>> context.getDefaultMethodCallConv()
>>> for all the used method definitions and calls.
>>> 
>>> What I personally don't like is that the method declarations are
>>> stored with CC_Default and we have to replace that with
>>> context.getDefaultMethodCallConv() wherever we use it.
>>> Unfortunately, we can't change the CC in the global declarations storage.
>>> I'd prefer to put the declarations into the storage with the right CC
>>> but not sure:
>>> a) if we should do it
>>> b) how to do it
>>> c) it's possible at all
>> 
>> I don't think modifying the AST is the right thing to do here anyway;
>> CC_Default is really the right way to model this.
> ok
> 
>>> OTOH, it looks like there are only two places we need to alter the
>>> calling convention: method call and method definition :)
>>> So maybe I'm trying too hard to generalize...
>> I mostly like your patch, but I think it'd be slightly better to make the
>> terminal arrangeFunctionType take some sort of "function kind"
>> parameter (function vs. C++ instance method) and then have
>> ClangCallConvToLLVMCallConv consider that when mapping
>> CC_Default.
>> 
>> Also, please put this in a more generically-named test, something like
>> microsoft-abi-methods.cpp.  I'm sure there will be other changes that
>> we'll want to test for as time goes on.
> I got your point.
> I'll try doing that - I've started with tests and this has
> uncovered a few bugs in ctor/dtor handling.
> Will update as soon as it's fixed.

Thanks!

>> Also, please test:
>>  - declarations of instance methods using CC_Default
> you mean, declarations without explicitly specified CC ?

Yes.

>>  - declarations of instance methods with an explicit CC
> Would you prefer __thiscall/__cdecl or __attribute__ ?

If we predefine __thiscall in MS mode, then testing that way is fine.

John.