From df8f34177e20edbceea66e632ff3c5f25bbf082f Mon Sep 17 00:00:00 2001 From: Elinor Fung Date: Wed, 3 Jul 2024 20:52:43 -0700 Subject: [PATCH 1/4] [LibraryImportGenerator] Use basic forwarder in down-level support if any parameters can't be marshalled --- .../PInvokeStubCodeGenerator.cs | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/src/libraries/System.Runtime.InteropServices/gen/LibraryImportGenerator/PInvokeStubCodeGenerator.cs b/src/libraries/System.Runtime.InteropServices/gen/LibraryImportGenerator/PInvokeStubCodeGenerator.cs index 939a623ab7dba8..2bf4202cec1837 100644 --- a/src/libraries/System.Runtime.InteropServices/gen/LibraryImportGenerator/PInvokeStubCodeGenerator.cs +++ b/src/libraries/System.Runtime.InteropServices/gen/LibraryImportGenerator/PInvokeStubCodeGenerator.cs @@ -80,7 +80,7 @@ public PInvokeStubCodeGenerator( } bool noMarshallingNeeded = true; - + bool hasForwardedTypes = false; foreach (BoundGenerator generator in _marshallers.SignatureMarshallers) { // Check if marshalling info and generator support the current target framework. @@ -90,6 +90,19 @@ public PInvokeStubCodeGenerator( // Check if generator is either blittable or just a forwarder. noMarshallingNeeded &= generator is { Generator: BlittableMarshaller, TypeInfo.IsByRef: false } or { Generator: Forwarder }; + + // Track if any generators are just forwarders - for types other than void, this indicates + // types that can't be marshalled by the source generated. + // In .NET 7+ support, we would have emitted a diagnostic error about lack of support + // In down-level support, we do not error - tracking this allows us to switch to generating a basic forwarder (DllImport declaration) + hasForwardedTypes |= generator is { Generator: Forwarder, TypeInfo.ManagedType: not SpecialTypeInfo { SpecialType: Microsoft.CodeAnalysis.SpecialType.System_Void } }; + } + + // For down-level support, if some parameters cannot be marshalled, consider the target framework as not supported + if (hasForwardedTypes + && (targetFramework != TargetFramework.Net || targetFrameworkVersion.Major < 7)) + { + SupportsTargetFramework = false; } StubIsBasicForwarder = !setLastError From a1f5f2306fad565c782d47705467767cdc7c2949 Mon Sep 17 00:00:00 2001 From: Elinor Fung Date: Wed, 3 Jul 2024 22:14:18 -0700 Subject: [PATCH 2/4] Add tests --- .../CodeSnippets.cs | 7 +++-- .../Compiles.cs | 27 +++++++++++++++++-- 2 files changed, 30 insertions(+), 4 deletions(-) diff --git a/src/libraries/System.Runtime.InteropServices/tests/LibraryImportGenerator.UnitTests/CodeSnippets.cs b/src/libraries/System.Runtime.InteropServices/tests/LibraryImportGenerator.UnitTests/CodeSnippets.cs index 19832861a38aaa..ddcd1a4bed16fc 100644 --- a/src/libraries/System.Runtime.InteropServices/tests/LibraryImportGenerator.UnitTests/CodeSnippets.cs +++ b/src/libraries/System.Runtime.InteropServices/tests/LibraryImportGenerator.UnitTests/CodeSnippets.cs @@ -756,13 +756,16 @@ partial class Test } """; - public static string BasicReturnAndParameterByValue(string returnType, string parameterType, string preDeclaration = "") => $$""" + /// + /// Declaration with a non-blittable parameter that is always supported for marshalling + /// + public static string BasicReturnAndParameterWithAlwaysSupportedParameter(string returnType, string parameterType, string preDeclaration = "") => $$""" using System.Runtime.InteropServices; {{preDeclaration}} partial class Test { [LibraryImport("DoesNotExist")] - public static partial {{returnType}} Method({{parameterType}} p); + public static partial {{returnType}} Method({{parameterType}} p, out int i); } """; diff --git a/src/libraries/System.Runtime.InteropServices/tests/LibraryImportGenerator.UnitTests/Compiles.cs b/src/libraries/System.Runtime.InteropServices/tests/LibraryImportGenerator.UnitTests/Compiles.cs index 5d37fffd52a5b6..eade927f6a4cbb 100644 --- a/src/libraries/System.Runtime.InteropServices/tests/LibraryImportGenerator.UnitTests/Compiles.cs +++ b/src/libraries/System.Runtime.InteropServices/tests/LibraryImportGenerator.UnitTests/Compiles.cs @@ -517,9 +517,32 @@ public static IEnumerable CodeSnippetsToValidateFallbackForwarder() yield return new object[] { ID(), code, TestTargetFramework.Framework, true }; } - // Confirm that if support is missing for any type (like arrays), we fall back to a forwarder even if other types are supported. + // Confirm that if support is missing for a type with a ITypeBasedMarshallingInfoProvider (like arrays), we fall back to a forwarder even if other types are supported. { - string code = CodeSnippets.BasicReturnAndParameterByValue("System.Runtime.InteropServices.SafeHandle", "int[]", CodeSnippets.LibraryImportAttributeDeclaration); + string code = CodeSnippets.BasicReturnAndParameterWithAlwaysSupportedParameter("void", "int[]", CodeSnippets.LibraryImportAttributeDeclaration); + yield return new object[] { ID(), code, TestTargetFramework.Net6, true }; + yield return new object[] { ID(), code, TestTargetFramework.Core, true }; + yield return new object[] { ID(), code, TestTargetFramework.Standard, true }; + yield return new object[] { ID(), code, TestTargetFramework.Framework, true }; + } + { + string code = CodeSnippets.BasicReturnAndParameterWithAlwaysSupportedParameter("int", "int[]", CodeSnippets.LibraryImportAttributeDeclaration); + yield return new object[] { ID(), code, TestTargetFramework.Net6, true }; + yield return new object[] { ID(), code, TestTargetFramework.Core, true }; + yield return new object[] { ID(), code, TestTargetFramework.Standard, true }; + yield return new object[] { ID(), code, TestTargetFramework.Framework, true }; + } + + // Confirm that if support is missing for a type without a ITypeBasedMarshallingInfoProvider (like StringBuilder), we fall back to a forwarder even if other types are supported. + { + string code = CodeSnippets.BasicReturnAndParameterWithAlwaysSupportedParameter("void", "System.Text.StringBuilder", CodeSnippets.LibraryImportAttributeDeclaration); + yield return new object[] { ID(), code, TestTargetFramework.Net6, true }; + yield return new object[] { ID(), code, TestTargetFramework.Core, true }; + yield return new object[] { ID(), code, TestTargetFramework.Standard, true }; + yield return new object[] { ID(), code, TestTargetFramework.Framework, true }; + } + { + string code = CodeSnippets.BasicReturnAndParameterWithAlwaysSupportedParameter("int", "System.Text.StringBuilder", CodeSnippets.LibraryImportAttributeDeclaration); yield return new object[] { ID(), code, TestTargetFramework.Net6, true }; yield return new object[] { ID(), code, TestTargetFramework.Core, true }; yield return new object[] { ID(), code, TestTargetFramework.Standard, true }; From f546ec7df3b47e1f449a2485ec41c79db8853a2c Mon Sep 17 00:00:00 2001 From: Elinor Fung Date: Wed, 3 Jul 2024 23:27:04 -0700 Subject: [PATCH 3/4] Remove tests using partial forwarding --- .../AttributeForwarding.cs | 74 ------------------- .../Diagnostics.cs | 14 +++- 2 files changed, 10 insertions(+), 78 deletions(-) diff --git a/src/libraries/System.Runtime.InteropServices/tests/LibraryImportGenerator.UnitTests/AttributeForwarding.cs b/src/libraries/System.Runtime.InteropServices/tests/LibraryImportGenerator.UnitTests/AttributeForwarding.cs index 1a5322bd859565..010fd97a6abfda 100644 --- a/src/libraries/System.Runtime.InteropServices/tests/LibraryImportGenerator.UnitTests/AttributeForwarding.cs +++ b/src/libraries/System.Runtime.InteropServices/tests/LibraryImportGenerator.UnitTests/AttributeForwarding.cs @@ -331,80 +331,6 @@ await VerifySourceGeneratorAsync( }); } - [Fact] - [OuterLoop("Uses the network for downlevel ref packs")] - public async Task InOutAttributes_Forwarded_To_ForwardedParameter() - { - // This code is invalid configuration from the source generator's perspective. - // We just use it as validation for forwarding the In and Out attributes. - string source = """ - using System.Runtime.CompilerServices; - using System.Runtime.InteropServices; - partial class C - { - [LibraryImportAttribute("DoesNotExist")] - [return: MarshalAs(UnmanagedType.Bool)] - public static partial bool Method1([In, Out] int {|SYSLIB1051:a|}); - } - """ + CodeSnippets.LibraryImportAttributeDeclaration; - - await VerifySourceGeneratorAsync( - source, - (targetMethod, newComp) => - { - INamedTypeSymbol marshalAsAttribute = newComp.GetTypeByMetadataName(TypeNames.System_Runtime_InteropServices_MarshalAsAttribute)!; - INamedTypeSymbol inAttribute = newComp.GetTypeByMetadataName(TypeNames.System_Runtime_InteropServices_InAttribute)!; - INamedTypeSymbol outAttribute = newComp.GetTypeByMetadataName(TypeNames.System_Runtime_InteropServices_OutAttribute)!; - Assert.Collection(targetMethod.Parameters, - param => Assert.Collection(param.GetAttributes(), - attr => - { - Assert.Equal(inAttribute, attr.AttributeClass, SymbolEqualityComparer.Default); - Assert.Empty(attr.ConstructorArguments); - Assert.Empty(attr.NamedArguments); - }, - attr => - { - Assert.Equal(outAttribute, attr.AttributeClass, SymbolEqualityComparer.Default); - Assert.Empty(attr.ConstructorArguments); - Assert.Empty(attr.NamedArguments); - })); - }, - TestTargetFramework.Standard); - } - - [Fact] - [OuterLoop("Uses the network for downlevel ref packs")] - public async Task MarshalAsAttribute_Forwarded_To_ForwardedParameter() - { - string source = """ - using System.Runtime.CompilerServices; - using System.Runtime.InteropServices; - partial class C - { - [LibraryImportAttribute("DoesNotExist")] - [return: MarshalAs(UnmanagedType.Bool)] - public static partial bool Method1([MarshalAs(UnmanagedType.I2)] int a); - } - """ + CodeSnippets.LibraryImportAttributeDeclaration; - - await VerifySourceGeneratorAsync( - source, - (targetMethod, newComp) => - { - INamedTypeSymbol marshalAsAttribute = newComp.GetTypeByMetadataName(TypeNames.System_Runtime_InteropServices_MarshalAsAttribute)!; - Assert.Collection(targetMethod.Parameters, - param => Assert.Collection(param.GetAttributes(), - attr => - { - Assert.Equal(marshalAsAttribute, attr.AttributeClass, SymbolEqualityComparer.Default); - Assert.Equal(UnmanagedType.I2, (UnmanagedType)attr.ConstructorArguments[0].Value!); - Assert.Empty(attr.NamedArguments); - })); - }, - TestTargetFramework.Standard); - } - private static Task VerifySourceGeneratorAsync(string source, Action targetPInvokeAssertion, TestTargetFramework targetFramework = TestTargetFramework.Net) { var test = new GeneratedTargetPInvokeTest(targetPInvokeAssertion, targetFramework) diff --git a/src/libraries/System.Runtime.InteropServices/tests/LibraryImportGenerator.UnitTests/Diagnostics.cs b/src/libraries/System.Runtime.InteropServices/tests/LibraryImportGenerator.UnitTests/Diagnostics.cs index aeb467e4bc77c3..646021b0b1adfc 100644 --- a/src/libraries/System.Runtime.InteropServices/tests/LibraryImportGenerator.UnitTests/Diagnostics.cs +++ b/src/libraries/System.Runtime.InteropServices/tests/LibraryImportGenerator.UnitTests/Diagnostics.cs @@ -250,7 +250,7 @@ partial class Test public static partial void {|#0:Method1|}(string s); [LibraryImport("DoesNotExist", StringMarshalling = StringMarshalling.Custom, StringMarshallingCustomType = typeof(Native))] - public static partial void Method2(string {|#1:s|}); + public static partial void {|#2:Method2|}(string {|#1:s|}); struct Native { @@ -266,7 +266,13 @@ public Native(string s) { } .WithArguments($"{nameof(TypeNames.LibraryImportAttribute)}{Type.Delimiter}{nameof(StringMarshalling)}={nameof(StringMarshalling)}{Type.Delimiter}{nameof(StringMarshalling.Utf8)}"), VerifyCS.Diagnostic(GeneratorDiagnostics.ParameterTypeNotSupportedWithDetails) .WithLocation(1) - .WithArguments("Marshalling string or char without explicit marshalling information is not supported. Specify 'LibraryImportAttribute.StringMarshalling', 'LibraryImportAttribute.StringMarshallingCustomType', 'MarshalUsingAttribute' or 'MarshalAsAttribute'.", "s") + .WithArguments("Marshalling string or char without explicit marshalling information is not supported. Specify 'LibraryImportAttribute.StringMarshalling', 'LibraryImportAttribute.StringMarshallingCustomType', 'MarshalUsingAttribute' or 'MarshalAsAttribute'.", "s"), + VerifyCS.Diagnostic(GeneratorDiagnostics.CannotForwardToDllImport) + .WithLocation(2) + .WithArguments($"{nameof(TypeNames.LibraryImportAttribute)}{Type.Delimiter}{nameof(StringMarshalling)}={nameof(StringMarshalling)}{Type.Delimiter}{nameof(StringMarshalling.Custom)}"), + VerifyCS.Diagnostic(GeneratorDiagnostics.CannotForwardToDllImport) + .WithLocation(2) + .WithArguments($"{nameof(TypeNames.LibraryImportAttribute)}{Type.Delimiter}{nameof(LibraryImportAttribute.StringMarshallingCustomType)}") }; var test = new VerifyCS.Test(TestTargetFramework.Standard) @@ -289,10 +295,10 @@ partial class Test { [{|#0:LibraryImport("DoesNotExist", StringMarshalling = StringMarshalling.Custom)|}] public static partial void Method1(out int i); - + [{|#1:LibraryImport("DoesNotExist", StringMarshalling = StringMarshalling.Utf8, StringMarshallingCustomType = typeof(Native))|}] public static partial void Method2(out int i); - + struct Native { public Native(string s) { } From 5911443210736f1e7c2a189d2d5d95974c47e74c Mon Sep 17 00:00:00 2001 From: Elinor Fung Date: Mon, 8 Jul 2024 08:32:08 -0700 Subject: [PATCH 4/4] Update servicing version for System.Net.Http.WinHttpHandler --- .../src/System.Net.Http.WinHttpHandler.csproj | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/System.Net.Http.WinHttpHandler/src/System.Net.Http.WinHttpHandler.csproj b/src/libraries/System.Net.Http.WinHttpHandler/src/System.Net.Http.WinHttpHandler.csproj index 21b2b54e3cbecf..4e4b50727b7fa8 100644 --- a/src/libraries/System.Net.Http.WinHttpHandler/src/System.Net.Http.WinHttpHandler.csproj +++ b/src/libraries/System.Net.Http.WinHttpHandler/src/System.Net.Http.WinHttpHandler.csproj @@ -5,7 +5,7 @@ true true true - 1 + 2 Provides a message handler for HttpClient based on the WinHTTP interface of Windows. While similar to HttpClientHandler, it provides developers more granular control over the application's HTTP communication than the HttpClientHandler. Commonly Used Types: