[cfe-dev] Handling ignored calling conventions

[Aaron Ballman]

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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: callconv.patch
Type: application/octet-stream
Size: 15492 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20120925/8129fbce/attachment.obj>