From e0a6d009938ff09483472bbc9426816c04849898 Mon Sep 17 00:00:00 2001 From: Jeremy Koritzinsky Date: Mon, 3 May 2021 11:12:29 -0700 Subject: [PATCH 1/3] Add support for abstract SafeHandle types for by-value marshalling. --- .../DllImportGenerator.UnitTests/CodeSnippets.cs | 9 +++++++++ .../DllImportGenerator.UnitTests/Compiles.cs | 1 + .../Marshalling/MarshallingGenerator.cs | 4 ++++ .../DllImportGenerator/TypePositionInfo.cs | 5 ++--- 4 files changed, 16 insertions(+), 3 deletions(-) diff --git a/DllImportGenerator/DllImportGenerator.UnitTests/CodeSnippets.cs b/DllImportGenerator/DllImportGenerator.UnitTests/CodeSnippets.cs index 8d630f9c2b62..eb4cc83858fe 100644 --- a/DllImportGenerator/DllImportGenerator.UnitTests/CodeSnippets.cs +++ b/DllImportGenerator/DllImportGenerator.UnitTests/CodeSnippets.cs @@ -843,6 +843,15 @@ public static partial void Method( {byRefKind} {typeName} p); }}"; + public static string BasicParameterByValue(string typeName) => @$" +using System.Runtime.InteropServices; +partial class Test +{{ + [GeneratedDllImport(""DoesNotExist"")] + public static partial void Method( + {typeName} p); +}}"; + public static string BasicReturnType(string typeName) => @$" using System.Runtime.InteropServices; partial class Test diff --git a/DllImportGenerator/DllImportGenerator.UnitTests/Compiles.cs b/DllImportGenerator/DllImportGenerator.UnitTests/Compiles.cs index 60d61de1d0b5..c9da24ff41a4 100644 --- a/DllImportGenerator/DllImportGenerator.UnitTests/Compiles.cs +++ b/DllImportGenerator/DllImportGenerator.UnitTests/Compiles.cs @@ -135,6 +135,7 @@ public static IEnumerable CodeSnippetsToCompile() // SafeHandle yield return new[] { CodeSnippets.BasicParametersAndModifiers("Microsoft.Win32.SafeHandles.SafeFileHandle") }; + yield return new[] { CodeSnippets.BasicParameterByValue("System.Runtime.InteropServices.SafeHandle") }; yield return new[] { CodeSnippets.SafeHandleWithCustomDefaultConstructorAccessibility(privateCtor: false) }; yield return new[] { CodeSnippets.SafeHandleWithCustomDefaultConstructorAccessibility(privateCtor: true) }; diff --git a/DllImportGenerator/DllImportGenerator/Marshalling/MarshallingGenerator.cs b/DllImportGenerator/DllImportGenerator/Marshalling/MarshallingGenerator.cs index e9d81561b75a..8bf4e24a7d67 100644 --- a/DllImportGenerator/DllImportGenerator/Marshalling/MarshallingGenerator.cs +++ b/DllImportGenerator/DllImportGenerator/Marshalling/MarshallingGenerator.cs @@ -237,6 +237,10 @@ private static IMarshallingGenerator CreateCore( { throw new MarshallingNotSupportedException(info, context); } + if (info.IsByRef && info.ManagedType.IsAbstract) + { + throw new MarshallingNotSupportedException(info, context); + } return new SafeHandleMarshaller(options); // Marshalling in new model. diff --git a/DllImportGenerator/DllImportGenerator/TypePositionInfo.cs b/DllImportGenerator/DllImportGenerator/TypePositionInfo.cs index 9cb216590cc0..0a5d4c921b22 100644 --- a/DllImportGenerator/DllImportGenerator/TypePositionInfo.cs +++ b/DllImportGenerator/DllImportGenerator/TypePositionInfo.cs @@ -312,11 +312,10 @@ static bool TryCreateTypeBasedMarshallingInfo(ITypeSymbol type, DefaultMarshalli var conversion = compilation.ClassifyCommonConversion(type, compilation.GetTypeByMetadataName(TypeNames.System_Runtime_InteropServices_SafeHandle)!); if (conversion.Exists && conversion.IsImplicit - && conversion.IsReference - && !type.IsAbstract) + && (conversion.IsReference || conversion.IsIdentity)) { bool hasAccessibleDefaultConstructor = false; - if (type is INamedTypeSymbol named && named.InstanceConstructors.Length > 0) + if (type is INamedTypeSymbol named && !named.IsAbstract && named.InstanceConstructors.Length > 0) { foreach (var ctor in named.InstanceConstructors) { From 899f7ea61c740ffe46baefaf53fd8817cee91422 Mon Sep 17 00:00:00 2001 From: Jeremy Koritzinsky Date: Tue, 4 May 2021 10:29:23 -0700 Subject: [PATCH 2/3] Add test for byref abstract SafeHandle. --- .../DllImportGenerator.UnitTests/CompileFails.cs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/DllImportGenerator/DllImportGenerator.UnitTests/CompileFails.cs b/DllImportGenerator/DllImportGenerator.UnitTests/CompileFails.cs index 61f98f45fff8..89622d1cdf23 100644 --- a/DllImportGenerator/DllImportGenerator.UnitTests/CompileFails.cs +++ b/DllImportGenerator/DllImportGenerator.UnitTests/CompileFails.cs @@ -92,6 +92,9 @@ public static IEnumerable CodeSnippetsToCompile() // Custom type marshalling in arrays (complex case with Value property) yield return new object[] { CodeSnippets.ArrayMarshallingWithCustomStructElementWithValueProperty, 5, 0 }; + + // Abstract SafeHandle type by reference + yield return new object[] { CodeSnippets.BasicParameterWithByRefModifier("ref", "System.Runtime.InteropServices.SafeHandle"), 1, 0 }; } [Theory] From 4c1c4d3501a939c75af1a448e639d69351f5e608 Mon Sep 17 00:00:00 2001 From: Jeremy Koritzinsky Date: Tue, 4 May 2021 10:32:57 -0700 Subject: [PATCH 3/3] Add abstract by-ref test. Add details for failure case. --- .../Marshalling/MarshallingGenerator.cs | 5 ++++- .../DllImportGenerator/Resources.Designer.cs | 9 +++++++++ DllImportGenerator/DllImportGenerator/Resources.resx | 3 +++ 3 files changed, 16 insertions(+), 1 deletion(-) diff --git a/DllImportGenerator/DllImportGenerator/Marshalling/MarshallingGenerator.cs b/DllImportGenerator/DllImportGenerator/Marshalling/MarshallingGenerator.cs index 8bf4e24a7d67..c8f13da707c3 100644 --- a/DllImportGenerator/DllImportGenerator/Marshalling/MarshallingGenerator.cs +++ b/DllImportGenerator/DllImportGenerator/Marshalling/MarshallingGenerator.cs @@ -239,7 +239,10 @@ private static IMarshallingGenerator CreateCore( } if (info.IsByRef && info.ManagedType.IsAbstract) { - throw new MarshallingNotSupportedException(info, context); + throw new MarshallingNotSupportedException(info, context) + { + NotSupportedDetails = Resources.SafeHandleByRefMustBeConcrete + }; } return new SafeHandleMarshaller(options); diff --git a/DllImportGenerator/DllImportGenerator/Resources.Designer.cs b/DllImportGenerator/DllImportGenerator/Resources.Designer.cs index e86940b256c6..87fac1a3e0ff 100644 --- a/DllImportGenerator/DllImportGenerator/Resources.Designer.cs +++ b/DllImportGenerator/DllImportGenerator/Resources.Designer.cs @@ -483,6 +483,15 @@ internal static string RefValuePropertyUnsupportedMessage { } } + /// + /// Looks up a localized string similar to An abstract type derived from 'SafeHandle' cannot be marshalled by reference. The provided type must be concrete.. + /// + internal static string SafeHandleByRefMustBeConcrete { + get { + return ResourceManager.GetString("SafeHandleByRefMustBeConcrete", resourceCulture); + } + } + /// /// Looks up a localized string similar to When constructor taking a Span<byte> 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.. /// diff --git a/DllImportGenerator/DllImportGenerator/Resources.resx b/DllImportGenerator/DllImportGenerator/Resources.resx index c6a549149339..f66d2945ad7a 100644 --- a/DllImportGenerator/DllImportGenerator/Resources.resx +++ b/DllImportGenerator/DllImportGenerator/Resources.resx @@ -259,6 +259,9 @@ The 'Value' property on the native type '{0}' must not be a 'ref' or 'readonly ref' property. + + An abstract type derived from 'SafeHandle' cannot be marshalled by reference. The provided type must be concrete. + When constructor taking a Span<byte> 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.