[PATCH] Bug 14984 - Implement __attribute__((ms_abi))

[Richard Smith]

"CallingConv& CC" should be "CallingConv &CC" throughout. I don't really
like that 'checkCallingConvention' modifies the calling convention, though.
I'd prefer that the ms_abi and sysv_abi attributes are instead mapped to
different calling conventions on different targets (so we never use
CC_X86_64SysV in cases where it is the same as CC_C).

[Ultimately, I think the problem is that CC_C is *not* a (specific) calling
convention; it's a symbolic name for whichever calling convention would be
used in C by default. But it may not be worth the trouble to fix that.]


On Thu, Aug 29, 2013 at 5:42 PM, Charles Davis <cdavis5x at gmail.com> wrote:

>
> On Aug 29, 2013, at 5:30 PM, Reid Kleckner wrote:
>
> Yep, LGTM.
>
> All right. Richard, any objections before I commit this?
>
> Chip
>
>
>
> On Thu, Aug 29, 2013 at 4:04 PM, Charles Davis <cdavis5x at gmail.com> wrote:
>
>>
>> On Aug 29, 2013, at 4:28 PM, Reid Kleckner wrote:
>>
>> On Thu, Aug 29, 2013 at 3:09 PM, Charles Davis <cdavis5x at gmail.com>wrote:
>>
>>>
>>> On Aug 29, 2013, at 11:32 AM, Reid Kleckner wrote:
>>>
>>> I think the lesson of CC_Default vs. CC_C is that there are *far* too
>>> many places to check for calling convention compatibility, and that it's
>>> much better to use a single representation for equivalent conventions.
>>>
>>> Therefore, I'd remove areCCsCompatible() and let
>>> TargetInfo::checkCallingConvention() adjust the the sysv or MS CCs to CC_C
>>> when appropriate.
>>>
>>> This way, when you target x64 Windows, the ms_abi attr will show up in
>>> the AST, but it will produce function types that are nicely canonically
>>> equivalent to normal function types.  Similarly, sysv_abi will be a no-op
>>> on sysv targets.
>>>
>>> You mean, like this?
>>>
>>> There's one side effect of your suggested change that still bugs me,
>>> though. Now, if we're on a target that uses the SysV calling convention,
>>> the diagnostics always say "cdecl", even if the function was explicitly
>>> declared "sysv_abi". (Same with Windows and "ms_abi".) This might be
>>> slightly confusing to your average user, but for obvious reasons I don't
>>> want to add special cases to the places these diagnostics get printed.
>>>
>>
>> I think the right way to handle this is to build AttributedType nodes for
>> the no-op attribute, but let equivalent and modified types be the same.
>>  The type printer should use __attribute__((sysv_abi/ms_abi)).  Looking at
>> SemaType, it's not clear if this happens.
>>
>> Can you write a test for the bad case with a FIXME maybe?
>>
>> There already is one (Clang :: Sema/ms_abi-sysv_abi.c). I'll add a FIXME
>> there.
>>
>> Otherwise, OK to commit?
>>
>> Chip
>>
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130829/8c49927e/attachment.html>