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 <[email protected]> wrote:
> 
> On Aug 29, 2013, at 4:28 PM, Reid Kleckner wrote:
> 
>> On Thu, Aug 29, 2013 at 3:09 PM, Charles Davis <[email protected]> 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
> 
> 

_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to