[release/8.0-staging] [LibraryImportGenerator] Use basic forwarder in down-level support if any parameters can't be marshalled (#104501)

Author: elinor-fung

src/libraries/System.Net.Http.WinHttpHandler/src/System.Net.Http.WinHttpHandler.csproj

@@ -5,7 +5,7 @@
     <AllowUnsafeBlocks>true</AllowUnsafeBlocks>
     <IsPackable>true</IsPackable>
     <GeneratePackageOnBuild>true</GeneratePackageOnBuild>
-    <ServicingVersion>1</ServicingVersion>
+    <ServicingVersion>2</ServicingVersion>
     <PackageDescription>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:

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

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<IMethodSymbol, Compilation> targetPInvokeAssertion, TestTargetFramework targetFramework = TestTargetFramework.Net)
         {
             var test = new GeneratedTargetPInvokeTest(targetPInvokeAssertion, targetFramework)

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 = "") => $$"""
+        /// <summary>
+        /// Declaration with a non-blittable parameter that is always supported for marshalling
+        /// </summary>
+        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);
             }
             """;
 

src/libraries/System.Runtime.InteropServices/tests/LibraryImportGenerator.UnitTests/Compiles.cs

@@ -517,9 +517,32 @@ public static IEnumerable<object[]> 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 };

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) { }