Implement unmanaged-to-managed direction for vtable stub generator

docs/design/libraries/ComInterfaceGenerator/UnmanagedToManagedEH.md

@@ -0,0 +1,59 @@
+# Unmanaged to Managed Exception Handling
+
+As part of building out the "vtable method stub" generator as the first steps on our path to the COM source generator, we need to determine how to handle exceptions at the managed code boundary in the unmanaged-calling-managed direction. When implementing a COM Callable Wrapper (or equivalent concept), the source generator will need to generate an `[UnmanagedCallersOnly]` wrapper of a method. We do not support propagating exceptions across an `[UnmanagedCallersOnly]` method on all platforms, so we need to determine how to handle exceptions.
+
+We determined that there were three options:
+
+1. Do nothing
+2. Match the existing behavior in the runtime for `[PreserveSig]` on a method on a `[ComImport]` interface.
+3. Allow the user to provide their own "exception -> return value" translation.
+
+We chose option 3 for a few reasons:
+
+1. Ecosystems like Swift don't use COM HResults as result types, so they may want different behavior for simple types like `int`.
+2. Not providing a customization option for exception handling would prevent dropping a single method down from the COM generator to the VTable generator to replicate the old `[PreserveSig]` behavior, and there's a desire to not support the `[PreserveSig]` attribute as it exists today.
+
+As a result, we decided to provide their own rules while providing an easy opt-in to the COM-style rules.
+
+## Option 1: Do Nothing
+
+This option seems the simplest option. The source generator will not emit a `try-catch` block and will just let the exception propogate when in a wrapper for a `VirtualMethodTableIndex` stub. There's a big problem with this path though. One goal of our COM source generator design is to let developers drop down a particular method from the "HRESULT-swapped, all nice COM defaults" mechanism to a `VirtualMethodTableIndex`-attributed method for particular methods when they do not match the expected default behaviors. This feature becomes untenable if we do not wrap the stub in a `try-catch`, as suddenly there is no mechanism to emulate the exception handling of exceptions thrown by marshallers. For cases where all marshallers are known to not throw exceptions except in cases where the process is unrecoverable (for example, `OutOfMemoryException`), this version may provide a slight performance improvement as there will be no exception handling in the generated stub.
+
+## Option 2: Match the existing behavior in the runtime for `[PreserveSig]` on a method on a `[ComImport]` interface
+
+The runtime has some existing built-in behavior for generating a return value from an `Exception` in an unmanaged-to-managed `COM` stub. The behavior depends on the return type, and matches the following table:
+
+| Type            | Return Value in Exceptional Scenario |
+|-----------------|--------------------------------------|
+| `void`          | Swallow the exception                |
+| `int`           | `exception.HResult`                  |
+| `uint`          | `(uint)exception.HResult`            |
+| `float`         | `float.NaN`                          |
+| `double`        | `double.NaN`                         |
+| All other types | `default`                            |
+
+We could match this behavior for all `VirtualMethodTableIndex`-attributed method stubs, but that would be forcibly encoding the rules of HResults and COM for our return types, and since COM is a Windows-centric technology, we don't want to lock users into this model in case they have different error code models that they want to integrate with. However, it does provide an exception handling boundary
+
+## Option 3: Allow the user to provide their own "exception -> return value" translation
+
+Another option would be to give the user control of how to handle the exception marshalling, with an option for some nice defaults to replicate the other options with an easy opt-in. We would provide the following enumeration and new members on `VirtualMethodTableIndexAttribute`:
+
+```diff
+namespace System.Runtime.InteropServices.Marshalling;
+
+public class VirtualMethodTableIndexAttribute
+{
++     public ExceptionMarshalling ExceptionMarshalling { get; }
++     public Type? ExceptionMarshallingType { get; } // When set to null or not set, equivalent to option 1
+}
+
++ public enum ExceptionMarshalling
++ {
++     Custom,
++     Com, // Match the COM-focused model described in Option 2
++ }
+```
+
+When the user sets `ExceptionMarshalling = ExceptionMarshalling.Custom`, they must specify a marshaller type in `ExceptionMarshallingType` that unmarshals a `System.Exception` to the same unmanaged return type as the marshaller of the return type in the method's signature.
+
+To implement the `ExceptionMarshalling.Com` option, we will provide some marshallers to implement the different rules above, and select the correct rule given the unmanaged return type from the return type's marshaller. By basing the decision on the unmanaged type instead of the managed type, this mechanism will be able to kick in correctly for projects that use their own custom `HResult` struct that wraps an `int` or `uint`, as the `HResult` will be marshalled to an `int` or `uint` if it is well-defined.

