Yep, LGTM.
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
