From 25fea0223b0bd51878d830dad6a7ea2a8250ab46 Mon Sep 17 00:00:00 2001
From: Jeremy Koritzinsky <jkoritzinsky@gmail.com>
Date: Thu, 13 Jan 2022 14:07:49 -0800
Subject: [PATCH 1/3] Write up our new design of using
 DisableRuntimeMarshallingAttribute to avoid needing to propagate blittability
 information through a custom user-defined attribute.

---
 .../DllImportGenerator/StructMarshalling.md   | 58 ++++++++++++-------
 1 file changed, 36 insertions(+), 22 deletions(-)

diff --git a/docs/design/libraries/DllImportGenerator/StructMarshalling.md b/docs/design/libraries/DllImportGenerator/StructMarshalling.md
index ab332616b74132..560bef2266c68e 100644
--- a/docs/design/libraries/DllImportGenerator/StructMarshalling.md
+++ b/docs/design/libraries/DllImportGenerator/StructMarshalling.md
@@ -19,16 +19,13 @@ We've been working around another problem for a while in the runtime-integrated
 
 I propose an opt-in design where the owner of a struct has to explicitly opt-in to usage for interop. This enables our team to add special support as desired for various types such as `Span<T>` while also avoiding the private reflection and limited type information issues mentioned above.
 
-This design would use these attributes:
+Both designs would use these attributes:
 
 ```csharp
 
 [AttributeUsage(AttributeTargets.Struct | AttributeTargets.Class)]
 public class GeneratedMarshallingAttribute : Attribute {}
 
-[AttributeUsage(AttributeTargets.Struct | AttributeTargets.Class)]
-public class BlittableTypeAttribute : Attribute {}
-
 [AttributeUsage(AttributeTargets.Struct | AttributeTargets.Class)]
 public class NativeMarshallingAttribute : Attribute
 {
@@ -111,27 +108,18 @@ When these members are present, the source generator will call the two-parameter
 
 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.
 
-### Usage
-
-There are 2 usage mechanisms of these attributes.
+### Determining if a type is "blittable"
 
-#### Usage 1, Source-generated interop
+For this design, we need to decide how to determine a type is blittable. We have two designs that we have experimented with below, and we have decided to go with design 2.
 
-The user can apply the `GeneratedMarshallingAttribute` to their structure `S`. The source generator will determine if the type is blittable. If it is blittable, the source generator will generate a partial definition and apply the `BlittableTypeAttribute` to the struct type `S`. Otherwise, it will generate a blittable representation of the struct with the aformentioned required shape and apply the `NativeMarshallingAttribute` and point it to the blittable representation. The blittable representation can either be generated as a separate top-level type or as a nested type on `S`.
-
-#### Usage 2, Manual interop
+#### Design 1: Introducing `BlittableTypeAttribute`
 
-The user may want to manually mark their types as marshalable in this system due to specific restrictions in their code base around marshaling specific types that the source generator does not account for. We could also use this internally to support custom types in source instead of in the code generator. In this scenario, the user would apply either the `BlittableTypeAttribute` or the `NativeMarshallingAttribute` attribute to their struct type. An analyzer would validate that the struct is blittable if the `BlittableTypeAttribute` is applied or validate that the native struct type is blittable and has marshalling methods of the required shape when the `NativeMarshallingAttribute` is applied.
+The built-in runtime marshalling system has an issue as mentioned above that the concept of `unmanaged` is not the same as the concept of `blittable`. Additionally, due to the ref assembly issue above, we cannot rely on ref assemblies to have accurate information in terms of fields of a type. To solve these issues in combination with the desire to enable manual interop, we need to provide a way for users to signal that a given type should be blittable and that the source generator should not generate marshalling code. We'll introduce a new attribute, the `BlittableTypeAttribute`:
 
-The P/Invoke source generator (as well as the struct source generator when nested struct types are used) would use the `BlittableTypeAttribute` and `NativeMarshallingAttribute` to determine how to marshal a value type parameter or field instead of looking at the fields of the struct directly.
-
-If a structure type does not have either the `BlittableTypeAttribute` or the `NativeMarshallingAttribute` applied at the type definition, the user can supply a `MarshalUsingAttribute` at the marshalling location (field, parameter, or return value) with a native type matching the same requirements as `NativeMarshallingAttribute`'s native type.
-
-All generated stubs will be marked with [`SkipLocalsInitAttribute`](https://docs.microsoft.com/dotnet/api/system.runtime.compilerservices.skiplocalsinitattribute) on supported frameworks. This does require attention when performing custom marshalling as the state of stub allocated memory will be in an undefined state.
-
-### Why do we need `BlittableTypeAttribute`?
-
-Based on the design above, it seems that we wouldn't need `BlittableTypeAttribute`. However, due to the ref assembly issue above in combination with the desire to enable manual interop, we need to provide a way for users to signal that a given type should be blittable and that the source generator should not generate marshalling code.
+```csharp
+[AttributeUsage(AttributeTargets.Struct | AttributeTargets.Class)]
+public class BlittableTypeAttribute : Attribute {}
+```
 
 I'll give a specific example for where we need the `BlittableTypeAttribute` below. Let's take a scenario where we don't have `BlittableTypeAttribute`.
 
@@ -210,7 +198,7 @@ When the source generator (either Struct, P/Invoke, Reverse P/Invoke, etc.) enco
 
 If someone actively disables the analyzer or writes their types in IL, then they have stepped out of the supported scenarios and marshalling code generated for their types may be inaccurate.
 
-#### Exception: Generics
+##### Exception: Generics
 
 Because the Roslyn compiler needs to be able to validate that there are not recursive struct definitions, reference assemblies have to contain a field of a type parameter type in the reference assembly if they do in the runtime assembly. As a result, we can inspect private generic fields reliably.
 
@@ -221,6 +209,32 @@ To enable blittable generics support in this struct marshalling model, we extend
 
 Since all fields typed with non-parameterized types are validated to be blittable at type definition time, we know that they are all blittable at type usage time. So, we only need to validate that the generic fields are instantiated with blittable types.
 
+#### Design 2: `DisableRuntimeMarshallingAttribute`
+
+As an alternative design, we can solve the `unmanaged` vs `blittable` dichotomy and the ref assembly problem by changing the definition of `blittable` to be the same as `unmanaged`. The `DisableRuntimeMarshallingAttribute` attribute helps us solve this problem. When applied to an assembly, this attribute causes the definition of `blittable` to change to "`unmanaged` types with no auto-layout fields" for all P/Invokes in the assembly, among other features. This definition will work for 99% of our scenarios.
+
+For the auto-layout clause, we have one small issue; today, our ref-assemblies do not expose if a value type is marked as `[StructLayout(LayoutKind.Auto)]`, so we'd still have some cases where we might have runtime failures. However, we can update the tooling used in dotnet/runtime, GenAPI, to expose this information if we so desire. Once that case is handled, we have a mechanism that we can safely use to determine, at compile time, which types are `blittable`. If we decide to not cover this case (as cases where users mark types as `LayoutKind.Auto` manually are exceptionally rare), we still have a solid design as Roslyn will automatically determine for us if a type is `unmanaged`, so we don't need to do any additional work.
+
+As `unmanaged` is a C# language concept, we can use Roslyn's APIs to determine if a type is `unmanaged` to determine if it is `blittable` without needing to define any new attributes and reshape the ecosystem. However, to enable this work, the DllImportGenerator, as well as any other source generators that generate calls to native code using the interop team's infrastructure, will need to require that the user applies the `DisableRuntimeMarshallingAttribute` to their assembly when custom user-defined types used. As we believe that users should be able to move over their assemblies to the new source-generated interop world as a whole assembly, we do not believe that this will cause any serious issues in adoption. To help support users in this case, the interop team will provide a code-fix that will generate the `DisableRuntimeMarshallingAttribute` for users when they use the source generator.
+
+### Usage
+
+There are 2 usage mechanisms of these attributes.
+
+#### Usage 1, Source-generated interop
+
+The user can apply the `GeneratedMarshallingAttribute` to their structure `S`. The source generator will determine if the type is blittable. If it is blittable, the source generator will generate a partial definition and apply the `BlittableTypeAttribute` to the struct type `S`. Otherwise, it will generate a blittable representation of the struct with the aformentioned required shape and apply the `NativeMarshallingAttribute` and point it to the blittable representation. The blittable representation can either be generated as a separate top-level type or as a nested type on `S`.
+
+#### Usage 2, Manual interop
+
+The user may want to manually mark their types as marshalable with custom marshalling rules in this system due to specific restrictions in their code base around marshaling specific types that the source generator does not account for. We could also use this internally to support custom types in source instead of in the code generator. In this scenario, the user would apply either the `NativeMarshallingAttribute` attribute to their struct type. An analyzer would validate that the native struct type is blittable and has marshalling methods of the required shape when the `NativeMarshallingAttribute` is applied.
+
+The P/Invoke source generator (as well as the struct source generator when nested struct types are used) would use the `NativeMarshallingAttribute` to determine how to marshal a non-blittable value type parameter or field.
+
+If a structure type is not blittable or the `NativeMarshallingAttribute` applied at the type definition, the user can supply a `MarshalUsingAttribute` at the marshalling location (field, parameter, or return value) with a native type matching the same requirements as `NativeMarshallingAttribute`'s native type.
+
+All generated stubs will be marked with [`SkipLocalsInitAttribute`](https://docs.microsoft.com/dotnet/api/system.runtime.compilerservices.skiplocalsinitattribute) on supported frameworks. This does require attention when performing custom marshalling as the state of stub allocated memory will be in an undefined state.
+
 ### Special case: Transparent Structures
 
 There has been discussion about Transparent Structures, structure types that are treated as their underlying types when passed to native code. The support for a `Value` property on a generated marshalling type supports the transparent struct support. For example, we could support strongly typed `HRESULT` returns with this model as shown below:

From 0566950fafe94a71bdf5bab99e3784345afbc2c1 Mon Sep 17 00:00:00 2001
From: Jeremy Koritzinsky <jkoritzinsky@gmail.com>
Date: Thu, 13 Jan 2022 15:08:22 -0800
Subject: [PATCH 2/3] Banish all mentions of the word "blittable" to the
 Problems section, the BlittableTypeAttribute section, and the start of the
 DisableRuntimeMarshalling section.

---
 .../DllImportGenerator/StructMarshalling.md   | 37 ++++++++++---------
 1 file changed, 19 insertions(+), 18 deletions(-)

diff --git a/docs/design/libraries/DllImportGenerator/StructMarshalling.md b/docs/design/libraries/DllImportGenerator/StructMarshalling.md
index 560bef2266c68e..ddc0401c5db79c 100644
--- a/docs/design/libraries/DllImportGenerator/StructMarshalling.md
+++ b/docs/design/libraries/DllImportGenerator/StructMarshalling.md
@@ -6,10 +6,11 @@ These types pose an interesting problem for a number of reasons listed below. Wi
 
 ## Problems
 
-- Unmanaged vs Blittable
-  - The C# language (and Roslyn) do not have a concept of "blittable types". It only has the concept of "unmanaged types", which is similar to blittable, but differs for `bool`s and `char`s. `bool` and `char` types are "unmanaged", but are never (in the case of `bool`), or only sometimes (in the case of `char`) blittable. As a result, we cannot use the "is this type unmanaged" check in Roslyn for structures.
+- What types require marshalling and what types can be passed as-is to native code?
+  - Unmanaged vs Blittable
+     - The C# language (and Roslyn) do not have a concept of "blittable types". It only has the concept of "unmanaged types", which is similar to blittable, but differs for `bool`s and `char`s. `bool` and `char` types are "unmanaged", but are never (in the case of `bool`), or only sometimes (in the case of `char`) blittable. As a result, we cannot use the "is this type unmanaged" check in Roslyn for structures without an additional mechanism provided by the runtime.
 - Limited type information in ref assemblies.
-  - In the ref assemblies generated by dotnet/runtime, we save space and prevent users from relying on private implementation details of structures by emitting limited information about their fields. Structures that have at least one non-object field are given a private `int` field, and structures that have at least one field that transitively contains an object are given one private `object`-typed field. As a result, we do not have full type information at code-generation time for any structures defined in the BCL when compiling a library that uses the ref assemblies.
+  - In the ref assemblies generated by dotnet/arcade's GenAPI (used in dotnet/runtime), we save space and prevent users from relying on private implementation details of structures by emitting limited information about their fields. Structures that have at least one non-object field are given a private `int` field, and structures that have at least one field that transitively contains an object are given one private `object`-typed field. As a result, we do not have full type information at code-generation time for any structures defined in the BCL when compiling a library that uses the ref assemblies.
 - Private reflection
   - Even when we do have information about all of the fields, we can't emit code that references them if they are private, so we would have to emit unsafe code and calculate offsets manually to support marshaling them.
 
@@ -19,7 +20,7 @@ We've been working around another problem for a while in the runtime-integrated
 
 I propose an opt-in design where the owner of a struct has to explicitly opt-in to usage for interop. This enables our team to add special support as desired for various types such as `Span<T>` while also avoiding the private reflection and limited type information issues mentioned above.
 
-Both designs would use these attributes:
+All design options would use these attributes:
 
 ```csharp
 
@@ -39,7 +40,7 @@ public class MarshalUsingAttribute : Attribute
 }
 ```
 
-The `NativeMarshallingAttribute` and `MarshalUsingAttribute` attributes would require that the provided native type `TNative` is a blittable `struct` and has a subset of three methods with the following names and shapes (with the managed type named TManaged):
+The `NativeMarshallingAttribute` and `MarshalUsingAttribute` attributes would require that the provided native type `TNative` is a `struct` that does not require any marshalling and has a subset of three methods with the following names and shapes (with the managed type named TManaged):
 
 ```csharp
 partial struct TNative
@@ -56,7 +57,7 @@ The analyzer will report an error if neither the constructor nor the `ToManaged`
 
 > :question: Does this API surface and shape work for all marshalling scenarios we plan on supporting? It may have issues with the current "layout class" by-value `[Out]` parameter marshalling where the runtime updates a `class` typed object in place. We already recommend against using classes for interop for performance reasons and a struct value passed via `ref` or `out` with the same members would cover this scenario.
 
-If the native type `TNative` also has a public `Value` property, then the value of the `Value` property will be passed to native code instead of the `TNative` value itself. As a result, the type `TNative` will be allowed to be non-blittable and the type of the `Value` property will be required to be blittable. If the `Value` property is settable, then when marshalling in the native-to-managed direction, a default value of `TNative` will have its `Value` property set to the native value. If `Value` does not have a setter, then marshalling from native to managed is not supported.
+If the native type `TNative` also has a public `Value` property, then the value of the `Value` property will be passed to native code instead of the `TNative` value itself. As a result, the type `TNative` will be allowed to require marshalling and the type of the `Value` property will be required be passable to native code without any additional marshalling. If the `Value` property is settable, then when marshalling in the native-to-managed direction, a default value of `TNative` will have its `Value` property set to the native value. If `Value` does not have a setter, then marshalling from native to managed is not supported.
 
 If a `Value` property is provided, the developer may also provide a ref-returning or readonly-ref-returning `GetPinnableReference` method. The `GetPinnableReference` method will be called before the `Value` property getter is called. The ref returned by `GetPinnableReference` will be pinned with a `fixed` statement, but the pinned value will not be used (it acts exclusively as a side-effect).
 