src/libraries/System.Runtime.InteropServices.JavaScript/gen/JSImportGenerator/JSExportCodeGenerator.cs

@@ -28,22 +28,29 @@ public JSExportCodeGenerator(
             Action<TypePositionInfo, MarshallingNotSupportedException> marshallingNotSupportedCallback,
             IMarshallingGeneratorFactory generatorFactory)
         {
+            Action<TypePositionInfo, string> extendedInvariantViolationsCallback = (info, details) =>
+                marshallingNotSupportedCallback(info, new MarshallingNotSupportedException(info, _context) { NotSupportedDetails = details });
             _signatureContext = signatureContext;
-            ManagedToNativeStubCodeContext innerContext = new ManagedToNativeStubCodeContext(targetFramework, targetFrameworkVersion, ReturnIdentifier, ReturnIdentifier);
+            NativeToManagedStubCodeContext innerContext = new NativeToManagedStubCodeContext(targetFramework, targetFrameworkVersion, ReturnIdentifier, ReturnIdentifier);
             _context = new JSExportCodeContext(attributeData, innerContext);
-            _marshallers = new BoundGenerators(argTypes, CreateGenerator);
+
+            _marshallers = BoundGenerators.Create(argTypes, generatorFactory, _context, new EmptyJSGenerator(), out var bindingFailures);
+            foreach (var failure in bindingFailures)
+            {
+                marshallingNotSupportedCallback(failure.Info, failure.Exception);
+            }
+
             if (_marshallers.ManagedReturnMarshaller.Generator.UsesNativeIdentifier(_marshallers.ManagedReturnMarshaller.TypeInfo, null))
             {
                 // If we need a different native return identifier, then recreate the context with the correct identifier before we generate any code.
-                innerContext = new ManagedToNativeStubCodeContext(targetFramework, targetFrameworkVersion, ReturnIdentifier, ReturnNativeIdentifier);
+                innerContext = new NativeToManagedStubCodeContext(targetFramework, targetFrameworkVersion, ReturnIdentifier, ReturnNativeIdentifier);
                 _context = new JSExportCodeContext(attributeData, innerContext);
-                _marshallers = new BoundGenerators(argTypes, CreateGenerator);
             }
 
             // validate task + span mix
             if (_marshallers.ManagedReturnMarshaller.TypeInfo.MarshallingAttributeInfo is JSMarshallingInfo(_, JSTaskTypeInfo))
             {
-                BoundGenerator spanArg = _marshallers.AllMarshallers.FirstOrDefault(m => m.TypeInfo.MarshallingAttributeInfo is JSMarshallingInfo(_, JSSpanTypeInfo));
+                BoundGenerator spanArg = _marshallers.SignatureMarshallers.FirstOrDefault(m => m.TypeInfo.MarshallingAttributeInfo is JSMarshallingInfo(_, JSSpanTypeInfo));
                 if (spanArg != default)
                 {
                     marshallingNotSupportedCallback(spanArg.TypeInfo, new MarshallingNotSupportedException(spanArg.TypeInfo, _context)
@@ -52,27 +59,14 @@ public JSExportCodeGenerator(
                     });
                 }
             }
-
-            IMarshallingGenerator CreateGenerator(TypePositionInfo p)
-            {
-                try
-                {
-                    return generatorFactory.Create(p, _context);
-                }
-                catch (MarshallingNotSupportedException e)
-                {
-                    marshallingNotSupportedCallback(p, e);
-                    return new EmptyJSGenerator();
-                }
-            }
         }
 
         public BlockSyntax GenerateJSExportBody()
         {
             StatementSyntax invoke = InvokeSyntax();
             GeneratedStatements statements = GeneratedStatements.Create(_marshallers, _context);
             bool shouldInitializeVariables = !statements.GuaranteedUnmarshal.IsEmpty || !statements.Cleanup.IsEmpty;
-            VariableDeclarations declarations = VariableDeclarations.GenerateDeclarationsForManagedToNative(_marshallers, _context, shouldInitializeVariables);
+            VariableDeclarations declarations = VariableDeclarations.GenerateDeclarationsForUnmanagedToManaged(_marshallers, _context, shouldInitializeVariables);
 
             var setupStatements = new List<StatementSyntax>();
             SetupSyntax(setupStatements);
@@ -222,7 +216,7 @@ private StatementSyntax InvokeSyntax()
 
         public (ParameterListSyntax ParameterList, TypeSyntax ReturnType, AttributeListSyntax? ReturnTypeAttributes) GenerateTargetMethodSignatureData()
         {
-            return _marshallers.GenerateTargetMethodSignatureData();
+            return _marshallers.GenerateTargetMethodSignatureData(_context);
         }
     }
 }

src/libraries/System.Runtime.InteropServices.JavaScript/gen/JSImportGenerator/JSImportCodeGenerator.cs

@@ -35,22 +35,28 @@ public JSImportCodeGenerator(
             Action<TypePositionInfo, MarshallingNotSupportedException> marshallingNotSupportedCallback,
             IMarshallingGeneratorFactory generatorFactory)
         {
+            Action<TypePositionInfo, string> extendedInvariantViolationsCallback = (info, details) =>
+                marshallingNotSupportedCallback(info, new MarshallingNotSupportedException(info, _context) { NotSupportedDetails = details });
             _signatureContext = signatureContext;
             ManagedToNativeStubCodeContext innerContext = new ManagedToNativeStubCodeContext(targetFramework, targetFrameworkVersion, ReturnIdentifier, ReturnIdentifier);
             _context = new JSImportCodeContext(attributeData, innerContext);
-            _marshallers = new BoundGenerators(argTypes, CreateGenerator);
+            _marshallers = BoundGenerators.Create(argTypes, generatorFactory, _context, new EmptyJSGenerator(), out var bindingFailures);
+
+            foreach (var failure in bindingFailures)
+            {
+                marshallingNotSupportedCallback(failure.Info, failure.Exception);
+            }
             if (_marshallers.ManagedReturnMarshaller.Generator.UsesNativeIdentifier(_marshallers.ManagedReturnMarshaller.TypeInfo, null))
             {
                 // If we need a different native return identifier, then recreate the context with the correct identifier before we generate any code.
                 innerContext = new ManagedToNativeStubCodeContext(targetFramework, targetFrameworkVersion, ReturnIdentifier, ReturnNativeIdentifier);
                 _context = new JSImportCodeContext(attributeData, innerContext);
-                _marshallers = new BoundGenerators(argTypes, CreateGenerator);
             }
 
             // validate task + span mix
             if (_marshallers.ManagedReturnMarshaller.TypeInfo.MarshallingAttributeInfo is JSMarshallingInfo(_, JSTaskTypeInfo))
             {
-                BoundGenerator spanArg = _marshallers.AllMarshallers.FirstOrDefault(m => m.TypeInfo.MarshallingAttributeInfo is JSMarshallingInfo(_, JSSpanTypeInfo));
+                BoundGenerator spanArg = _marshallers.SignatureMarshallers.FirstOrDefault(m => m.TypeInfo.MarshallingAttributeInfo is JSMarshallingInfo(_, JSSpanTypeInfo));
                 if (spanArg != default)
                 {
                     marshallingNotSupportedCallback(spanArg.TypeInfo, new MarshallingNotSupportedException(spanArg.TypeInfo, _context)
@@ -59,28 +65,14 @@ public JSImportCodeGenerator(
                     });
                 }
             }
-
-
-            IMarshallingGenerator CreateGenerator(TypePositionInfo p)
-            {
-                try
-                {
-                    return generatorFactory.Create(p, _context);
-                }
-                catch (MarshallingNotSupportedException e)
-                {
-                    marshallingNotSupportedCallback(p, e);
-                    return new EmptyJSGenerator();
-                }
-            }
         }
 
         public BlockSyntax GenerateJSImportBody()
         {
             StatementSyntax invoke = InvokeSyntax();
             GeneratedStatements statements = GeneratedStatements.Create(_marshallers, _context);
             bool shouldInitializeVariables = !statements.GuaranteedUnmarshal.IsEmpty || !statements.Cleanup.IsEmpty;
-            VariableDeclarations declarations = VariableDeclarations.GenerateDeclarationsForManagedToNative(_marshallers, _context, shouldInitializeVariables);
+            VariableDeclarations declarations = VariableDeclarations.GenerateDeclarationsForManagedToUnmanaged(_marshallers, _context, shouldInitializeVariables);
 
             var setupStatements = new List<StatementSyntax>();
             BindSyntax(setupStatements);
@@ -209,7 +201,7 @@ private StatementSyntax InvokeSyntax()
 
         public (ParameterListSyntax ParameterList, TypeSyntax ReturnType, AttributeListSyntax? ReturnTypeAttributes) GenerateTargetMethodSignatureData()
         {
-            return _marshallers.GenerateTargetMethodSignatureData();
+            return _marshallers.GenerateTargetMethodSignatureData(_context);
         }
     }
 }

