Skip to content

[DllImportGenerator] Update buffer size field for custom marshalling #63316

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
elinor-fung merged 3 commits into from
Jan 4, 2022
Merged

[DllImportGenerator] Update buffer size field for custom marshalling #63316

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d32d7eb
[DllImportGenerator] Update buffer size field for custom marshalling
elinor-fung Jan 3, 2022
8dca307
Update src/libraries/System.Runtime.InteropServices/gen/DllImportGene…
elinor-fung Jan 4, 2022
a096ab8
Update code snippet test case
elinor-fung Jan 4, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions src/libraries/Common/src/System/Runtime/InteropServices/ArrayMarshaller.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,8 @@ public ArrayMarshaller(T[]? managed, Span<byte> stackSpace, int sizeOfNativeElem
/// Number kept small to ensure that P/Invokes with a lot of array parameters doesn't
/// blow the stack since this is a new optimization in the code-generated interop.
/// </summary>
public const int StackBufferSize = 0x200;
public const int BufferSize = 0x200;
public const bool RequiresStackBuffer = true;

public Span<T> ManagedValues => _managedArray;

Expand Down Expand Up @@ -160,7 +161,8 @@ public PtrArrayMarshaller(T*[]? managed, Span<byte> stackSpace, int sizeOfNative
/// Number kept small to ensure that P/Invokes with a lot of array parameters doesn't
/// blow the stack since this is a new optimization in the code-generated interop.
/// </summary>
public const int StackBufferSize = 0x200;
public const int BufferSize = 0x200;
public const bool RequiresStackBuffer = true;

public Span<IntPtr> ManagedValues => Unsafe.As<IntPtr[]>(_managedArray);

Expand Down
4 changes: 2 additions & 2 deletions ...es/System.Runtime.InteropServices/gen/DllImportGenerator/Analyzers/AnalyzerDiagnostics.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,8 @@ public static class Ids
public const string ValuePropertyMustHaveSetter = Prefix + "008";
public const string ValuePropertyMustHaveGetter = Prefix + "009";
public const string GetPinnableReferenceShouldSupportAllocatingMarshallingFallback = Prefix + "010";
public const string StackallocMarshallingShouldSupportAllocatingMarshallingFallback = Prefix + "011";
public const string StackallocConstructorMustHaveStackBufferSizeConstant = Prefix + "012";
public const string CallerAllocMarshallingShouldSupportAllocatingMarshallingFallback = Prefix + "011";
public const string CallerAllocConstructorMustHaveStackBufferSizeConstant = Prefix + "012";
public const string RefValuePropertyUnsupported = Prefix + "014";
public const string NativeGenericTypeMustBeClosedOrMatchArity = Prefix + "016";
public const string MarshallerGetPinnableReferenceRequiresValueProperty = Prefix + "018";
Expand Down
44 changes: 22 additions & 22 deletions ...Runtime.InteropServices/gen/DllImportGenerator/Analyzers/ManualTypeMarshallingAnalyzer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,25 +126,25 @@ public class ManualTypeMarshallingAnalyzer : DiagnosticAnalyzer
isEnabledByDefault: true,
description: GetResourceString(nameof(Resources.GetPinnableReferenceShouldSupportAllocatingMarshallingFallbackDescription)));

public static readonly DiagnosticDescriptor StackallocMarshallingShouldSupportAllocatingMarshallingFallbackRule =
public static readonly DiagnosticDescriptor CallerAllocMarshallingShouldSupportAllocatingMarshallingFallbackRule =
new DiagnosticDescriptor(
Ids.StackallocMarshallingShouldSupportAllocatingMarshallingFallback,
"StackallocMarshallingShouldSupportAllocatingMarshallingFallback",
GetResourceString(nameof(Resources.StackallocMarshallingShouldSupportAllocatingMarshallingFallbackMessage)),
Ids.CallerAllocMarshallingShouldSupportAllocatingMarshallingFallback,
"CallerAllocMarshallingShouldSupportAllocatingMarshallingFallback",
GetResourceString(nameof(Resources.CallerAllocMarshallingShouldSupportAllocatingMarshallingFallbackMessage)),
Category,
DiagnosticSeverity.Warning,
isEnabledByDefault: true,
description: GetResourceString(nameof(Resources.StackallocMarshallingShouldSupportAllocatingMarshallingFallbackDescription)));
description: GetResourceString(nameof(Resources.CallerAllocMarshallingShouldSupportAllocatingMarshallingFallbackDescription)));