@@ -87,7 +88,7 @@ public struct TMarshaler
 
 #### Pinning
 
-Since C# 7.3 added a feature to enable custom pinning logic for user types, we should also add support for custom pinning logic. If the user provides a `GetPinnableReference` method on the managed type that matches the requirements to be used in a `fixed` statement and the pointed-to type is blittable, then we will support using pinning to marshal the managed value when possible. The analyzer should issue a warning when the pointed-to type would not match the final native type, accounting for the `Value` property on the native type. Since `MarshalUsingAttribute` is applied at usage time instead of at type authoring time, we will not enable the pinning feature since the implementation of `GetPinnableReference` is likely designed to match the default marshalling rules provided by the type author, not the rules provided by the marshaller provided by the `MarshalUsingAttribute`.
+Since C# 7.3 added a feature to enable custom pinning logic for user types, we should also add support for custom pinning logic. If the user provides a `GetPinnableReference` method on the managed type that matches the requirements to be used in a `fixed` statement and the pointed-to type would not require any additional marshalling, then we will support using pinning to marshal the managed value when possible. The analyzer should issue a warning when the pointed-to type would not match the final native type, accounting for the `Value` property on the native type. Since `MarshalUsingAttribute` is applied at usage time instead of at type authoring time, we will not enable the pinning feature since the implementation of `GetPinnableReference` is likely designed to match the default marshalling rules provided by the type author, not the rules provided by the marshaller provided by the `MarshalUsingAttribute`.
 
 #### Caller-allocated memory
 