src/libraries/System.Runtime.InteropServices.JavaScript/gen/JSImportGenerator/Marshaling/BaseJSGenerator.cs

@@ -22,7 +22,6 @@ protected BaseJSGenerator(MarshalerType marshalerType, IMarshallingGenerator inn
         }
 
         public ManagedTypeInfo AsNativeType(TypePositionInfo info) => _inner.AsNativeType(info);
-        public ParameterSyntax AsParameter(TypePositionInfo info) => _inner.AsParameter(info);
         public bool IsSupported(TargetFramework target, Version version) => _inner.IsSupported(target, version);
         public virtual bool UsesNativeIdentifier(TypePositionInfo info, StubCodeContext context) => _inner.UsesNativeIdentifier(info, context);
         public SignatureBehavior GetNativeSignatureBehavior(TypePositionInfo info) => _inner.GetNativeSignatureBehavior(info);

src/libraries/System.Runtime.InteropServices/gen/ComInterfaceGenerator/ComInterfaceGenerator.csproj

@@ -21,6 +21,7 @@
     <None Include="$(TargetPath)" Pack="true" PackagePath="analyzers/dotnet/cs" Visible="false" />
     <None Include="$(AssemblyName).props" Pack="true" PackagePath="build" />
     <Compile Include="..\Common\DefaultMarshallingInfoParser.cs" Link="Common\DefaultMarshallingInfoParser.cs" />
+    <Compile Include="..\..\tests\Common\ExceptionMarshalling.cs" Link="Common\ExceptionMarshalling.cs" />
   </ItemGroup>
 
   <ItemGroup>

src/libraries/System.Runtime.InteropServices/gen/ComInterfaceGenerator/ComInterfaceGeneratorHelpers.cs

@@ -56,6 +56,8 @@ internal static class ComInterfaceGeneratorHelpers
                         ? MarshalMode.ManagedToUnmanagedOut
                         : MarshalMode.UnmanagedToManagedIn));
 
+            generatorFactory = new NativeToManagedThisMarshallerFactory(generatorFactory);
+
             generatorFactory = new ByValueContentsMarshalKindValidator(generatorFactory);
 
             return MarshallingGeneratorFactoryKey.Create((env.TargetFramework, env.TargetFrameworkVersion), generatorFactory);

src/libraries/System.Runtime.InteropServices/gen/ComInterfaceGenerator/GeneratorDiagnostics.cs

@@ -28,7 +28,7 @@ public class Ids
         public static readonly DiagnosticDescriptor InvalidAttributedMethodSignature =
             new DiagnosticDescriptor(
             Ids.InvalidLibraryImportAttributeUsage,
-            GetResourceString(nameof(SR.InvalidLibraryImportAttributeUsageTitle)),
+            GetResourceString(nameof(SR.InvalidVirtualMethodIndexAttributeUsage)),
             GetResourceString(nameof(SR.InvalidAttributedMethodSignatureMessage)),
             Category,
             DiagnosticSeverity.Error,
@@ -38,7 +38,7 @@ public class Ids
         public static readonly DiagnosticDescriptor InvalidAttributedMethodContainingTypeMissingModifiers =
             new DiagnosticDescriptor(
             Ids.InvalidLibraryImportAttributeUsage,
-            GetResourceString(nameof(SR.InvalidLibraryImportAttributeUsageTitle)),
+            GetResourceString(nameof(SR.InvalidVirtualMethodIndexAttributeUsage)),
             GetResourceString(nameof(SR.InvalidAttributedMethodContainingTypeMissingModifiersMessage)),
             Category,
             DiagnosticSeverity.Error,
@@ -48,13 +48,23 @@ public class Ids
         public static readonly DiagnosticDescriptor InvalidStringMarshallingConfiguration =
             new DiagnosticDescriptor(
             Ids.InvalidLibraryImportAttributeUsage,
-            GetResourceString(nameof(SR.InvalidLibraryImportAttributeUsageTitle)),
+            GetResourceString(nameof(SR.InvalidVirtualMethodIndexAttributeUsage)),
             GetResourceString(nameof(SR.InvalidStringMarshallingConfigurationMessage)),
             Category,
             DiagnosticSeverity.Error,
             isEnabledByDefault: true,
             description: GetResourceString(nameof(SR.InvalidStringMarshallingConfigurationDescription)));
 
+        public static readonly DiagnosticDescriptor InvalidExceptionMarshallingConfiguration =
+            new DiagnosticDescriptor(
+            Ids.InvalidLibraryImportAttributeUsage,
+            GetResourceString(nameof(SR.InvalidVirtualMethodIndexAttributeUsage)),
+            GetResourceString(nameof(SR.InvalidExceptionMarshallingConfigurationMessage)),
+            Category,
+            DiagnosticSeverity.Error,
+            isEnabledByDefault: true,
+            description: GetResourceString(nameof(SR.InvalidExceptionMarshallingConfigurationDescription)));
+
         public static readonly DiagnosticDescriptor ParameterTypeNotSupported =
             new DiagnosticDescriptor(
                 Ids.TypeNotSupported,
@@ -167,6 +177,24 @@ public void ReportInvalidStringMarshallingConfiguration(
                     detailsMessage));
         }
 
+        /// <summary>
+        /// Report diagnostic for invalid configuration for string marshalling.
+        /// </summary>
+        /// <param name="attributeData">Attribute specifying the invalid configuration</param>
+        /// <param name="methodName">Name of the method</param>
+        /// <param name="detailsMessage">Specific reason the configuration is invalid</param>
+        public void ReportInvalidExceptionMarshallingConfiguration(
+            AttributeData attributeData,
+            string methodName,
+            string detailsMessage)
+        {
+            _diagnostics.Add(
+                attributeData.CreateDiagnostic(
+                    GeneratorDiagnostics.InvalidExceptionMarshallingConfiguration,
+                    methodName,
+                    detailsMessage));
+        }
+
         /// <summary>
         /// Report diagnostic for configuration that is not supported by the DLL import source generator
         /// </summary>