-
Notifications
You must be signed in to change notification settings - Fork 107
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 support for GeneratedComClassAttribute on WinRT classes #1858
base: master
Are you sure you want to change the base?
Conversation
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.
There is one problem with this approach. The generated stubs will use:
try
{
// Code...
}
catch (global::System.Exception __exception)
{
ExceptionAsHResultMarshaller<int>.ConvertToUnmanaged(__exception);
}
But that will lose stacktrace info. We need:
try
{
// Code...
}
catch (global::System.Exception __exception)
{
ExceptionHelpers.SetErrorInfo(__exception);
return ExceptionHelpers.GetHRForException(__exception);
}
Is there a way for users to annotate either the interface, or methods, or something, so the generated code can use the appropriate marshaller for this? We likely also need to actually implement this marshaller in WinRT.Runtime, so people can actually use it.
Without support for this, I feel like this isn't really usable yet.
cc. @manodasanW thoughts?
Something like this? [CustomMarshaller(typeof(Exception), MarshalMode.UnmanagedToManagedOut, typeof(ExceptionHelpersHResultMarshaller<>))]
public static class ExceptionHelpersHResultMarshaller<T>
where T : unmanaged, INumber<T>
{
public static T ConvertToUnmanaged(Exception e)
{
ExceptionHelpers.SetErrorInfo(e);
return T.CreateTruncating(ExceptionHelpers.GetHRForException(e));
}
} And then you should be able to use it like this maybe? idk: [Guid("6234C2F7-9917-469F-BDB4-3E8C630598AF")]
[GeneratedComInterface(Options = ComInterfaceOptions.ManagedObjectWrapper)]
public partial interface IFoo
{
[return: MarshalUsing(typeof(ExceptionHelpersHResultMarshaller<int>))]
void Bar();
} |
Added Jeremy for his thoughts on this. |
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.
I think this looks great and I love the integration with the modern source-generated COM experiences!
Sergio's comments about the exception experience are warranted. I think there should be a manual validation pass that the COM-style interfaces on WinRT types are using COM-style error handling, not WinRT-style handling, just to make sure there's a decent experience there.
What makes me a bit sad is that until we support this, eg. I won't be able to migrate interfaces like this to this 🥲 I'll go open an API proposal in the runtime repo for tracking. |
Do we know whether these COM exceptions should be propagated through |
Fixes #1722.
Fixes #1851.
This PR includes two changes:
WindowsRuntimeTypeAttribute
to be considered a WinRT type, rather than all internal interfaces (which is the current behavior)