@@ -108,13 +109,13 @@ When these members are present, the source generator will call the two-parameter
 
 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.
 
-### Determining if a type is "blittable"
+### Determining if a type is doesn't need marshalling
 
-For this design, we need to decide how to determine a type is blittable. We have two designs that we have experimented with below, and we have decided to go with design 2.
+For this design, we need to decide how to determine a type doesn't need to be marshalled and already has a representation we can pass directly to native code. We have two designs that we have experimented with below, and we have decided to go with design 2.
 
 #### Design 1: Introducing `BlittableTypeAttribute`
 
-The built-in runtime marshalling system has an issue as mentioned above that the concept of `unmanaged` is not the same as the concept of `blittable`. Additionally, due to the ref assembly issue above, we cannot rely on ref assemblies to have accurate information in terms of fields of a type. To solve these issues in combination with the desire to enable manual interop, we need to provide a way for users to signal that a given type should be blittable and that the source generator should not generate marshalling code. We'll introduce a new attribute, the `BlittableTypeAttribute`:
+Traditionally, the concept of "doesn't need marshalling" is referred to as "blittable". The built-in runtime marshalling system has an issue as mentioned above that the concept of `unmanaged` is not the same as the concept of `blittable`. Additionally, due to the ref assembly issue above, we cannot rely on ref assemblies to have accurate information in terms of fields of a type. To solve these issues in combination with the desire to enable manual interop, we need to provide a way for users to signal that a given type should be blittable and that the source generator should not generate marshalling code. We'll introduce a new attribute, the `BlittableTypeAttribute`:
 
 ```csharp
 [AttributeUsage(AttributeTargets.Struct | AttributeTargets.Class)]
@@ -211,11 +212,11 @@ Since all fields typed with non-parameterized types are validated to be blittabl
 
 #### Design 2: `DisableRuntimeMarshallingAttribute`
 
-As an alternative design, we can solve the `unmanaged` vs `blittable` dichotomy and the ref assembly problem by changing the definition of `blittable` to be the same as `unmanaged`. The `DisableRuntimeMarshallingAttribute` attribute helps us solve this problem. When applied to an assembly, this attribute causes the definition of `blittable` to change to "`unmanaged` types with no auto-layout fields" for all P/Invokes in the assembly, among other features. This definition will work for 99% of our scenarios.
+As an alternative design, we can use a different definition of "does not require marshalling". This design proposes changing the definition from "the runtime's definition of blittable" to "types considered `unmanaged` in C#). The `DisableRuntimeMarshallingAttribute` attribute helps us solve this problem. When applied to an assembly, this attribute causes the runtime to not do any marshalling for any types that are `unmanaged` types and do not have any auto-layout fields for all P/Invokes in the assembly even when the types do not fit the runtime's definition of "blittable", among other features. This definition will work for all of our scenarios, with one issue listed below.
 
-For the auto-layout clause, we have one small issue; today, our ref-assemblies do not expose if a value type is marked as `[StructLayout(LayoutKind.Auto)]`, so we'd still have some cases where we might have runtime failures. However, we can update the tooling used in dotnet/runtime, GenAPI, to expose this information if we so desire. Once that case is handled, we have a mechanism that we can safely use to determine, at compile time, which types are `blittable`. If we decide to not cover this case (as cases where users mark types as `LayoutKind.Auto` manually are exceptionally rare), we still have a solid design as Roslyn will automatically determine for us if a type is `unmanaged`, so we don't need to do any additional work.
+For the auto-layout clause, we have one small issue; today, our ref-assemblies do not expose if a value type is marked as `[StructLayout(LayoutKind.Auto)]`, so we'd still have some cases where we might have runtime failures. However, we can update the tooling used in dotnet/runtime, GenAPI, to expose this information if we so desire. Once that case is handled, we have a mechanism that we can safely use to determine, at compile time, which types will not require marshalling. If we decide to not cover this case (as cases where users mark types as `LayoutKind.Auto` manually are exceptionally rare), we still have a solid design as Roslyn will automatically determine for us if a type is `unmanaged`, so we don't need to do any additional work.
 
-As `unmanaged` is a C# language concept, we can use Roslyn's APIs to determine if a type is `unmanaged` to determine if it is `blittable` without needing to define any new attributes and reshape the ecosystem. However, to enable this work, the DllImportGenerator, as well as any other source generators that generate calls to native code using the interop team's infrastructure, will need to require that the user applies the `DisableRuntimeMarshallingAttribute` to their assembly when custom user-defined types used. As we believe that users should be able to move over their assemblies to the new source-generated interop world as a whole assembly, we do not believe that this will cause any serious issues in adoption. To help support users in this case, the interop team will provide a code-fix that will generate the `DisableRuntimeMarshallingAttribute` for users when they use the source generator.
+As `unmanaged` is a C# language concept, we can use Roslyn's APIs to determine if a type is `unmanaged` to determine if it does not require marshalling without needing to define any new attributes and reshape the ecosystem. However, to enable this work, the DllImportGenerator, as well as any other source generators that generate calls to native code using the interop team's infrastructure, will need to require that the user applies the `DisableRuntimeMarshallingAttribute` to their assembly when custom user-defined types used. As we believe that users should be able to move over their assemblies to the new source-generated interop world as a whole assembly, we do not believe that this will cause any serious issues in adoption. To help support users in this case, the interop team will provide a code-fix that will generate the `DisableRuntimeMarshallingAttribute` for users when they use the source generator.
 
 ### Usage
 
@@ -223,15 +224,15 @@ There are 2 usage mechanisms of these attributes.
 
 #### Usage 1, Source-generated interop
 
-The user can apply the `GeneratedMarshallingAttribute` to their structure `S`. The source generator will determine if the type is blittable. If it is blittable, the source generator will generate a partial definition and apply the `BlittableTypeAttribute` to the struct type `S`. Otherwise, it will generate a blittable representation of the struct with the aformentioned required shape and apply the `NativeMarshallingAttribute` and point it to the blittable representation. The blittable representation can either be generated as a separate top-level type or as a nested type on `S`.
+The user can apply the `GeneratedMarshallingAttribute` to their structure `S`. The source generator will determine if the type requires marshalling. If it does, it will generate a representation of the struct that does not require marshalling with the aformentioned required shape and apply the `NativeMarshallingAttribute` and point it to that new type. This generated representation can either be generated as a separate top-level type or as a nested type on `S`.
 
 #### Usage 2, Manual interop
 
-The user may want to manually mark their types as marshalable with custom marshalling rules in this system due to specific restrictions in their code base around marshaling specific types that the source generator does not account for. We could also use this internally to support custom types in source instead of in the code generator. In this scenario, the user would apply either the `NativeMarshallingAttribute` attribute to their struct type. An analyzer would validate that the native struct type is blittable and has marshalling methods of the required shape when the `NativeMarshallingAttribute` is applied.
+The user may want to manually mark their types as marshalable with custom marshalling rules in this system due to specific restrictions in their code base around marshaling specific types that the source generator does not account for. We could also use this internally to support custom types in source instead of in the code generator. In this scenario, the user would apply either the `NativeMarshallingAttribute` attribute to their struct type. An analyzer would validate that the native struct type does not require marshalling and has marshalling methods of the required shape when the `NativeMarshallingAttribute` is applied.
 
-The P/Invoke source generator (as well as the struct source generator when nested struct types are used) would use the `NativeMarshallingAttribute` to determine how to marshal a non-blittable value type parameter or field.
+The P/Invoke source generator (as well as the struct source generator when nested struct types are used) would use the `NativeMarshallingAttribute` to determine how to marshal a parameter or field with an unknown type.
 
-If a structure type is not blittable or the `NativeMarshallingAttribute` applied at the type definition, the user can supply a `MarshalUsingAttribute` at the marshalling location (field, parameter, or return value) with a native type matching the same requirements as `NativeMarshallingAttribute`'s native type.
+If a structure type does not meet the requirements to not require marshalling or the `NativeMarshallingAttribute` applied at the type definition, the user can supply a `MarshalUsingAttribute` at the marshalling location (field, parameter, or return value) with a native type matching the same requirements as `NativeMarshallingAttribute`'s native type.
 
 All generated stubs will be marked with [`SkipLocalsInitAttribute`](https://docs.microsoft.com/dotnet/api/system.runtime.compilerservices.skiplocalsinitattribute) on supported frameworks. This does require attention when performing custom marshalling as the state of stub allocated memory will be in an undefined state.
 
@@ -264,7 +265,7 @@ struct HRESULT
 
 In this case, the underlying native type would actually be an `int`, but the user could use the strongly-typed `HResult` type as the public surface area.
 
-> :question: Should we support transparent structures on manually annotated blittable types? If we do, we should do so in an opt-in manner to make it possible to have a `Value` property on the blittable type.
+> :question: Should we support transparent structures on manually annotated types that wouldn't need marshalling otherwise types? If we do, we should do so in an opt-in manner to make it possible to have a `Value` property on the type without assuming that it is for interop in all cases.
 
 #### Example: ComWrappers marshalling with Transparent Structures
 

From a5048c2fe5068e94e78f46cdc6e915c90039368f Mon Sep 17 00:00:00 2001
From: Jeremy Koritzinsky <jkoritzinsky@gmail.com>
Date: Wed, 19 Jan 2022 15:06:12 -0800
Subject: [PATCH 3/3] Apply suggestions from code review

Co-authored-by: Elinor Fung <elfung@microsoft.com>
---
 .../DllImportGenerator/StructMarshalling.md        | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/docs/design/libraries/DllImportGenerator/StructMarshalling.md b/docs/design/libraries/DllImportGenerator/StructMarshalling.md
index ddc0401c5db79c..63781d6d77dbb2 100644
--- a/docs/design/libraries/DllImportGenerator/StructMarshalling.md
+++ b/docs/design/libraries/DllImportGenerator/StructMarshalling.md
@@ -111,11 +111,11 @@ Type authors can pass down the `buffer` pointer to native code by defining a `Va
 
 ### Determining if a type is doesn't need marshalling
 
-For this design, we need to decide how to determine a type doesn't need to be marshalled and already has a representation we can pass directly to native code. We have two designs that we have experimented with below, and we have decided to go with design 2.
+For this design, we need to decide how to determine a type doesn't need to be marshalled and already has a representation we can pass directly to native code - that is, we need a definition for "does not require marshalling". We have two designs that we have experimented with below, and we have decided to go with design 2.
 
 #### Design 1: Introducing `BlittableTypeAttribute`
 
-Traditionally, the concept of "doesn't need marshalling" is referred to as "blittable". The built-in runtime marshalling system has an issue as mentioned above that the concept of `unmanaged` is not the same as the concept of `blittable`. Additionally, due to the ref assembly issue above, we cannot rely on ref assemblies to have accurate information in terms of fields of a type. To solve these issues in combination with the desire to enable manual interop, we need to provide a way for users to signal that a given type should be blittable and that the source generator should not generate marshalling code. We'll introduce a new attribute, the `BlittableTypeAttribute`:
+Traditionally, the concept of "does not require marshalling" is referred to as "blittable". The built-in runtime marshalling system has an issue as mentioned above that the concept of `unmanaged` is not the same as the concept of `blittable`. Additionally, due to the ref assembly issue above, we cannot rely on ref assemblies to have accurate information in terms of fields of a type. To solve these issues in combination with the desire to enable manual interop, we need to provide a way for users to signal that a given type should be blittable and that the source generator should not generate marshalling code. We'll introduce a new attribute, the `BlittableTypeAttribute`:
 
 ```csharp
 [AttributeUsage(AttributeTargets.Struct | AttributeTargets.Class)]
@@ -210,13 +210,13 @@ To enable blittable generics support in this struct marshalling model, we extend
 
 Since all fields typed with non-parameterized types are validated to be blittable at type definition time, we know that they are all blittable at type usage time. So, we only need to validate that the generic fields are instantiated with blittable types.
 
-#### Design 2: `DisableRuntimeMarshallingAttribute`
+#### Design 2: [`DisableRuntimeMarshallingAttribute`](https://github.com/dotnet/runtime/issues/60639)
 
-As an alternative design, we can use a different definition of "does not require marshalling". This design proposes changing the definition from "the runtime's definition of blittable" to "types considered `unmanaged` in C#). The `DisableRuntimeMarshallingAttribute` attribute helps us solve this problem. When applied to an assembly, this attribute causes the runtime to not do any marshalling for any types that are `unmanaged` types and do not have any auto-layout fields for all P/Invokes in the assembly even when the types do not fit the runtime's definition of "blittable", among other features. This definition will work for all of our scenarios, with one issue listed below.
+As an alternative design, we can use a different definition of "does not require marshalling". This design proposes changing the definition from "the runtime's definition of blittable" to "types considered `unmanaged` in C#". The `DisableRuntimeMarshallingAttribute` attribute helps us solve this problem. When applied to an assembly, this attribute causes the runtime to not do any marshalling for any types that are `unmanaged` types and do not have any auto-layout fields for all P/Invokes in the assembly; this includes when the types do not fit the runtime's definition of "blittable". This definition of "does not require marshalling" will work for all of our scenarios, with one issue listed below.
 
 For the auto-layout clause, we have one small issue; today, our ref-assemblies do not expose if a value type is marked as `[StructLayout(LayoutKind.Auto)]`, so we'd still have some cases where we might have runtime failures. However, we can update the tooling used in dotnet/runtime, GenAPI, to expose this information if we so desire. Once that case is handled, we have a mechanism that we can safely use to determine, at compile time, which types will not require marshalling. If we decide to not cover this case (as cases where users mark types as `LayoutKind.Auto` manually are exceptionally rare), we still have a solid design as Roslyn will automatically determine for us if a type is `unmanaged`, so we don't need to do any additional work.
 
-As `unmanaged` is a C# language concept, we can use Roslyn's APIs to determine if a type is `unmanaged` to determine if it does not require marshalling without needing to define any new attributes and reshape the ecosystem. However, to enable this work, the DllImportGenerator, as well as any other source generators that generate calls to native code using the interop team's infrastructure, will need to require that the user applies the `DisableRuntimeMarshallingAttribute` to their assembly when custom user-defined types used. As we believe that users should be able to move over their assemblies to the new source-generated interop world as a whole assembly, we do not believe that this will cause any serious issues in adoption. To help support users in this case, the interop team will provide a code-fix that will generate the `DisableRuntimeMarshallingAttribute` for users when they use the source generator.
+As `unmanaged` is a C# language concept, we can use Roslyn's APIs to determine if a type is `unmanaged` to determine if it does not require marshalling without needing to define any new attributes and reshape the ecosystem. However, to enable this work, the DllImportGenerator, as well as any other source generators that generate calls to native code using the interop team's infrastructure, will need to require that the user applies the `DisableRuntimeMarshallingAttribute` to their assembly when custom user-defined types are used. As we believe that users should be able to move over their assemblies to the new source-generated interop world as a whole assembly, we do not believe that this will cause any serious issues in adoption. To help support users in this case, the interop team will provide a code-fix that will generate the `DisableRuntimeMarshallingAttribute` for users when they use the source generator.
 
 ### Usage
 
@@ -232,7 +232,7 @@ The user may want to manually mark their types as marshalable with custom marsha
 
 The P/Invoke source generator (as well as the struct source generator when nested struct types are used) would use the `NativeMarshallingAttribute` to determine how to marshal a parameter or field with an unknown type.
 
-If a structure type does not meet the requirements to not require marshalling or the `NativeMarshallingAttribute` applied at the type definition, the user can supply a `MarshalUsingAttribute` at the marshalling location (field, parameter, or return value) with a native type matching the same requirements as `NativeMarshallingAttribute`'s native type.
+If a structure type does not meet the requirements to not require marshalling or does not have the `NativeMarshallingAttribute` applied at the type definition, the user can supply a `MarshalUsingAttribute` at the marshalling location (field, parameter, or return value) with a native type matching the same requirements as `NativeMarshallingAttribute`'s native type.
 
 All generated stubs will be marked with [`SkipLocalsInitAttribute`](https://docs.microsoft.com/dotnet/api/system.runtime.compilerservices.skiplocalsinitattribute) on supported frameworks. This does require attention when performing custom marshalling as the state of stub allocated memory will be in an undefined state.
 
@@ -265,7 +265,7 @@ struct HRESULT
 
 In this case, the underlying native type would actually be an `int`, but the user could use the strongly-typed `HResult` type as the public surface area.
 
-> :question: Should we support transparent structures on manually annotated types that wouldn't need marshalling otherwise types? If we do, we should do so in an opt-in manner to make it possible to have a `Value` property on the type without assuming that it is for interop in all cases.
+> :question: Should we support transparent structures on manually annotated types that wouldn't need marshalling otherwise? If we do, we should do so in an opt-in manner to make it possible to have a `Value` property on the type without assuming that it is for interop in all cases.
 
 #### Example: ComWrappers marshalling with Transparent Structures
 
