[PATCH] Emit diagnostic on illegal calling convention

[John McCall]

On Mar 28, 2013, at 1:27 PM, Alexander Zinenko <ftynse at gmail.com> wrote:
> On 28 March 2013 20:07, John McCall <rjmccall at apple.com> wrote:
> On Mar 28, 2013, at 10:57 AM, Alexander Zinenko <ftynse at gmail.com> wrote:
>> On 23 March 2013 01:22, Alexander Zinenko <ftynse at gmail.com> wrote:
>> Hi John, João!
>> 
>> This patch adds a diagnostic if the following declarations have illegal calling conventions:
>> * pointer to function, reference to function, pointer to member function variable (or function attribute) declaration;
>> * function declaration;
>> * typedef declaration.
>> 
>> It was proposed in PR13457 discussion (http://llvm.org/bugs/show_bug.cgi?id=13457#c22).
>> 
>> We already have such error for variadic functions declared with CC_X86Fastcall that is emitted from SemaType.  But microsoft ABI has different allowed CCs depending on whether it is a member function and it can't be determined given FunctionType only (AFAIK, standard doesn't differentiate between free function and member function types).  So this checking has to be done on declarations.
>> 
>> With this patch, allowed calling conventions are ABI-specific. Therefore if we teach affected ABIs to disallow fastcall on variadic functions, we can remove the previous diagnostic.
>> 
>> Please review!
>> 
>> --
>> Alex
>> 
>> <callconv.patch>
> 
> I would only pass a flag distinguishing non-static member functions vs. everything else.
> 
> It makes perfect sense in ABI part, fixed.  
> But I left separate flags in the function that emits diagnostic since it allows to make it more precise.  Namely to distinguish between __thiscall used on non-member function and on static member function.  It is difficult to do otherwise while keeping the single diagnostic message because we would have to know the reason why check failed rather than just passed-or-not flag.
>  
> 
> +  if (T->isFunctionProtoType())
> +    ProtoType = T->getAs<FunctionProtoType>();
> +  else if (T->isFunctionPointerType())
> +    ProtoType = T->getPointeeType()->getAs<FunctionProtoType>();
> +  else if (T->isReferenceType()) {
> +    QualType RefType = T->getAs<ReferenceType>()->getPointeeType();
> +    ProtoType = RefType->getAs<FunctionProtoType>();
> +  } else if (T->isMemberFunctionPointerType()) {
> +    QualType PointerType = T->getAs<MemberPointerType>()->getPointeeType();
> 
> Test the result of getAs<>, please.
> Done.

Is there any reasonable way we can tie this to applying an explicit CC attribute instead of putting explicit checks in a ton of places?  It's not so much that I'm concerned about the compile-time impact of checking this in a bunch of places (although there is that).  It's that there's really no earthly way that the three or four places you've found to insert a check for this are really covering all the cases, and it seems unlikely that you'll reasonably be able to find them all.  (e.g., what if a pointer-to-function appears at an arbitrary position within some other type?)

John.

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130328/4b08505d/attachment.html>