[PATCH] Emit diagnostic on illegal calling convention

[John McCall]

On Apr 17, 2013, at 3:36 PM, Alexander Zinenko <ftynse at gmail.com> wrote:
> On 17 April 2013 21:41, John McCall <rjmccall at apple.com> wrote:
>> 
>> Okay, so we don't actually move the CC attribute effectively.  Good to know.
>> 
>> 
>> You wouldn't normally write it, but this is totally legal:
>>  typedef ((((__stdcall (*((ptr)))))()));
>> 
> 
> Thanks, added as an extra test case.
> 
>> 
>> Yeah, this is a consequence of my misunderstanding about moving the
>> attribute.
>> 
>> Okay, so here's what you should do:
>>  - If we're on a function declarator chunk right now, walk "inwards".
>>  - Keep walking inwards past an arbitrary number of parens.
>>  - If you run out of declarator chunks, this is a function declarator, and you should check the declaring context, whether this is a friend, whether it's static, etc.
>>  - Otherwise, if it's a member-pointer chunk, this is an instance method context.
>>  - Otherwise, it's a "normal" function pointer context.
>> 
> 
> Done.
> 
>> 
>> Yes, I know.  What I mean is that we can get away with just checking for explicit storage-class specifiers, friend specifiers, etc. because, if this is an out-of-line method definition, we won't be able to accept an illegal CC anyway because it won't match the CC from the original declaration.
>> 
> 
> Sure. And ignore the specifiers if this is not a function declaration,
> but something else, for example function pointer variable which is
> declared static.
> 
>>> On the other hand, friend declarations will not be handled correctly
>>> this way since we can no longer differentiate between friend free
>>> functions and friend methods from other classes. And we have no
>>> diagnostic if friend declaration uses different CC than original
>>> declaration.
>> 
>> Really?  That's a bug.
> 
> Actually not. Sorry for the noise, this patch set the attribute as
> invalid so that CC mismatch diagnostic ignored it.


+    // We can rely on immediate DeclSpec since illegal CCs on redeclarations
+    // won't be accepted due to mismatch with the original declaration CC.
+    IsStatic = (D.getDeclSpec().getStorageClassSpec() == DeclSpec::SCS_static);

+  if (!S.Context.isCallConvAllowed(IsVariadic, IsCXXMethod && !IsStatic, CC)) {

Out-of-line declarations can't be declared with 'static';  they just inherit that from the primary declaration.  So trying to reason about out-of-line redeclarations seems like a waste of time to me.  I would just suppress all this checking whenever the declaration has a scope qualifier and let redeclaration checking handle it.

John.