"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).
[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.] On Thu, Aug 29, 2013 at 5:42 PM, Charles Davis <[email protected]> wrote: > > 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
