On May 15, 2012, at 11:09 AM, Timur Iskhodzhanov wrote: > On Fri, May 11, 2012 at 9:55 PM, John McCall <[email protected]> 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. _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
