<div dir="ltr">"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).<div>
<br></div><div>[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.]</div>
</div><div class="gmail_extra"><br><br><div class="gmail_quote">On Thu, Aug 29, 2013 at 5:42 PM, Charles Davis <span dir="ltr"><<a href="mailto:cdavis5x@gmail.com" target="_blank">cdavis5x@gmail.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word"><br><div><div>On Aug 29, 2013, at 5:30 PM, Reid Kleckner wrote:</div><br><blockquote type="cite">
<div dir="ltr"><div>Yep, LGTM.<br></div></div></blockquote>All right. Richard, any objections before I commit this?</div><div><br></div><div>Chip</div><div><div class="h5"><div><br><blockquote type="cite"><div class="gmail_extra">
<br><br><div class="gmail_quote">On Thu, Aug 29, 2013 at 4:04 PM, Charles Davis <span dir="ltr"><<a href="mailto:cdavis5x@gmail.com" target="_blank">cdavis5x@gmail.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word"><div><div><br><div><div>On Aug 29, 2013, at 4:28 PM, Reid Kleckner wrote:</div>
<br><blockquote type="cite"><div dir="ltr">On Thu, Aug 29, 2013 at 3:09 PM, Charles Davis <span dir="ltr"><<a href="mailto:cdavis5x@gmail.com" target="_blank">cdavis5x@gmail.com</a>></span> wrote:<br><div class="gmail_extra">

<div class="gmail_quote">
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word"><br><div><div><div>On Aug 29, 2013, at 11:32 AM, Reid Kleckner wrote:</div>
<br><blockquote type="cite"><div dir="ltr">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.<div>



<br></div><div>Therefore, I'd remove areCCsCompatible() and let TargetInfo::checkCallingConvention() adjust the the sysv or MS CCs to CC_C when appropriate.</div><div><br></div><div>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.</div>


</div></blockquote></div>You mean, like this?</div><div><br></div><div>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.</div>


</div></blockquote><div><br></div><div>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.</div>


<div><br>Can you write a test for the bad case with a FIXME maybe?</div></div></div></div>
</blockquote></div></div></div>There already is one (Clang :: Sema/ms_abi-sysv_abi.c). I'll add a FIXME there.<div><br></div><div>Otherwise, OK to commit?<br><div><br></div><div>Chip</div><div><br></div></div></div></blockquote>

</div><br></div>
</blockquote></div><br></div></div></div></blockquote></div><br></div>