diff --git a/src/libraries/System.Runtime.InteropServices/gen/DllImportGenerator/TypePositionInfo.cs b/src/libraries/System.Runtime.InteropServices/gen/DllImportGenerator/TypePositionInfo.cs index 06d6040ea3c07..8d23b274ee3a0 100644 --- a/src/libraries/System.Runtime.InteropServices/gen/DllImportGenerator/TypePositionInfo.cs +++ b/src/libraries/System.Runtime.InteropServices/gen/DllImportGenerator/TypePositionInfo.cs @@ -153,7 +153,12 @@ private static MarshallingInfo GetMarshallingInfo(ITypeSymbol type, IEnumerable< if (SymbolEqualityComparer.Default.Equals(compilation.GetTypeByMetadataName(TypeNames.BlittableTypeAttribute), attributeClass)) { - return new BlittableTypeAttributeInfo(); + // If type is generic, then we need to re-evaluate that it is blittable at usage time. + if (type is INamedTypeSymbol { IsGenericType: false } || type.HasOnlyBlittableFields()) + { + return new BlittableTypeAttributeInfo(); + } + break; } else if (SymbolEqualityComparer.Default.Equals(compilation.GetTypeByMetadataName(TypeNames.NativeMarshallingAttribute), attributeClass)) { @@ -335,6 +340,16 @@ static bool TryCreateTypeBasedMarshallingInfo(ITypeSymbol type, DefaultMarshalli marshallingInfo = new ArrayMarshallingInfo(GetMarshallingInfo(elementType, Array.Empty(), defaultInfo, compilation, diagnostics, scopeSymbol)); return true; } + + if (type is INamedTypeSymbol { IsValueType: true } valueType + && !valueType.IsExposedOutsideOfCurrentCompilation() + && valueType.IsConsideredBlittable()) + { + // Allow implicit [BlittableType] on internal value types. + marshallingInfo = new BlittableTypeAttributeInfo(); + return true; + } + marshallingInfo = NoMarshallingInfo.Instance; return false; } diff --git a/src/libraries/System.Runtime.InteropServices/gen/DllImportGenerator/TypeSymbolExtensions.cs b/src/libraries/System.Runtime.InteropServices/gen/DllImportGenerator/TypeSymbolExtensions.cs index 745f861f61019..4495704b3dccc 100644 --- a/src/libraries/System.Runtime.InteropServices/gen/DllImportGenerator/TypeSymbolExtensions.cs +++ b/src/libraries/System.Runtime.InteropServices/gen/DllImportGenerator/TypeSymbolExtensions.cs @@ -13,34 +13,44 @@ namespace Microsoft.Interop { static class TypeSymbolExtensions { - public static bool HasOnlyBlittableFields(this ITypeSymbol type) + public static bool HasOnlyBlittableFields(this ITypeSymbol type) => HasOnlyBlittableFields(type, new HashSet(SymbolEqualityComparer.Default)); + + private static bool HasOnlyBlittableFields(this ITypeSymbol type, HashSet seenTypes) { + if (seenTypes.Contains(type)) + { + // A recursive struct type isn't blittable. + // It's also illegal in C#, but I believe that source generators run + // before that is detected, so we check here to avoid a stack overflow. + return false; + } + seenTypes.Add(type); foreach (var field in type.GetMembers().OfType()) { - bool? fieldBlittable = field switch + if (!field.IsStatic) { - { IsStatic : true } => null, - { Type : { IsReferenceType : true }} => false, - { Type : IPointerTypeSymbol ptr } => IsConsideredBlittable(ptr.PointedAtType), - { Type : IFunctionPointerTypeSymbol } => true, - not { Type : { SpecialType : SpecialType.None }} => IsSpecialTypeBlittable(field.Type.SpecialType), - // Assume that type parameters that can be blittable are blittable. - // We'll re-evaluate blittability for generic fields of generic types at instantation time. - { Type : ITypeParameterSymbol } => true, - { Type : { IsValueType : false }} => false, - // A recursive struct type isn't blittable. - // It's also illegal in C#, but I believe that source generators run - // before that is detected, so we check here to avoid a stack overflow. - // [TODO]: Handle mutual recursion. - _ => !SymbolEqualityComparer.Default.Equals(field.Type, type) && IsConsideredBlittable(field.Type) - }; - - if (fieldBlittable is false) - { - return false; + bool fieldBlittable = field switch + { + { Type: { IsReferenceType: true } } => false, + { Type: IPointerTypeSymbol ptr } => IsConsideredBlittable(ptr.PointedAtType), + { Type: IFunctionPointerTypeSymbol } => true, + not { Type: { SpecialType: SpecialType.None } } => IsSpecialTypeBlittable(field.Type.SpecialType), + // Assume that type parameters that can be blittable are blittable. + // We'll re-evaluate blittability for generic fields of generic types at instantation time. + { Type: ITypeParameterSymbol } => true, + { Type: { IsValueType: false } } => false, + _ => IsConsideredBlittable(field.Type, seenTypes) + }; + + if (!fieldBlittable) + { + seenTypes.Remove(type); + return false; + } } } + seenTypes.Remove(type); return true; } @@ -62,14 +72,16 @@ or SpecialType.System_IntPtr _ => false }; - public static bool IsConsideredBlittable(this ITypeSymbol type) + public static bool IsConsideredBlittable(this ITypeSymbol type) => IsConsideredBlittable(type, new HashSet(SymbolEqualityComparer.Default)); + + private static bool IsConsideredBlittable(this ITypeSymbol type, HashSet seenTypes) { if (type.SpecialType != SpecialType.None) { return IsSpecialTypeBlittable(type.SpecialType); } - if (type.TypeKind == TypeKind.FunctionPointer) + if (type.TypeKind is TypeKind.FunctionPointer or TypeKind.Pointer) { return true; } @@ -79,14 +91,9 @@ public static bool IsConsideredBlittable(this ITypeSymbol type) return false; } - if (type is IPointerTypeSymbol { PointedAtType: ITypeSymbol pointedAtType }) - { - return pointedAtType.IsConsideredBlittable(); - } - if (type is INamedTypeSymbol { TypeKind: TypeKind.Enum, EnumUnderlyingType: ITypeSymbol underlyingType }) { - return underlyingType!.IsConsideredBlittable(); + return underlyingType.IsConsideredBlittable(seenTypes); } bool hasNativeMarshallingAttribute = false; @@ -107,7 +114,7 @@ public static bool IsConsideredBlittable(this ITypeSymbol type) // since we are guaranteed that if a type has generic fields, // they will be present in the contract assembly to ensure // that recursive structs can be identified at build time. - return generic.HasOnlyBlittableFields(); + return generic.HasOnlyBlittableFields(seenTypes); } return true; } @@ -126,7 +133,16 @@ public static bool IsConsideredBlittable(this ITypeSymbol type) // The struct type has generated marshalling via a source generator. // We can't guarantee that we can see the results of the struct source generator, // so we re-calculate if the type is blittable here. - return type.HasOnlyBlittableFields(); + return type.HasOnlyBlittableFields(seenTypes); + } + + if (type is INamedTypeSymbol namedType + && namedType.DeclaringSyntaxReferences.Length != 0 + && !namedType.IsExposedOutsideOfCurrentCompilation()) + { + // If a type is declared in the current compilation and not exposed outside of it, + // we will allow it to be considered blittable if its fields are considered blittable. + return type.HasOnlyBlittableFields(seenTypes); } return false; } @@ -165,5 +181,19 @@ or SpecialType.System_IntPtr _ => false }; } + + public static bool IsExposedOutsideOfCurrentCompilation(this INamedTypeSymbol type) + { + for (; type is not null; type = type.ContainingType) + { + Accessibility accessibility = type.DeclaredAccessibility; + + if (accessibility is Accessibility.Internal or Accessibility.ProtectedAndInternal or Accessibility.Private or Accessibility.Friend or Accessibility.ProtectedAndFriend) + { + return false; + } + } + return true; + } } } diff --git a/src/libraries/System.Runtime.InteropServices/tests/DllImportGenerator.UnitTests/CodeSnippets.cs b/src/libraries/System.Runtime.InteropServices/tests/DllImportGenerator.UnitTests/CodeSnippets.cs index 8d74efa7ea8e6..b89acc1f63dbc 100644 --- a/src/libraries/System.Runtime.InteropServices/tests/DllImportGenerator.UnitTests/CodeSnippets.cs +++ b/src/libraries/System.Runtime.InteropServices/tests/DllImportGenerator.UnitTests/CodeSnippets.cs @@ -511,14 +511,6 @@ struct MyStruct private int i; private short s; }"; - public static string GenericBlittableStructParametersAndModifiers = BasicParametersAndModifiers("MyStruct") + @" -#pragma warning disable CS0169 -[BlittableType] -struct MyStruct -{ - private T t; - private short s; -}"; public static string ArrayParametersAndModifiers(string elementType) => $@" using System.Runtime.InteropServices; @@ -1040,5 +1032,56 @@ public static partial int Method2( #endif public static int Foo() => throw null; }}"; + + public static string MaybeBlittableGenericTypeParametersAndModifiers(string typeArgument) => BasicParametersAndModifiers($"Generic<{typeArgument}>") + @" +[BlittableType] +struct Generic +{ +#pragma warning disable CS0649 + public T field; +} +"; + + public static string MaybeBlittableGenericTypeParametersAndModifiers() => + MaybeBlittableGenericTypeParametersAndModifiers(typeof(T).ToString()); + + public static string ImplicitlyBlittableStructParametersAndModifiers(string visibility = "") => BasicParametersAndModifiers("ImplicitBlittable") + $@" +#pragma warning disable CS0649 +#pragma warning disable CS0169 +{visibility} struct ImplicitBlittable +{{ + int i; +}}"; + + public static string ImplicitlyBlittableGenericTypeParametersAndModifiers(string typeArgument, string visibility = "") => BasicParametersAndModifiers($"Generic<{typeArgument}>") + $@" +{visibility} struct Generic +{{ +#pragma warning disable CS0649 +#pragma warning disable CS0169 + public T field; +}} +"; + + public static string ImplicitlyBlittableGenericTypeParametersAndModifiers(string visibility = "") => + ImplicitlyBlittableGenericTypeParametersAndModifiers(typeof(T).ToString(), visibility); + + public static string RecursiveImplicitlyBlittableStruct => BasicParametersAndModifiers("RecursiveStruct") + @" +struct RecursiveStruct +{ + RecursiveStruct s; + int i; +}"; + public static string MutuallyRecursiveImplicitlyBlittableStruct => BasicParametersAndModifiers("RecursiveStruct1") + @" +struct RecursiveStruct1 +{ + RecursiveStruct2 s; + int i; +} + +struct RecursiveStruct2 +{ + RecursiveStruct1 s; + int i; +}"; } } diff --git a/src/libraries/System.Runtime.InteropServices/tests/DllImportGenerator.UnitTests/CompileFails.cs b/src/libraries/System.Runtime.InteropServices/tests/DllImportGenerator.UnitTests/CompileFails.cs index 89622d1cdf233..19736372a2535 100644 --- a/src/libraries/System.Runtime.InteropServices/tests/DllImportGenerator.UnitTests/CompileFails.cs +++ b/src/libraries/System.Runtime.InteropServices/tests/DllImportGenerator.UnitTests/CompileFails.cs @@ -95,6 +95,15 @@ public static IEnumerable CodeSnippetsToCompile() // Abstract SafeHandle type by reference yield return new object[] { CodeSnippets.BasicParameterWithByRefModifier("ref", "System.Runtime.InteropServices.SafeHandle"), 1, 0 }; + + // Non-blittable instantiation of generic type + yield return new object[] { CodeSnippets.MaybeBlittableGenericTypeParametersAndModifiers(), 5, 0 }; + + // No marshalling annotations + + yield return new object[] { CodeSnippets.ImplicitlyBlittableStructParametersAndModifiers("public"), 5, 0 }; + yield return new object[] { CodeSnippets.ImplicitlyBlittableGenericTypeParametersAndModifiers(), 5, 0 }; + yield return new object[] { CodeSnippets.ImplicitlyBlittableGenericTypeParametersAndModifiers("public"), 5, 0 }; } [Theory] @@ -113,5 +122,28 @@ public async Task ValidateSnippets(string source, int expectedGeneratorErrors, i int compilerErrors = newComp.GetDiagnostics().Count(d => d.Severity == DiagnosticSeverity.Error); Assert.Equal(expectedCompilerErrors, compilerErrors); } + + public static IEnumerable CodeSnippetsToCompile_InvalidCode() + { + yield return new object[] { CodeSnippets.RecursiveImplicitlyBlittableStruct, 5, 1 }; + yield return new object[] { CodeSnippets.MutuallyRecursiveImplicitlyBlittableStruct, 5, 2 }; + } + + [Theory] + [MemberData(nameof(CodeSnippetsToCompile_InvalidCode))] + public async Task ValidateSnippets_InvalidCodeGracefulFailure(string source, int expectedGeneratorErrors, int expectedCompilerErrors) + { + // Do not validate that the compilation has no errors that the generator will not fix. + Compilation comp = await TestUtils.CreateCompilation(source); + + var newComp = TestUtils.RunGenerators(comp, out var generatorDiags, new Microsoft.Interop.DllImportGenerator()); + + // Verify the compilation failed with errors. + int generatorErrors = generatorDiags.Count(d => d.Severity == DiagnosticSeverity.Error); + Assert.Equal(expectedGeneratorErrors, generatorErrors); + + int compilerErrors = newComp.GetDiagnostics().Count(d => d.Severity == DiagnosticSeverity.Error); + Assert.Equal(expectedCompilerErrors, compilerErrors); + } } } diff --git a/src/libraries/System.Runtime.InteropServices/tests/DllImportGenerator.UnitTests/Compiles.cs b/src/libraries/System.Runtime.InteropServices/tests/DllImportGenerator.UnitTests/Compiles.cs index f3efff8e40bbd..a785ed75ac654 100644 --- a/src/libraries/System.Runtime.InteropServices/tests/DllImportGenerator.UnitTests/Compiles.cs +++ b/src/libraries/System.Runtime.InteropServices/tests/DllImportGenerator.UnitTests/Compiles.cs @@ -131,7 +131,6 @@ public static IEnumerable CodeSnippetsToCompile() // Structs yield return new[] { CodeSnippets.BlittableStructParametersAndModifiers }; - yield return new[] { CodeSnippets.GenericBlittableStructParametersAndModifiers }; // SafeHandle yield return new[] { CodeSnippets.BasicParametersAndModifiers("Microsoft.Win32.SafeHandles.SafeFileHandle") }; @@ -184,6 +183,26 @@ public static IEnumerable CodeSnippetsToCompile() yield return new[] { CodeSnippets.ByValueParameterWithName("Method", "@event") }; yield return new[] { CodeSnippets.ByValueParameterWithName("Method", "@var") }; yield return new[] { CodeSnippets.ByValueParameterWithName("@params", "i") }; + + // Generics + yield return new[] { CodeSnippets.MaybeBlittableGenericTypeParametersAndModifiers() }; + yield return new[] { CodeSnippets.MaybeBlittableGenericTypeParametersAndModifiers() }; + yield return new[] { CodeSnippets.MaybeBlittableGenericTypeParametersAndModifiers() }; + yield return new[] { CodeSnippets.MaybeBlittableGenericTypeParametersAndModifiers() }; + yield return new[] { CodeSnippets.MaybeBlittableGenericTypeParametersAndModifiers() }; + yield return new[] { CodeSnippets.MaybeBlittableGenericTypeParametersAndModifiers() }; + yield return new[] { CodeSnippets.MaybeBlittableGenericTypeParametersAndModifiers() }; + yield return new[] { CodeSnippets.MaybeBlittableGenericTypeParametersAndModifiers() }; + yield return new[] { CodeSnippets.MaybeBlittableGenericTypeParametersAndModifiers() }; + yield return new[] { CodeSnippets.MaybeBlittableGenericTypeParametersAndModifiers() }; + yield return new[] { CodeSnippets.MaybeBlittableGenericTypeParametersAndModifiers() }; + yield return new[] { CodeSnippets.MaybeBlittableGenericTypeParametersAndModifiers() }; + + // Implicit blittable types + yield return new[] { CodeSnippets.ImplicitlyBlittableStructParametersAndModifiers() }; + yield return new[] { CodeSnippets.ImplicitlyBlittableStructParametersAndModifiers("internal") }; + yield return new[] { CodeSnippets.ImplicitlyBlittableGenericTypeParametersAndModifiers() }; + yield return new[] { CodeSnippets.ImplicitlyBlittableGenericTypeParametersAndModifiers("internal") }; } [Theory]