-
Notifications
You must be signed in to change notification settings - Fork 5.2k
Add custom marshaller tests for delegates #62691
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add custom marshaller tests for delegates #62691
Conversation
These tests added to better test ICustomMarshaller support in NativeAOT as per discussed with @jkotas
jkoritzinsky
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for improving our test coverage!
Too much copy-pasta is bad for your health.
|
@jkoritzinsky anything else needed for this PR? |
|
Can you remove the places where you set CallingConvention? In native code, STDMETHODCALLTYPE is stdcall on Windows and cdecl on non-Windows, aka the DllImport default. |
| using TestDelegate = LPCSTR(STDMETHODCALLTYPE*)(int); | ||
| using TestDelegateRef = LPCSTR(STDMETHODCALLTYPE*)(int*); | ||
|
|
||
| extern "C" int DLL_EXPORT STDMETHODCALLTYPE NativeParseInt(LPCSTR str) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please keep the STDMETHODCALLTYPEs on the native methods without adding a CallingConvention= setter in managed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah. I reread again and understand that you want like that. Done.
|
@jkoritzinsky can you merge now? I would like to continue work on ICustomMarshaller for NativeAOT |
These tests added to better test ICustomMarshaller support in NativeAOT as per discussed with @jkotas