Skip to content

Commit

Permalink
Support implicit blittability for structs declared in and not exposed…
Browse files Browse the repository at this point in the history
… outside of the current compilation. (dotnet/runtimelab#1126)

Commit migrated from dotnet/runtimelab@51d9ad8
  • Loading branch information
jkoritzinsky authored May 18, 2021
1 parent d205672 commit 7d9a4d9
Show file tree
Hide file tree
Showing 5 changed files with 180 additions and 41 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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))
{
Expand Down Expand Up @@ -335,6 +340,16 @@ static bool TryCreateTypeBasedMarshallingInfo(ITypeSymbol type, DefaultMarshalli
marshallingInfo = new ArrayMarshallingInfo(GetMarshallingInfo(elementType, Array.Empty<AttributeData>(), 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;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<ITypeSymbol>(SymbolEqualityComparer.Default));

private static bool HasOnlyBlittableFields(this ITypeSymbol type, HashSet<ITypeSymbol> 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<IFieldSymbol>())
{
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;
}

Expand All @@ -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<ITypeSymbol>(SymbolEqualityComparer.Default));

private static bool IsConsideredBlittable(this ITypeSymbol type, HashSet<ITypeSymbol> 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;
}
Expand All @@ -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;
Expand All @@ -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;
}
Expand All @@ -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;
}
Expand Down Expand Up @@ -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;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -511,14 +511,6 @@ struct MyStruct
private int i;
private short s;
}";
public static string GenericBlittableStructParametersAndModifiers = BasicParametersAndModifiers("MyStruct<int>") + @"
#pragma warning disable CS0169
[BlittableType]
struct MyStruct<T>
{
private T t;
private short s;
}";

public static string ArrayParametersAndModifiers(string elementType) => $@"
using System.Runtime.InteropServices;
Expand Down Expand Up @@ -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<T>
{
#pragma warning disable CS0649
public T field;
}
";

public static string MaybeBlittableGenericTypeParametersAndModifiers<T>() =>
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<T>
{{
#pragma warning disable CS0649
#pragma warning disable CS0169
public T field;
}}
";

public static string ImplicitlyBlittableGenericTypeParametersAndModifiers<T>(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;
}";
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,15 @@ public static IEnumerable<object[]> 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<bool>(), 5, 0 };

// No marshalling annotations

yield return new object[] { CodeSnippets.ImplicitlyBlittableStructParametersAndModifiers("public"), 5, 0 };
yield return new object[] { CodeSnippets.ImplicitlyBlittableGenericTypeParametersAndModifiers<bool>(), 5, 0 };
yield return new object[] { CodeSnippets.ImplicitlyBlittableGenericTypeParametersAndModifiers<int>("public"), 5, 0 };
}

[Theory]
Expand All @@ -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<object[]> 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);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,6 @@ public static IEnumerable<object[]> CodeSnippetsToCompile()

// Structs
yield return new[] { CodeSnippets.BlittableStructParametersAndModifiers };
yield return new[] { CodeSnippets.GenericBlittableStructParametersAndModifiers };

// SafeHandle
yield return new[] { CodeSnippets.BasicParametersAndModifiers("Microsoft.Win32.SafeHandles.SafeFileHandle") };
Expand Down Expand Up @@ -184,6 +183,26 @@ public static IEnumerable<object[]> 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<byte>() };
yield return new[] { CodeSnippets.MaybeBlittableGenericTypeParametersAndModifiers<sbyte>() };
yield return new[] { CodeSnippets.MaybeBlittableGenericTypeParametersAndModifiers<short>() };
yield return new[] { CodeSnippets.MaybeBlittableGenericTypeParametersAndModifiers<ushort>() };
yield return new[] { CodeSnippets.MaybeBlittableGenericTypeParametersAndModifiers<int>() };
yield return new[] { CodeSnippets.MaybeBlittableGenericTypeParametersAndModifiers<uint>() };
yield return new[] { CodeSnippets.MaybeBlittableGenericTypeParametersAndModifiers<long>() };
yield return new[] { CodeSnippets.MaybeBlittableGenericTypeParametersAndModifiers<ulong>() };
yield return new[] { CodeSnippets.MaybeBlittableGenericTypeParametersAndModifiers<float>() };
yield return new[] { CodeSnippets.MaybeBlittableGenericTypeParametersAndModifiers<double>() };
yield return new[] { CodeSnippets.MaybeBlittableGenericTypeParametersAndModifiers<IntPtr>() };
yield return new[] { CodeSnippets.MaybeBlittableGenericTypeParametersAndModifiers<UIntPtr>() };

// Implicit blittable types
yield return new[] { CodeSnippets.ImplicitlyBlittableStructParametersAndModifiers() };
yield return new[] { CodeSnippets.ImplicitlyBlittableStructParametersAndModifiers("internal") };
yield return new[] { CodeSnippets.ImplicitlyBlittableGenericTypeParametersAndModifiers<int>() };
yield return new[] { CodeSnippets.ImplicitlyBlittableGenericTypeParametersAndModifiers<int>("internal") };
}

[Theory]
Expand Down

0 comments on commit 7d9a4d9

Please sign in to comment.