[PATCH] PR13457 (part 1) explicit/implicit calling conventions compatibility in overloads

[Reid Kleckner]

So, after digging into this, I discovered that even though we have enums
for the calling conventions in AttributedType, they are dead.  They are
only used in case labels.  John added AttributedType with the enum in 2011
here:
http://llvm.org/viewvc/llvm-project?view=revision&revision=122942

I think his intention was that we should be using AttributedType for
calling conventions.  Today what we do is unwrap and rewrap the
FunctionType with a new calling convention.  CC_Default is used everywhere
there wasn't an explicit attribute.  There are exceptions like -mrtd which
force the CC to stdcall, but it looks pretty buggy with lots of checks to
try to infer if the stdcall convention was applied by -mrtd or explicitly
with an attribute.

It looks like in the long run handleFunctionTypeAttr should be doing what
it does today, except it should wrap FunctionType in some AttributedType
sugar which points to the original function type and the new function type.

Does that make sense?

On Mon, Jul 8, 2013 at 5:37 PM, Reid Kleckner <rnk at google.com> wrote:

> On Tue, Mar 26, 2013 at 1:39 PM, Alexander Zinenko <ftynse at gmail.com>wrote:
>
>> Hello Richard, Timur,
>>
>> I tried different approaches to this issue, but they are not elegant
>> enough as for me.
>>
>> The first is to replace CC_Default with explicit default CC in
>> declarations.  It requires changing function, typedef, variable and
>> parameter declarations to address our problem.  But this approach results
>> in over 20 test failures due to diagnostics change (adds extra
>> __attribute__ to function type).  Although attribute printing is not
>> perfect now since it prints __attribute__((cdecl)) instead of __cdecl,
>> adding extra attributes in the output may be unwanted.
>>
>
> I talked to Richard last week, and I think we agreed that this is the way
> to go.  There are two places where it matters if the CC is implicit or
> explicit: merging attributes on redeclaration, and type printing.  In both
> cases, you should be able to detect an explicit calling convention by
> looking for AttributedType sugar.
>
> Is this something you can work on in the near future, or would it be OK if
> I looked into this approach?  You have awesome tests, by the way.  :)
>
> I think DeclaratorChunk might need more information in order to get the CC
> right to handle cases like:
> struct Foo {
>   void (*(*foo())())();
> };
>
> If I didn't spell it right, that's supposed to be a method that returns a
> function ptr that returns a function ptr.  Parameters have their own
> parsing context, so that's not an issue.
>
> Similarly, a member function pointer type would need to use thiscall.  I
> believe the whole declarator is parsed in a MemberContext, so that isn't
> enough info to pick the CC.
>
>
>> Another one is to try changing only the canonical type.  Function
>> canonical types are handled in ASTContext::getFunctionType and we already
>> have a notion of canonical calling convention (for MRTD).  But MS ABI has
>> different default calling conventions for instance methods and everything
>> else.  FunctionType does not differentiate between method types and free
>> function types as well as has no information about storage classes.  We can
>> introduce extra arguments with default values to getFunctionType, but it
>> might clutter the code and doesn't look like a good idea.
>>
>> The third one is the inverse to the first: change explicit default CCs to
>> CC_Default.  This doesn't work because it breaks declaration merge:
>> void __cdecl foo(); // CC_C is default, so drop it. Becomes equivalent to
>> void foo().
>> void __stdcall foo(); // Set CC_X86StdCall since the previous was without
>> CC (we dropped it).
>>
>> And the last one is my initial approach in overload resolution.
>>
>> I would like to have an advice which approach do we prefer?
>>
>>
>>
>> On 20 March 2013 23:19, Richard Smith <richard at metafoo.co.uk> wrote:
>>
>>> On Tue, Mar 19, 2013 at 10:24 AM, Alexander Zinenko <ftynse at gmail.com>wrote:
>>>
>>>> This is possible when acting on FunctionDecl, but it can affect
>>>> diagnostics reporting in the following way: when outputting FunctionType
>>>> with non-default CC, diagnostics engine assumes CC was specified explicitly
>>>> via __attribute__ syntax.  I am not sure this is a desired behavior.  It
>>>> would be better if there was a way to work around this, does it exist?
>>>>
>>>
>>> FunctionDecls have both a canonical type and a type as written. This
>>> transformation should only be applied to the canonical type.
>>>
>>>
>>>> By the way, we already have such behavior if an unknown CC is
>>>> automatically replaced with CC_C on some targets, like here
>>>> void __stdcall foo(); double x = foo;
>>>> cannot initialize a variable of type 'double' with an lvalue of type
>>>> 'void () __attribute__((cdecl))'
>>>>
>>>> On 19 March 2013 00:32, Richard Smith <richard at metafoo.co.uk> wrote:
>>>>
>>>>> On Fri, Mar 1, 2013 at 2:39 PM, Alexander Zinenko <ftynse at gmail.com>wrote:
>>>>>
>>>>>> Hello!
>>>>>>
>>>>>> This patch addresses the compatibility issue between explicitly
>>>>>> specified default calling conventions and implicit ones in overload
>>>>>> resolution.  As of now, such implicit conversions are only known in
>>>>>> Microsoft ABI.  The corresponding CodeGen already knows about these
>>>>>> implicit defaults, so the conversion kind could be just NoOp.
>>>>>>
>>>>>> The patch covers:
>>>>>> * pointers to free functions (__cdecl by default);
>>>>>> * references to free functions;
>>>>>> * pointers to static member functions and variadic member functions
>>>>>> (__cdecl by default);
>>>>>> * pointers to non-static non-variadic member functions (__thiscall by
>>>>>> default);
>>>>>> * simultaneous calling convention adjustment and base-to-derived
>>>>>> implicit conversion for member functions.
>>>>>>
>>>>>> The patch does not cover function templates, although the approach to
>>>>>> this is exactly the same as for PR15291.
>>>>>>
>>>>>> I suppose functions for determining calling convention compatibility
>>>>>> could be moved somewhere in TargetCXXABI if needed in other places.
>>>>>>
>>>>>> Please, review!
>>>>>> Suggestions are welcome.
>>>>>>
>>>>>
>>>>> This seems rather more brute force than is required. Could you instead
>>>>> transform CC_Default to Context.getTargetInfo().getDefaultCallingConv(...)
>>>>> in the relevant places?
>>>>>
>>>>
>>>>
>>>
>>
>> _______________________________________________
>> cfe-commits mailing list
>> cfe-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130710/b58e12c2/attachment.html>