[cfe-dev] Handling ignored calling conventions

[Aaron Ballman]

Ping?

On Tue, Sep 25, 2012 at 3:54 PM, Aaron Ballman <aaron at aaronballman.com> wrote:
> On Tue, Sep 25, 2012 at 3:47 AM, John McCall <rjmccall at apple.com> wrote:
>> On Sep 14, 2012, at 2:11 PM, Aaron Ballman wrote:
>>> Here's the latest iteration of the patch; this time it pushes decision
>>> making down into the TargetInfo subclasses to decide whether a given
>>> calling convention is acceptable, and what the default calling
>>> convention should be.
>>>
>>> This patch also updates a lot of the test cases and clarifies the
>>> target architecture for some of them (to ensure we're getting correct
>>> test coverage).
>>
>> Sorry for the delay.  A couple points.
>
> Thanks for the review!
>
>> First, making Basic depend on an AST header isn't okay;  you need
>> to pull the enum into Basic, probably into Specifiers.h.
>
> This was my biggest concern as well, but I wasn't certain where they
> should live.  Specifiers.h makes perfect sense.
>
>> Second, the caps convention for method names isLikeThis. It's not
>> universally followed in clang because we don't like massive restyling
>> commits, but please don't introduce more counter-convention names.
>
> Simple slip of the fingers; good catch.
>
>> Third, the boolean value for AcceptsCallConv is backwards.
>> That's okay given that you want to extend AcceptsCallConv later,
>> but it suggests that you shouldn't name your method like a
>> predicate, because I would totally be tempted to do something like:
>>   if (Target.acceptsCallingConv(...)) {
>
> That's a good point.  I've renamed the method to testCallingConvention
> which should be a bit more clear.  I've kept AcceptsCallConv as the
> enumeration name.
>
>> Fourth, the default for a target should not be to accept all CCs.
>> It's okay to expect targets to implement this function if they want
>> to support variant CCs.
>>
>> Finally, I claim that CC_C should be accepted by all targets.
>
> Done and done.
>
> I've attached the fixed patch; hopefully it addresses everything!  Thanks!
>
> ~Aaron