Skip to content
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

Jump to bottom

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

Merged
merged 4 commits into from
Jul 12, 2024
Merged

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

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
df8f341
[LibraryImportGenerator] Use basic forwarder in down-level support if…
elinor-fung Jul 4, 2024
a1f5f23
Add tests
elinor-fung Jul 4, 2024
f546ec7
Remove tests using partial forwarding
elinor-fung Jul 4, 2024
5911443
Update servicing version for System.Net.Http.WinHttpHandler
elinor-fung Jul 8, 2024
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
15 changes: 14 additions & 1 deletion ...ies/System.Runtime.InteropServices/gen/LibraryImportGenerator/PInvokeStubCodeGenerator.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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
Expand Down
74 changes: 0 additions & 74 deletions ...tem.Runtime.InteropServices/tests/LibraryImportGenerator.UnitTests/AttributeForwarding.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
7 changes: 5 additions & 2 deletions ...ies/System.Runtime.InteropServices/tests/LibraryImportGenerator.UnitTests/CodeSnippets.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
""";

Expand Down
27 changes: 25 additions & 2 deletions ...braries/System.Runtime.InteropServices/tests/LibraryImportGenerator.UnitTests/Compiles.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 };
Expand Down
14 changes: 10 additions & 4 deletions ...ries/System.Runtime.InteropServices/tests/LibraryImportGenerator.UnitTests/Diagnostics.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand All @@ -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)
Expand All @@ -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) { }
Expand Down
Loading