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

[Charles Davis]

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/c4a2d481/attachment.html>