public static readonly DiagnosticDescriptor StackallocConstructorMustHaveStackBufferSizeConstantRule =
public static readonly DiagnosticDescriptor CallerAllocConstructorMustHaveBufferSizeConstantRule =
new DiagnosticDescriptor(
Ids.StackallocConstructorMustHaveStackBufferSizeConstant,
"StackallocConstructorMustHaveStackBufferSizeConstant",
GetResourceString(nameof(Resources.StackallocConstructorMustHaveStackBufferSizeConstantMessage)),
Ids.CallerAllocConstructorMustHaveStackBufferSizeConstant,
"CallerAllocConstructorMustHaveBufferSizeConstant",
GetResourceString(nameof(Resources.CallerAllocConstructorMustHaveBufferSizeConstantMessage)),
Category,
DiagnosticSeverity.Error,
isEnabledByDefault: true,
description: GetResourceString(nameof(Resources.StackallocConstructorMustHaveStackBufferSizeConstantDescription)));
description: GetResourceString(nameof(Resources.CallerAllocConstructorMustHaveBufferSizeConstantDescription)));

public static readonly DiagnosticDescriptor RefValuePropertyUnsupportedRule =
new DiagnosticDescriptor(
Expand Down Expand Up @@ -189,8 +189,8 @@ public class ManualTypeMarshallingAnalyzer : DiagnosticAnalyzer
ValuePropertyMustHaveSetterRule,
ValuePropertyMustHaveGetterRule,
GetPinnableReferenceShouldSupportAllocatingMarshallingFallbackRule,
StackallocMarshallingShouldSupportAllocatingMarshallingFallbackRule,
StackallocConstructorMustHaveStackBufferSizeConstantRule,
CallerAllocMarshallingShouldSupportAllocatingMarshallingFallbackRule,
CallerAllocConstructorMustHaveBufferSizeConstantRule,
RefValuePropertyUnsupportedRule,
NativeGenericTypeMustBeClosedOrMatchArityRule,
MarshallerGetPinnableReferenceRequiresValuePropertyRule);
Expand Down Expand Up @@ -425,7 +425,7 @@ private void AnalyzeNativeMarshalerType(SymbolAnalysisContext context, ITypeSymb
}

