[API Proposal]: Specify supported custom type marshalling features on CustomTypeMarshallerAttribute

[jkoritzinsky]

Background and motivation

For our source-generated interop work, we have recently approved the System.Runtime.InteropServices.CustomTypeMarshallerAttribute, which we will use to mark user-defined custom marshallers to help us guide users to define their marshaller types with the correct shapes (as interfaces are not usable with ref structs).

Currently, the design calls for us to emit code fixes to offer up the different members that users may want to add. However, we have no mechanism to allow users to say "I want to support these features in my marshaller, so validate at marshaller authoring time that I have the right members".

This issue proposes an enum and an extension of the CustomTypeMarshallerAttribute to enable developers to declaratively state which features they plan to support. This will allow our analyzer to enforce that the members required by each shape for each feature are present, and it will allow users of the marshaller types to know which features they can use when using the marshaller.

Our analyzer will enforce that all members required for the features specified by the users are present. The interop source generator will only use the features of the marshaller type specified in the attribute, even if other members are specified.

API Proposal

namespace System.Runtime.InteropServices
{
    public class CustomTypeMarshallerAttribute : Attribute
    {
+        public CustomTypeMarshallerDirection Direction { get; set; } = CustomTypeMarshallerDirection.Ref;
+        public CustomTypeMarshallerFeatures Features { get; set; }
    }

+    [Flags]
+    public enum CustomTypeMarshallerDirection
+    {
+        In = 0x1,
+        Out = 0x2,
+        Ref = CustomTypeMarshallerDirection.In | CustomTypeMarshallerDirection.Out
+    }
+    [Flags]
+    public enum CustomTypeMarshallerFeatures
+    {
+        None = 0x0,
+        UnmanagedResources = 0x1,
+        CallerAllocatedBuffer = 0x2,
+        TwoStageMarshalling = 0x4,
+    }
}

API Usage

[CustomTypeMarshaller(typeof(string), Features = CustomTypeMarshallerFeatures.UnmanagedResources | CustomTypeMarshallerFeatures.CallerAllocatedBuffer | CustomTypeMarshallerFeatures.TwoStageMarshalling, BufferSize = 0x200 )]
public struct Utf16StringMarshaller
{

    // Required for In direction
    public Utf16StringMarshaller(string s);
    // Required for In direction and CallerAllocatedBuffer feature
    public Utf16StringMarshaller(string s, Span<byte> buffer);

   // Required for In direction and TwoStageMarsahalling feature
    public IntPtr ToNativeValue();
   // Required for Out direction and TwoStageMarsahalling feature
    public void FromNativeValue(IntPtr value);

   // Required for Out direction
    public string ToManaged();

    // Required for UnmanagedResources feature
    public void FreeNative();
}

public struct MultiBoolStruct
{
    public bool b1;
    public bool b2;
}

[CustomTypeMarshaller(typeof(MultiBoolStruct), Direction = CustomTypeMarshallerDirection.In )]
public struct MultiBoolStructMarshaller
{
    // Required for In direction
    public MultiBoolStructMarshaller(MultiBoolStruct s);
}

Alternative Designs

We could instead keep the API as-is today and require the generators to inspect the custom marshaller type's API surface as usage time to ensure that it has all of the required members.

Risks

Enforcement will depend on an analyzer, with can be turned off. However, if someone turns off the analyzer then they'll just end up with a type that can't be used by the source generator due to their own choices, so I don't think this is a large concern.

We have a limited number of bits for features, so we need to make sure that the features are general enough that we won't exceed our limited bitmask size.

[ghost]

Tagging subscribers to this area: @dotnet/interop-contrib
See info in area-owners.md if you want to be subscribed.

Issue Details

---

Background and motivation

For our source-generated interop work, we have recently approved the System.Runtime.InteropServices.CustomTypeMarshallerAttribute, which we will use to mark user-defined custom marshallers to help us guide users to define their marshaller types with the correct shapes (as interfaces are not usable with ref structs).

Currently, the design calls for us to emit code fixes to offer up the different members that users may want to add. However, we have no mechanism to allow users to say "I want to support these features in my marshaller, so validate at marshaller authoring time that I have the right members".

This issue proposes an enum and an extension of the CustomTypeMarshallerAttribute to enable developers to declaratively state which features they plan to support. This will allow our analyzer to enforce that the members required by each shape for each feature are present, and it will allow users of the marshaller types to know which features they can use when using the marshaller.

Our analyzer will enforce that all members required for the features specified by the users are present. The interop source generator will only use the features of the marshaller type specified in the attribute, even if other members are specified.

API Proposal

namespace System.Runtime.InteropServices
{
    public class CustomTypeMarshallerAttribute : Attribute
    {
+        public CustomTypeMarshallerFeatures Features { get; } = CustomTypeMarshallerFeatures.ManagedToNative | CustomTypeMarshallerFeatures.NativeToManaged;
    }

    [Flags]
    public enum CustomTypeMarshallerFeatures
    {
        ManagedToNative = 0x1,
        NativeToManaged = 0x2,
        FreeNativeResources = 0x4,
        ManagedToNativeWithCallerAllocatedBuffer = 0x8,
        TwoStageMarshalling = 0x10,
        PinnableMarshallerType = 0x20,
    }
}

API Usage

[CustomTypeMarshaller(typeof(string), Features = CustomTypeMarshallerFeatures.ManagedToNative | CustomTypeMarshallerFeatures.NativeToManaged | CustomTypeMarshallerFeatures.FreeNativeResources | CustomTypeMarshallerFeatures.ManagedToNativeWithCallerAllocatedBuffer | CustomTypeMarshallerFeatures.TwoStageMarshalling, BufferSize = 0x200 )]
public struct Utf16StringMarshaller
{
    public Utf16StringMarshaller(string s);
    public Utf16StringMarshaller(string s, Span<byte> buffer);

    public IntPtr ToNativeValue();
    public void FromNativeValue(IntPtr value);

    public string ToManaged();

    public void FreeNative();
}

Alternative Designs

We could instead keep the API as-is today and require the generators to inspect the custom marshaller type's API surface as usage time to ensure that it has all of the required members.

Risks

Enforcement will depend on an analyzer, with can be turned off. However, if someone turns off the analyzer then they'll just end up with a type that can't be used by the source generator due to their own choices, so I don't think this is a large concern.

Author:	jkoritzinsky
Assignees:	-
Labels:	api-suggestion, area-System.Runtime.InteropServices, source-generator
Milestone:	-

[jkoritzinsky]

cc: @pavelsavara

[terrajobst]

Video

• We should add a zero-value for CustomTypeMarshallerDirection such that default initialized values have an associated member. We should call it None and simply reject this as invalid for CustomTypeMarshallerAttribute.Direction. We should hide it.

namespace System.Runtime.InteropServices
{
    public partial class CustomTypeMarshallerAttribute : Attribute
    {
        public CustomTypeMarshallerDirection Direction { get; set; } = CustomTypeMarshallerDirection.Ref;
        public CustomTypeMarshallerFeatures Features { get; set; }
    }
    [Flags]
    public enum CustomTypeMarshallerDirection
    {
        [EditorBrowsable(EditorBrowsableState.Never)]
        None = 0x0,
        In = 0x1,
        Out = 0x2,
        Ref = CustomTypeMarshallerDirection.In | CustomTypeMarshallerDirection.Out
    }
    [Flags]
    public enum CustomTypeMarshallerFeatures
    {
        None = 0x0,
        UnmanagedResources = 0x1,
        CallerAllocatedBuffer = 0x2,
        TwoStageMarshalling = 0x4,
    }
}

[marhja]

Seems like the Framework design guidelines only recommend a None value for simple enums. It recommends that for flags enums, like CustomTypeMarshallerDirection, enum values of zero should be avoided unless it means all flags are cleared, which makes no sense in this case.