Skip to content

Conversation

@kant2002
Copy link
Contributor

These tests added to better test ICustomMarshaller support in NativeAOT as per discussed with @jkotas

These tests added to better test ICustomMarshaller support in NativeAOT as per discussed with @jkotas
@ghost ghost added community-contribution Indicates that the PR has been added by a community member area-Interop-coreclr and removed community-contribution Indicates that the PR has been added by a community member labels Dec 12, 2021
Copy link
Member

@jkoritzinsky jkoritzinsky left a 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.
@kant2002
Copy link
Contributor Author

@jkoritzinsky anything else needed for this PR?

@jkoritzinsky
Copy link
Member

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)
Copy link
Member

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.

Copy link
Contributor Author

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.

@ghost ghost added needs-author-action An issue or pull request that requires more info or actions from the author. and removed needs-author-action An issue or pull request that requires more info or actions from the author. labels Dec 14, 2021
@kant2002
Copy link
Contributor Author

@jkoritzinsky can you merge now? I would like to continue work on ICustomMarshaller for NativeAOT

@jkoritzinsky jkoritzinsky merged commit a9bc1b7 into dotnet:main Dec 15, 2021
@kant2002 kant2002 deleted the kant/icustommarhsller-tests branch December 15, 2021 08:53
@ghost ghost locked as resolved and limited conversation to collaborators Jan 14, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants