awslabs / aws-crt-dotnet

.NET bindings for the AWS Common Runtime
Apache License 2.0
10 stars 2 forks source link

MDA exception resolution plus cleanup #72

Closed bretambrose closed 3 years ago

bretambrose commented 3 years ago

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

justinboswell commented 3 years ago

This is fine, though cumbersome on future authors. Was there no way to add the calling convention at runtime in the NativeAPI binding instead?

Ok, based on some research mucking about in the .NET source and looking around, this is possible, but has to be done via reflection, so it's non-trivial. Maybe give me a follow-up ticket and I'll take a stab at some point. I would prefer not to require an attribute everywhere because people WILL mess it up, but for now, this totally works.

Actually, to that end, could you add an assert that looks for the attribute on the Delegate and if it doesn't find it, dies? That's straightforward to do.

For reference: https://stackoverflow.com/questions/14663763/how-to-add-an-attribute-to-a-property-at-runtime

bretambrose commented 3 years ago

This is fine, though cumbersome on future authors. Was there no way to add the calling convention at runtime in the NativeAPI binding instead?

I was thinking of the opposite. I was thinking we extend this pattern fully and get rid of the DOTNET_CALL macro required for native->managed which is genuinely fatal if you get it wrong (crash hard) and require all of our pinvoke delegates to be cdecl annotated.