<html><head></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><br><div><div>On Aug 29, 2013, at 5:30 PM, Reid Kleckner wrote:</div><br class="Apple-interchange-newline"><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><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 class="h5"><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></body></html>