Skip to content

Conversation

@Berrysoft
Copy link
Contributor

According to microsoft/win32metadata#917 and microsoft/win32metadata#33 , the win32 metadata doesn't recognize the varargs declarations, which gives much inconvenience. This PR adds simple dll-import support for varargs, and generate C# code with undocumented keyword __arglist.

Comment on lines +1925 to +1931
if (cursor is FunctionDecl functionDecl)
{
if (functionDecl.IsVariadic)
{
return CallingConvention.Cdecl;
}
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs to support remapping still. The default should definitely be Cdecl, but ClangSharp supports users doing any kind of remapping, even ones that technically may not make sense.

Copy link
Contributor Author

@Berrysoft Berrysoft Jun 8, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The document of __stdcall indicates that even the vararg function is marked as __stdcall, the compiler makes it a __cdecl. Therefore I force Cdecl here.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, but that's not necessarily true for any and all potential platforms. ClangSharp has reasonable defaults and allows devs to override the default to fit their own needs.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we are generating proper dll imports. If a programmer writes a method like

extern "C" int __stdcall foo(...);

The compiler won't raise errors. However, if we generate bindings like

[DllImport("foo.dll", CallingConvention = CallingConvention.StdCall)]
static extern int foo(__arglist);

The program will raise runtime System.TypeLoadException says that varargs function should use cdecl.

@tannergooding
Copy link
Member

Sorry for the delay in reviewing this. It generally looks correct to me but has a couple fixups that would be good to include.

There should ideally be some handling for a variadic function pointer as well. C# doesn't support them, but ClangSharp still needs to generate something (likely just a diagnostic indicating it couldn't be handled and falling back to void*).

When the param name is empty.

Actually the only possible case is when
type is __arglist.
@tannergooding tannergooding merged commit ae60be4 into dotnet:main Jul 15, 2022
@Berrysoft Berrysoft deleted the dev/varargs branch July 16, 2022 01:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants