Skip to content

Commit 61673ac

Browse files
committed
PR feedback.
1 parent 59b6732 commit 61673ac

File tree

7 files changed

+137
-52
lines changed

7 files changed

+137
-52
lines changed

docs/design/libraries/LibraryImportGenerator/SpanMarshallers.md

Lines changed: 37 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -24,11 +24,11 @@ We have decided to match the managed semantics of `(ReadOnly)Span<T>` to provide
2424

2525
As part of this design, we would also want to include some in-box marshallers that follow the design laid out in the [Struct Marshalling design doc](./StructMarshalling.md) to support some additional scenarios:
2626

27-
- A marshaler that marshals an empty span as a non-null pointer.
27+
- A marshaller that marshals an empty span as a non-null pointer.
2828
- This marshaller would only support empty spans as it cannot correctly represent non-empty spans of non-blittable types.
29-
- A marshaler that marshals out a pointer to the native memory as a Span instead of copying the data into a managed array.
29+
- A marshaller that marshals out a pointer to the native memory as a Span instead of copying the data into a managed array.
3030
- This marshaller would only support blittable spans by design.
31-
- This marshaler will require the user to manually release the memory. Since this will be an opt-in marshaler, this scenario is already advanced and that additional requirement should be understandable to users who use this marshaler.
31+
- This marshaller will require the user to manually release the memory. Since this will be an opt-in marshaller, this scenario is already advanced and that additional requirement should be understandable to users who use this marshaller.
3232
- Since there is no mechansim to provide a collection length, the question of how to provide the span's length in this case is still unresolved. One option would be to always provide a length 1 span and require the user to create a new span with the correct size, but that feels like a bad design.
3333

3434
### Pros/Cons of Design 1
@@ -40,7 +40,7 @@ Pros:
4040

4141
Cons:
4242

43-
- Defining custom marshalers for non-empty spans of non-blittable types generically is impossible since the marshalling rules of the element's type cannot be known.
43+
- Defining custom marshallers for non-empty spans of non-blittable types generically is impossible since the marshalling rules of the element's type cannot be known.
4444
- Custom non-default marshalling of the span element types is impossible for non-built-in types.
4545
- Inlining the span marshalling fully into the stub increases on-disk IL size.
4646
- This design does not enable developers to easily define custom marshalling support for their own collection types, which may be desireable.
@@ -83,30 +83,55 @@ namespace System.Runtime.InteropServices
8383
The attribute would be used with a collection type like `Span<T>` as follows:
8484

8585
```csharp
86-
[NativeTypeMarshalling(typeof(DefaultSpanMarshaler<>))]
86+
[NativeTypeMarshalling(typeof(DefaultSpanMarshaller<>))]
8787
public ref struct Span<T>
8888
{
8989
...
9090
}
9191

9292
[CustomTypeMarshaller(typeof(Span<>), CustomTypeMarshallerKind.LinearCollection)]
93-
public ref struct DefaultSpanMarshaler<T>
93+
public ref struct DefaultSpanMarshaller<T>
9494
{
9595
...
9696
}
9797
```
9898

99-
The `CustomTypeMarshallerKind.LinearCollection` kind is applied to a generic marshaler type with the "LinearCollection marshaller shape" described below.
99+
The `CustomTypeMarshallerKind.LinearCollection` kind is applied to a generic marshaller type with the "LinearCollection marshaller shape" described below.
100100

101101
#### Supporting generics
102102

103-
Since generic parameters cannot be used in attributes, open generic types will be permitted in the `NativeTypeMarshallingAttribute` and the `CustomTypeMarshallerAttribute` as long as they have the same arity as the type the attribute is applied to and generic parameters provided to the applied-to type can also be used to construct the type passed as a parameter.
103+
Since generic parameters cannot be used in attributes, open generic types will be permitted in the `NativeTypeMarshallingAttribute` and the `CustomTypeMarshallerAttribute` as long as they have the same arity as the type with the attribute and generic parameters provided to the type with the attribute can also be used to construct the type passed as a parameter.
104104

105105
If a `CustomTypeMarshaller`-attributed type is a marshaller for a type for a pointer, an array, or a combination of pointers and arrays, the `CustomTypeMarshallerAttribute.GenericPlaceholder` type can be used in the place of the first generic parameter of the marshaller type.
106106

107+
For example:
108+
109+
```csharp
110+
[CustomTypeMarshaller(typeof(CustomTypeMarshallerAttribute.GenericPlaceholder), Direction = CustomTypeMarshallerDirection.In)]
111+
struct Marshaller<T>
112+
{
113+
public Marshaller(T managed);
114+
}
115+
[CustomTypeMarshaller(typeof(CustomTypeMarshallerAttribute.GenericPlaceholder[]), Direction = CustomTypeMarshallerDirection.In)]
116+
struct Marshaller<T>
117+
{
118+
public Marshaller(T[] managed);
119+
}
120+
[CustomTypeMarshaller(typeof(CustomTypeMarshallerAttribute.GenericPlaceholder*), Direction = CustomTypeMarshallerDirection.In)]
121+
struct Marshaller<T> where T : unmanaged
122+
{
123+
public Marshaller(T* managed);
124+
}
125+
[CustomTypeMarshaller(typeof(CustomTypeMarshallerAttribute.GenericPlaceholder*[]), Direction = CustomTypeMarshallerDirection.In)]
126+
struct Marshaller<T> where T : unmanaged
127+
{
128+
public Marshaller(T*[] managed);
129+
}
130+
```
131+
107132
#### LinearCollection marshaller shape
108133

109-
A generic collection marshaller would be required to have the following shape, in addition to the requirements for marshaler types used with the `CustomTypeMarshallerKind.Value` shape, excluding the constructors.
134+
A generic collection marshaller would be required to have the following shape, in addition to the requirements for marshaller types used with the `CustomTypeMarshallerKind.Value` shape, excluding the constructors.
110135

111136
```csharp
112137
[CustomTypeMarshaller(typeof(GenericCollection<, , ,...>), CustomTypeMarshallerKind.LinearCollection)]
@@ -118,8 +143,6 @@ public struct GenericContiguousCollectionMarshallerImpl<T, U, V,...>
118143
public GenericContiguousCollectionMarshallerImpl(GenericCollection<T, U, V, ...> collection, int nativeSizeOfElement);
119144
public GenericContiguousCollectionMarshallerImpl(GenericCollection<T, U, V, ...> collection, Span<byte> stackSpace, int nativeSizeOfElement); // optional
120145
121-
public const int StackBufferSize = /* */; // required if the span-based constructor is supplied.
122-
123146
/// <summary>
124147
/// A span that points to the memory where the managed values of the collection are stored (in the marshalling case) or should be stored (in the unmarshalling case).
125148
/// </summary>
@@ -180,7 +203,7 @@ To support supplying information about collection element counts, a parameterles
180203
The `ElementIndirectionLevel` property is added to support supplying marshalling info for element types in a collection. For example, if the user is passing a `List<List<Foo>>` from managed to native code, they could provide the following attributes to specify marshalling rules for the outer and inner lists and `Foo` separately:
181204

182205
```csharp
183-
private static partial void Bar([MarshalUsing(typeof(ListAsArrayMarshaller<List<Foo>>), CountElementName = nameof(count)), MarshalUsing(ConstantElementCount = 10, ElementIndirectionLevel = 1), MarshalUsing(typeof(FooMarshaler), ElementIndirectionLevel = 2)] List<List<Foo>> foos, int count);
206+
private static partial void Bar([MarshalUsing(typeof(ListAsArrayMarshaller<List<Foo>>), CountElementName = nameof(count)), MarshalUsing(ConstantElementCount = 10, ElementIndirectionLevel = 1), MarshalUsing(typeof(FooMarshaller), ElementIndirectionLevel = 2)] List<List<Foo>> foos, int count);
184207
```
185208

186209
Multiple `MarshalUsing` attributes can only be supplied on the same parameter or return value if the `ElementIndirectionLevel` property is set to distinct values. One `MarshalUsing` attribute per parameter or return value can leave the `ElementIndirectionLevel` property unset. This attribute controls the marshalling of the collection object passed in as the parameter. The sequence of managed types for `ElementIndirectionLevel` is based on the elements of the `ManagedValues` span on the collection marshaller of the previous indirection level. For example, for the marshalling info for `ElementIndirectionLevel = 1` above, the managed type is the type of the following C# expression: `ListAsArrayMarshaller<List<Foo>>.ManagedValues[0]`.
@@ -193,13 +216,13 @@ This design could be used to provide a default marshaller for spans and arrays.
193216

194217
```csharp
195218
[CustomTypeMarshaller(typeof(Span<>), CustomTypeMarshallerKind.LinearCollection)]
196-
public ref struct SpanMarshaler<T>
219+
public ref struct SpanMarshaller<T>
197220
{
198221
private Span<T> managedCollection;
199222

200223
private int nativeElementSize;
201224

202-
public SpanMarshaler(Span<T> collection, int nativeSizeOfElement)
225+
public SpanMarshaller(Span<T> collection, int nativeSizeOfElement)
203226
{
204227
managedCollection = collection;
205228
Value = Marshal.AllocCoTaskMem(collection.Length * nativeSizeOfElement);
@@ -290,8 +313,6 @@ public struct GenericContiguousCollectionMarshallerImpl<T, U, V,...>
290313
public GenericContiguousCollectionMarshallerImpl(GenericCollection<T, U, V, ...> collection, int nativeSizeOfElements);
291314
public GenericContiguousCollectionMarshallerImpl(GenericCollection<T, U, V, ...> collection, Span<byte> stackSpace, int nativeSizeOfElements); // optional
292315

293-
public const int StackBufferSize = /* */; // required if the span-based constructor is supplied.
294-
295316
- public Span<TCollectionElement> ManagedValues { get; }
296317

297318
- public void SetUnmarshalledCollectionLength(int length);

docs/design/libraries/LibraryImportGenerator/StructMarshalling.md

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,6 @@ public sealed class CustomTypeMarshallerAttribute : Attribute
5151
public Type ManagedType { get; }
5252
public CustomTypeMarshallerKind MarshallerKind { get; }
5353
public int BufferSize { get; set; }
54-
public bool RequiresStackBuffer { get; set; }
5554
}
5655

5756
public enum CustomTypeMarshallerKind
@@ -85,16 +84,16 @@ If a `Value` property is provided, the developer may also provide a ref-returnin
8584
A `ref` or `ref readonly` typed `Value` property is unsupported. If a ref-return is required, the type author can supply a `GetPinnableReference` method on the native type to pin the desired `ref` to return and then use `System.Runtime.CompilerServices.Unsafe.AsPointer` to get a pointer from the `ref` that will have already been pinned by the time the `Value` getter is called.
8685

8786
```csharp
88-
[NativeMarshalling(typeof(TMarshaler))]
87+
[NativeMarshalling(typeof(TMarshaller))]
8988
public struct TManaged
9089
{
9190
// ...
9291
}
9392

9493
[CustomTypeMarshaller(typeof(TManaged))]
95-
public struct TMarshaler
94+
public struct TMarshaller
9695
{
97-
public TMarshaler(TManaged managed) {}
96+
public TMarshaller(TManaged managed) {}
9897
public TManaged ToManaged() {}
9998

10099
public void FreeNative() {}
@@ -117,14 +116,14 @@ Since C# 7.3 added a feature to enable custom pinning logic for user types, we s
117116
Custom marshalers of collection-like types or custom string encodings (such as UTF-32) may want to use stack space for extra storage for additional performance when possible. If the `TNative` type provides additional constructor with the following signature and sets the `BufferSize` field on the `CustomTypeMarshallerAttribute`, then it will opt in to using a caller-allocated buffer:
118117

119118
```csharp
120-
[CustomTypeMarshaller(typeof(TManaged), BufferSize = /* */, RequiresStackBuffer = /* */)]
119+
[CustomTypeMarshaller(typeof(TManaged), BufferSize = /* */)]
121120
partial struct TNative
122121
{
123122
public TNative(TManaged managed, Span<byte> buffer) {}
124123
}
125124
```
126125

127-
When these members are present, the source generator will call the two-parameter constructor with a possibly stack-allocated buffer of `BufferSize` bytes when a stack-allocated buffer is usable. If a stack-allocated buffer is a requirement, the `RequiresStackBuffer` field should be set to `true` and the `buffer` will be guaranteed to be allocated on the stack. Setting the `RequiresStackBuffer` field to `false` is the same as not specifying the value in the attribute. Since a dynamically allocated buffer is not usable in all scenarios, for example Reverse P/Invoke and struct marshalling, a one-parameter constructor must also be provided for usage in those scenarios. This may also be provided by providing a two-parameter constructor with a default value for the second parameter.
126+
When these members are present, the source generator will call the two-parameter constructor with a possibly stack-allocated buffer of `BufferSize` bytes when a stack-allocated buffer is usable. Since a dynamically allocated buffer is not usable in all scenarios, for example Reverse P/Invoke and struct marshalling, a one-parameter constructor must also be provided for usage in those scenarios.
128127

129128
Type authors can pass down the `buffer` pointer to native code by defining a `Value` property that returns a pointer to the first element, generally through code using `MemoryMarshal.GetReference()` and `Unsafe.AsPointer`. If `RequiresStackBuffer` is not provided or set to `false`, the `buffer` span must be pinned to be used safely. The `buffer` span can be pinned by defining a `GetPinnableReference()` method on the native type that returns a reference to the first element of the span.
130129

docs/project/list-of-diagnostics.md

Lines changed: 18 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -158,18 +158,21 @@ The diagnostic id values reserved for .NET Libraries analyzer warnings are `SYSL
158158
| __`SYSLIB1052`__ | Specified configuration is not supported by source-generated P/Invokes |
159159
| __`SYSLIB1053`__ | Current target framework is not supported by source-generated P/Invokes |
160160
| __`SYSLIB1054`__ | Specified LibraryImportAttribute arguments cannot be forwarded to DllImportAttribute |
161-
| __`SYSLIB1055`__ | *_`SYSLIB1055`-`SYSLIB1069` reserved for Microsoft.Interop.LibraryImportGenerator._* |
162-
| __`SYSLIB1056`__ | *_`SYSLIB1055`-`SYSLIB1069` reserved for Microsoft.Interop.LibraryImportGenerator._* |
163-
| __`SYSLIB1057`__ | *_`SYSLIB1055`-`SYSLIB1069` reserved for Microsoft.Interop.LibraryImportGenerator._* |
164-
| __`SYSLIB1058`__ | *_`SYSLIB1055`-`SYSLIB1069` reserved for Microsoft.Interop.LibraryImportGenerator._* |
165-
| __`SYSLIB1059`__ | *_`SYSLIB1055`-`SYSLIB1069` reserved for Microsoft.Interop.LibraryImportGenerator._* |
166-
| __`SYSLIB1060`__ | *_`SYSLIB1055`-`SYSLIB1069` reserved for Microsoft.Interop.LibraryImportGenerator._* |
167-
| __`SYSLIB1061`__ | *_`SYSLIB1055`-`SYSLIB1069` reserved for Microsoft.Interop.LibraryImportGenerator._* |
168-
| __`SYSLIB1062`__ | *_`SYSLIB1055`-`SYSLIB1069` reserved for Microsoft.Interop.LibraryImportGenerator._* |
169-
| __`SYSLIB1063`__ | *_`SYSLIB1055`-`SYSLIB1069` reserved for Microsoft.Interop.LibraryImportGenerator._* |
170-
| __`SYSLIB1064`__ | *_`SYSLIB1055`-`SYSLIB1069` reserved for Microsoft.Interop.LibraryImportGenerator._* |
171-
| __`SYSLIB1065`__ | *_`SYSLIB1055`-`SYSLIB1069` reserved for Microsoft.Interop.LibraryImportGenerator._* |
172-
| __`SYSLIB1066`__ | *_`SYSLIB1055`-`SYSLIB1069` reserved for Microsoft.Interop.LibraryImportGenerator._* |
173-
| __`SYSLIB1067`__ | *_`SYSLIB1055`-`SYSLIB1069` reserved for Microsoft.Interop.LibraryImportGenerator._* |
174-
| __`SYSLIB1068`__ | *_`SYSLIB1055`-`SYSLIB1069` reserved for Microsoft.Interop.LibraryImportGenerator._* |
175-
| __`SYSLIB1069`__ | *_`SYSLIB1055`-`SYSLIB1069` reserved for Microsoft.Interop.LibraryImportGenerator._* |
161+
| __`SYSLIB1055`__ | *_`SYSLIB1055`-`SYSLIB1072` reserved for Microsoft.Interop.LibraryImportGenerator._* |
162+
| __`SYSLIB1056`__ | *_`SYSLIB1055`-`SYSLIB1072` reserved for Microsoft.Interop.LibraryImportGenerator._* |
163+
| __`SYSLIB1057`__ | *_`SYSLIB1055`-`SYSLIB1072` reserved for Microsoft.Interop.LibraryImportGenerator._* |
164+
| __`SYSLIB1058`__ | *_`SYSLIB1055`-`SYSLIB1072` reserved for Microsoft.Interop.LibraryImportGenerator._* |
165+
| __`SYSLIB1059`__ | *_`SYSLIB1055`-`SYSLIB1072` reserved for Microsoft.Interop.LibraryImportGenerator._* |
166+
| __`SYSLIB1060`__ | *_`SYSLIB1055`-`SYSLIB1072` reserved for Microsoft.Interop.LibraryImportGenerator._* |
167+
| __`SYSLIB1061`__ | *_`SYSLIB1055`-`SYSLIB1072` reserved for Microsoft.Interop.LibraryImportGenerator._* |
168+
| __`SYSLIB1062`__ | *_`SYSLIB1055`-`SYSLIB1072` reserved for Microsoft.Interop.LibraryImportGenerator._* |
169+
| __`SYSLIB1063`__ | *_`SYSLIB1055`-`SYSLIB1072` reserved for Microsoft.Interop.LibraryImportGenerator._* |
170+
| __`SYSLIB1064`__ | *_`SYSLIB1055`-`SYSLIB1072` reserved for Microsoft.Interop.LibraryImportGenerator._* |
171+
| __`SYSLIB1065`__ | *_`SYSLIB1055`-`SYSLIB1072` reserved for Microsoft.Interop.LibraryImportGenerator._* |
172+
| __`SYSLIB1066`__ | *_`SYSLIB1055`-`SYSLIB1072` reserved for Microsoft.Interop.LibraryImportGenerator._* |
173+
| __`SYSLIB1067`__ | *_`SYSLIB1055`-`SYSLIB1072` reserved for Microsoft.Interop.LibraryImportGenerator._* |
174+
| __`SYSLIB1068`__ | *_`SYSLIB1055`-`SYSLIB1072` reserved for Microsoft.Interop.LibraryImportGenerator._* |
175+
| __`SYSLIB1069`__ | *_`SYSLIB1055`-`SYSLIB1072` reserved for Microsoft.Interop.LibraryImportGenerator._* |
176+
| __`SYSLIB1070`__ | *_`SYSLIB1055`-`SYSLIB1072` reserved for Microsoft.Interop.LibraryImportGenerator._* |
177+
| __`SYSLIB1071`__ | *_`SYSLIB1055`-`SYSLIB1072` reserved for Microsoft.Interop.LibraryImportGenerator._* |
178+
| __`SYSLIB1072`__ | *_`SYSLIB1055`-`SYSLIB1072` reserved for Microsoft.Interop.LibraryImportGenerator._* |
Lines changed: 18 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,18 @@
11
// Licensed to the .NET Foundation under one or more agreements.
22
// The .NET Foundation licenses this file to you under the MIT license.
33

4-
#nullable enable
5-
64
#if !TEST_CORELIB
75
using System.ComponentModel;
86
#endif
97

10-
//
11-
// Types in this file are used for generated p/invokes (docs/design/features/source-generator-pinvokes.md).
12-
//
138
namespace System.Runtime.InteropServices
149
{
10+
/// <summary>
11+
/// A direction of marshalling data into or out of the managed environment
12+
/// </summary>
13+
/// <remarks>
14+
/// <seealso cref="CustomTypeMarshallerAttribute.Direction"/>
15+
/// </remarks>
1516
[Flags]
1617
#if LIBRARYIMPORT_GENERATOR_TEST
1718
public
@@ -20,12 +21,24 @@ namespace System.Runtime.InteropServices
2021
#endif
2122
enum CustomTypeMarshallerDirection
2223
{
24+
/// <summary>
25+
/// No marshalling direction
26+
/// </summary>
2327
#if !TEST_CORELIB
2428
[EditorBrowsable(EditorBrowsableState.Never)]
2529
#endif
2630
None = 0,
31+
/// <summary>
32+
/// Marshalling from a managed environment to an unmanaged environment
33+
/// </summary>
2734
In = 0x1,
35+
/// <summary>
36+
/// Marshalling from an unmanaged environment to a managed environment
37+
/// </summary>
2838
Out = 0x2,
39+
/// <summary>
40+
/// Marshalling to and from managed and unmanaged environments
41+
/// </summary>
2942
Ref = In | Out,
3043
}
3144
}
Lines changed: 18 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,14 @@
11
// Licensed to the .NET Foundation under one or more agreements.
22
// The .NET Foundation licenses this file to you under the MIT license.
33

4-
#nullable enable
5-
6-
//
7-
// Types in this file are used for generated p/invokes (docs/design/features/source-generator-pinvokes.md).
8-
//
94
namespace System.Runtime.InteropServices
105
{
6+
/// <summary>
7+
/// Optional features supported by custom type marshallers.
8+
/// </summary>
9+
/// <remarks>
10+
/// <seealso cref="CustomTypeMarshallerAttribute.Features"/>
11+
/// </remarks>
1112
[Flags]
1213
#if LIBRARYIMPORT_GENERATOR_TEST
1314
public
@@ -16,9 +17,21 @@ namespace System.Runtime.InteropServices
1617
#endif
1718
enum CustomTypeMarshallerFeatures
1819
{
20+
/// <summary>
21+
/// No optional features supported
22+
/// </summary>
1923
None = 0,
24+
/// <summary>
25+
/// The marshaller owns unmanaged resources that must be freed
26+
/// </summary>
2027
UnmanagedResources = 0x1,
28+
/// <summary>
29+
/// The marshaller can use a caller-allocated buffer instead of allocating in some scenarios
30+
/// </summary>
2131
CallerAllocatedBuffer = 0x2,
32+
/// <summary>
33+
/// The marshaller uses the two-stage marshalling design for its <see cref="CustomTypeMarshallerKind"/> instead of the one-stage design.
34+
/// </summary>
2235
TwoStageMarshalling = 0x4
2336
}
2437
}

0 commit comments

Comments
 (0)