[PATCH] Emit diagnostic on illegal calling convention

[Alexander Zinenko]

Hello John!

Thanks for the review, I have to provide some explanation (see below).
Which changes do you think are worth making then?

On 17 April 2013 03:12, John McCall <rjmccall at apple.com> wrote:
>
>
> This is the right general idea, thanks!
>
> +  int ChunkIndex = state.getCurrentChunkIndex();
> +  while (ChunkIndex != 0 &&
> +         D.getTypeObject(ChunkIndex).Kind == DeclaratorChunk::Paren)
> +    ChunkIndex--;
>
> Isn't the current ChunkIndex always a DeclaratorChunk::Function?

Nope, it can be DeclaratorChunk::Paren for typedefs, for example:
typedef (__stdcall *ptr)();
will have pointer, paren and function chunks and the checking is called on paren
or for variable declarations, for example
void (*__thiscall foo)();
and the checking is called on pointer.

> I think you want D.getTypeObject(ChunkIndex - 1) here, although I guess
> I might be off-by-one somehow.

if (D.getTypeObject(ChunkIndex).Kind == DeclaratorChunk::Paren)
  --ChunkIndex;
works for all my tests and frankly I can't figure out a declarator
with two consecutive paren chunks. However, most places with similar
analysis in SemaType allow skipping multiple consecutive paren chunks.


>
>
> +  bool IsCXXMethod =
> +      D.getTypeObject(ChunkIndex).Kind == DeclaratorChunk::MemberPointer;
> +
> +  // Non-friend functions declared in member context are methods.
> +  IsCXXMethod = IsCXXMethod || (D.getContext() == Declarator::MemberContext
> +                                && D.isFunctionDeclarator()
> +                                && !D.getDeclSpec().isFriendSpecified());
>
> I think the right thing to do here is more like:
>   bool IsCXXMethod;
>   if (ChunkIndex == 0) { // CC applies to the function being declared
>     assert(D.isFunctionDeclarator());

This is not necessarily a function declarator, consider for example
variable decl
void (*__fastcall fastpfunc)();
chunks are: pointer, paren, function and the checking is called for
the the first one.


>     IsCXXMethod = (D.getContext() == Declarator::MemberContext && !D.getDeclSpec().isFriendSpecified() && D.getDeclSpec().getStorageClassSpecifier() != SC_Static);
>   } else {
>     IsCXXMethod = D.getTypeObject(ChunkIndex - 1).Kind == DeclaratorChunk::MemberPointer;
>   }
>
> For out-of-line method declarations, we should just be able rely on redeclaration checking.

It looks like at this point we don't have redeclaration info yet.
Types are computed before looking up previous declaration in
HandleDeclarator.
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.

--
Alex