bool hasConstructor = false;
bool hasStackallocConstructor = false;
bool hasCallerAllocSpanConstructor = false;
foreach (IMethodSymbol ctor in marshalerType.Constructors)
{
if (ctor.IsStatic)
Expand All @@ -435,15 +435,15 @@ private void AnalyzeNativeMarshalerType(SymbolAnalysisContext context, ITypeSymb

hasConstructor = hasConstructor || ManualTypeMarshallingHelper.IsManagedToNativeConstructor(ctor, type, variant);

if (!hasStackallocConstructor && ManualTypeMarshallingHelper.IsStackallocConstructor(ctor, type, _spanOfByte, variant))
if (!hasCallerAllocSpanConstructor && ManualTypeMarshallingHelper.IsCallerAllocatedSpanConstructor(ctor, type, _spanOfByte, variant))
{
hasStackallocConstructor = true;
IFieldSymbol stackAllocSizeField = nativeType.GetMembers("StackBufferSize").OfType<IFieldSymbol>().FirstOrDefault();
if (stackAllocSizeField is null or { DeclaredAccessibility: not Accessibility.Public } or { IsConst: false } or { Type: not { SpecialType: SpecialType.System_Int32 } })
hasCallerAllocSpanConstructor = true;
IFieldSymbol bufferSizeField = nativeType.GetMembers(ManualTypeMarshallingHelper.BufferSizeFieldName).OfType<IFieldSymbol>().FirstOrDefault();
if (bufferSizeField is null or { DeclaredAccessibility: not Accessibility.Public } or { IsConst: false } or { Type: not { SpecialType: SpecialType.System_Int32 } })
{
context.ReportDiagnostic(
GetDiagnosticLocations(context, ctor, nativeMarshalerAttributeData).CreateDiagnostic(
StackallocConstructorMustHaveStackBufferSizeConstantRule,
CallerAllocConstructorMustHaveBufferSizeConstantRule,
nativeType.ToDisplayString()));
}
}
Expand All @@ -452,7 +452,7 @@ private void AnalyzeNativeMarshalerType(SymbolAnalysisContext context, ITypeSymb
bool hasToManaged = ManualTypeMarshallingHelper.HasToManagedMethod(marshalerType, type);

// Validate that the native type has at least one marshalling method (either managed to native or native to managed)
if (!hasConstructor && !hasStackallocConstructor && !hasToManaged)
if (!hasConstructor && !hasCallerAllocSpanConstructor && !hasToManaged)
{
context.ReportDiagnostic(
GetDiagnosticLocations(context, marshalerType, nativeMarshalerAttributeData).CreateDiagnostic(
Expand All @@ -462,11 +462,11 @@ private void AnalyzeNativeMarshalerType(SymbolAnalysisContext context, ITypeSymb
}

// Validate that this type can support marshalling when stackalloc is not usable.
if (isNativeMarshallingAttribute && hasStackallocConstructor && !hasConstructor)
if (isNativeMarshallingAttribute && hasCallerAllocSpanConstructor && !hasConstructor)
{
context.ReportDiagnostic(
GetDiagnosticLocations(context, marshalerType, nativeMarshalerAttributeData).CreateDiagnostic(
StackallocMarshallingShouldSupportAllocatingMarshallingFallbackRule,
CallerAllocMarshallingShouldSupportAllocatingMarshallingFallbackRule,
marshalerType.ToDisplayString()));
}

Expand All @@ -490,7 +490,7 @@ private void AnalyzeNativeMarshalerType(SymbolAnalysisContext context, ITypeSymb
// We error if either of the conditions below are partially met but not fully met:
// - a constructor and a Value property getter
// - a ToManaged method and a Value property setter
if ((hasConstructor || hasStackallocConstructor) && valueProperty.GetMethod is null)
if ((hasConstructor || hasCallerAllocSpanConstructor) && valueProperty.GetMethod is null)
{
context.ReportDiagnostic(
GetDiagnosticLocations(context, valueProperty, nativeMarshalerAttributeData).CreateDiagnostic(
Expand Down
72 changes: 36 additions & 36 deletions src/libraries/System.Runtime.InteropServices/gen/DllImportGenerator/Resources.Designer.cs
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

24 changes: 12 additions & 12 deletions src/libraries/System.Runtime.InteropServices/gen/DllImportGenerator/Resources.resx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,18 @@
<data name="BlittableTypeMustBeBlittableMessage" xml:space="preserve">
<value>Type '{0}' is marked with 'BlittableTypeAttribute' but is not blittable</value>
</data>
<data name="CallerAllocConstructorMustHaveBufferSizeConstantDescription" xml:space="preserve">
<value>When a constructor taking a Span&lt;byte&gt; is specified on the native type, the type must also have a public integer constant named BufferSize to provide the size of the caller-allocated buffer.</value>
</data>
<data name="CallerAllocConstructorMustHaveBufferSizeConstantMessage" xml:space="preserve">
<value>The native type '{0}' must have a 'public const int BufferSize' field that specifies the size of the stack buffer because it has a constructor that takes a caller-allocated Span&lt;byte&gt;</value>
</data>
<data name="CallerAllocMarshallingShouldSupportAllocatingMarshallingFallbackDescription" xml:space="preserve">
<value>A type that supports marshalling from managed to native using a caller-allocated buffer should also support marshalling from managed to native where using a caller-allocated buffer is impossible.</value>
</data>
<data name="CallerAllocMarshallingShouldSupportAllocatingMarshallingFallbackMessage" xml:space="preserve">
<value>Native type '{0}' has a constructor taking a caller-allocated buffer, but does not support marshalling in scenarios where using a caller-allocated buffer is impossible</value>
Copy link
Member

@danmoseley danmoseley Jan 4, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
<value>Native type '{0}' has a constructor taking a caller-allocated buffer, but does not support marshalling in scenarios where using a caller-allocated buffer is impossible</value>
<value>Native type '{0}' has a constructor taking a caller-allocated buffer, but does not support marshalling in scenarios where using a caller-allocated buffer is impossible.</value>

Copy link
Member Author

@elinor-fung elinor-fung Jan 4, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since it is a single sentence, I believe the message should not have the period (RS1032).

</data>
<data name="CannotHaveMultipleMarshallingAttributesDescription" xml:space="preserve">
<value>The 'BlittableTypeAttribute' and 'NativeMarshallingAttribute' attributes are mutually exclusive.</value>
</data>
Expand Down Expand Up @@ -262,18 +274,6 @@
<data name="SafeHandleByRefMustBeConcrete" xml:space="preserve">
<value>An abstract type derived from 'SafeHandle' cannot be marshalled by reference. The provided type must be concrete.</value>
</data>
<data name="StackallocConstructorMustHaveStackBufferSizeConstantDescription" xml:space="preserve">
<value>When constructor taking a Span&lt;byte&gt; is specified on the native type, the type must also have a public integer constant named StackBufferSize to provide the size of the stack-allocated buffer.</value>
</data>
<data name="StackallocConstructorMustHaveStackBufferSizeConstantMessage" xml:space="preserve">
<value>The native type '{0}' must have a 'public const int StackBufferSize' field that specifies the size of the stack buffer because it has a constructor that takes a stack-allocated Span&lt;byte&gt;</value>
</data>
<data name="StackallocMarshallingShouldSupportAllocatingMarshallingFallbackDescription" xml:space="preserve">
<value>A type that supports marshalling from managed to native by stack allocation should also support marshalling from managed to native where stack allocation is impossible.</value>
</data>
<data name="StackallocMarshallingShouldSupportAllocatingMarshallingFallbackMessage" xml:space="preserve">
<value>Native type '{0}' has a stack-allocating constructor does not support marshalling in scenarios where stack allocation is impossible</value>
</data>
<data name="TargetFrameworkNotSupportedDescription" xml:space="preserve">
<value>P/Invoke source generation is not supported on unknown target framework v{0}. The generated source will not be compatible with other frameworks.</value>
<comment>{0} is a version number</comment>
Expand Down
5 changes: 3 additions & 2 deletions ...ime.InteropServices/gen/Microsoft.Interop.SourceGeneration/ManualTypeMarshallingHelper.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,8 @@ public static class ManualTypeMarshallingHelper
{
public const string ValuePropertyName = "Value";
public const string GetPinnableReferenceName = "GetPinnableReference";
public const string StackBufferSizeFieldName = "StackBufferSize";
public const string BufferSizeFieldName = "BufferSize";
public const string RequiresStackBufferFieldName = "RequiresStackBuffer";
public const string ToManagedMethodName = "ToManaged";
public const string FreeNativeMethodName = "FreeNative";
public const string ManagedValuesPropertyName = "ManagedValues";
Expand Down Expand Up @@ -57,7 +58,7 @@ public static bool IsManagedToNativeConstructor(
&& SymbolEqualityComparer.Default.Equals(managedType, ctor.Parameters[0].Type);
}

public static bool IsStackallocConstructor(
public static bool IsCallerAllocatedSpanConstructor(
IMethodSymbol ctor,
ITypeSymbol managedType,
ITypeSymbol spanOfByte,
Expand Down
Loading