Add support for GeneratedComClassAttribute on WinRT classes #1858
Conversation
There was a problem hiding this comment.
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.
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:
WindowsRuntimeTypeAttributeto be considered a WinRT type, rather than all internal interfaces (which is the current behavior)