-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
Proposal: allow exception marshallers via '[return: MarshalUsing]' on generated COM interfaces methods without '[PreserveSig]' #109522
Comments
Tagging subscribers to this area: @dotnet/interop-contrib |
What does this look like for methods with return value? |
My understanding is there would be no difference, because at the ABI level, the return is always an HRESULT anyway. So the generated ABI method will still always return an Eg. example codegen for: [Guid("6234C2F7-9917-469F-BDB4-3E8C630598AF")]
[GeneratedComInterface(Options = ComInterfaceOptions.ManagedObjectWrapper)]
public partial interface IFoo
{
double Bar();
} You get: file unsafe partial interface InterfaceImplementation
{
[global::System.Runtime.InteropServices.UnmanagedCallersOnlyAttribute(CallConvs = new[] { typeof(global::System.Runtime.CompilerServices.CallConvMemberFunction) })]
internal static int ABI_Bar(global::System.Runtime.InteropServices.ComWrappers.ComInterfaceDispatch* __this_native, double* __invokeRetValUnmanaged__param)
{
global::IFoo @this = default;
ref double __invokeRetValUnmanaged = ref *__invokeRetValUnmanaged__param;
double __invokeRetVal = default;
int __retVal = default;
try
{
// Unmarshal - Convert native data to managed data.
@this = global::System.Runtime.InteropServices.ComWrappers.ComInterfaceDispatch.GetInstance<global::IFoo>(__this_native);
__invokeRetVal = @this.Bar();
// NotifyForSuccessfulInvoke - Keep alive any managed objects that need to stay alive across the call.
__retVal = 0; // S_OK
// Marshal - Convert managed data to native data.
__invokeRetValUnmanaged = __invokeRetVal;
}
catch (global::System.Exception __exception)
{
__retVal = global::System.Runtime.InteropServices.Marshalling.ExceptionAsHResultMarshaller<int>.ConvertToUnmanaged(__exception);
}
return __retVal;
}
} We want to override that __retVal = global::System.Runtime.InteropServices.Marshalling.ExceptionAsHResultMarshaller<int>.ConvertToUnmanaged(__exception); |
What about properties that return strings or other types that need marshalling? I can write this today:
If I want to override the exception marshaling too, do I specify two |
I see what you mean. Right, that syntax was meant to be an example, but the real thing would likely need some other attribute targeting methods that would allow to indicate an exception marshaller. I just don't know which exact API surface it would require. If we want to brainstorm what an actual API proposal to bring to review would look like, I'd imagine a starting point could maybe be: namespace System.Runtime.InteropServices.Marshalling
{
[AttributeUsage(AttributeTargets.Method, AllowMultiple = true)]
public sealed class ExceptionMarshallerAttribute : Attribute
{
public ExceptionMarshallerAttribute(MarshalMode marshalMode, Type marshallerType);
public MarshalMode MarshalMode { get; }
public Type MarshallerType { get; }
}
} And then you'd use it as follows? public partial interface IFoo
{
[ExceptionMarshaller(MarshalMode.ManagedToUnmanagedIn, typeof(ExceptionHelpersHResultMarshaller<int>))]
void Bar();
} cc. @jkoritzinsky for thoughts, since you also mentioned we'd need a new API for this likely 🙂 EDIT: not entirely sure if we need namespace System.Runtime.InteropServices.Marshalling
{
[AttributeUsage(AttributeTargets.Method, AllowMultiple = true)]
public sealed class ExceptionMarshallerAttribute : Attribute
{
public ExceptionMarshallerAttribute(Type marshallerType);
public Type MarshallerType { get; }
}
} And then you just do: public partial interface IFoo
{
[ExceptionMarshaller(typeof(ExceptionHelpersHResultMarshaller<int>))]
void Bar();
} |
I believe we should have a default exception handler for |
We've talked about this on the team and here's the API shape we thought of: class GeneratedComInterfaceAttribute
{
+ public Type? ExceptionToUnmanagedMarshaller { get; set; }
} This marshaller would be used to marshal an Exception to the unmanaged return value. We want to put it on the interface as a whole as it's very likely that a given interface wants to handle errors consistently, and we feel that COM interfaces that use A marshaller type that is assigned to If we feel that the non-
Source-Generated COM uses the built-in |
Should be |
Yep, updating now. |
This looks good to me, thank you for taking a look! We can add our own marshaller type for this for WinRT scenarios in CsWinRT, and then direct people to using that. We could even add an analyzer to warn if someone uses a generated COM interface on a WinRT type without specifying this marshaller, to help with discoverability. Basically the scenario from this issue would be like: [Guid("6234C2F7-9917-469F-BDB4-3E8C630598AF")]
[GeneratedComInterface(
Options = ComInterfaceOptions.ManagedObjectWrapper,
ExceptionToUnmanagedMarshaller = typeof(ExceptionHelpersHResultMarshaller))]
public partial interface IFoo
{
void Bar();
} Where @jkoritzinsky what else are we missing to make this proposal ready for review? 🙂 |
I think we just need to update the top post into the API review format and then we can mark this as
api-ready-for-review
|
Stakeholder sign off :) |
Thank you! I've updated the first post with the updated API proposal 😄 |
namespace System.Runtime.InteropServices.Marshalling;
public partial class GeneratedComInterfaceAttribute
{
public Type? ExceptionToUnmanagedMarshaller { get; set; }
} |
Overview
This came up in microsoft/CsWinRT#1858. We'd like for the COM generator to support
[return: MarshalUsing]
to use custom marshallers for exceptions, so that CsWinRT could correctly allow using the necessary error propagation logic from generated COM interfaces. Currently, trying the code below doesn't do anything (but also it doesn't emit any warnings, it just silently does nothing).API proposal
class GeneratedComInterfaceAttribute { + public Type? ExceptionToUnmanagedMarshaller { get; set; } }
Use case example
Users will be able to implement COM interfaces like this, implemented on WinRT-exposed types:
With
ExceptionHelpersHResultMarshaller
being a WinRT-compatible marshaller type we'll include in CsWinRT.Risks
None that I can think of.
Alternatives
Developers can work around this by manually writing the entire vtable backend (eg. here).
This is extremely clunky and error prone.
The text was updated successfully, but these errors were encountered: