From 87b5eb8fec2c926bea5413eb1b1ed5e3a64c3d07 Mon Sep 17 00:00:00 2001 From: Jeremy Koritzinsky Date: Fri, 2 Jul 2021 14:42:09 -0700 Subject: [PATCH 1/3] Update analyzer to correctly handle generic marshallers of generic types. --- .../ManualTypeMarshallingAnalyzerTests.cs | 58 ++++++++++++++++- .../Analyzers/AnalyzerDiagnostics.cs | 2 +- .../ManualTypeMarshallingAnalyzer.cs | 63 +++++++++++++------ .../DllImportGenerator/Resources.Designer.cs | 12 ++-- .../DllImportGenerator/Resources.resx | 8 +-- .../TypeSymbolExtensions.cs | 2 +- 6 files changed, 111 insertions(+), 34 deletions(-) diff --git a/DllImportGenerator/DllImportGenerator.UnitTests/ManualTypeMarshallingAnalyzerTests.cs b/DllImportGenerator/DllImportGenerator.UnitTests/ManualTypeMarshallingAnalyzerTests.cs index 0aae34ff4953..61800fd3b035 100644 --- a/DllImportGenerator/DllImportGenerator.UnitTests/ManualTypeMarshallingAnalyzerTests.cs +++ b/DllImportGenerator/DllImportGenerator.UnitTests/ManualTypeMarshallingAnalyzerTests.cs @@ -952,7 +952,7 @@ public Native(S s) } [Fact] - public async Task UninstantiatedGenericNativeType_ReportsDiagnostic() + public async Task UninstantiatedGenericNativeTypeOnNonGeneric_ReportsDiagnostic() { string source = @" @@ -976,7 +976,61 @@ public Native(S s) public T Value { get; set; } }"; - await VerifyCS.VerifyAnalyzerAsync(source, VerifyCS.Diagnostic(NativeGenericTypeMustBeClosedRule).WithLocation(0).WithArguments("Native<>", "S")); + await VerifyCS.VerifyAnalyzerAsync(source, VerifyCS.Diagnostic(NativeGenericTypeMustBeClosedOrMatchArityRule).WithLocation(0).WithArguments("Native<>", "S")); + } + + [Fact] + public async Task UninstantiatedGenericNativeTypeOnGenericWithArityMismatch_ReportsDiagnostic() + { + string source = @" +using System.Runtime.InteropServices; + +[{|#0:NativeMarshalling(typeof(Native<,>))|}] +struct S +{ + public string s; +} + +struct Native + where T : new() +{ + public Native(S s) + { + Value = 0; + } + + public S ToManaged() => new S(); + + public int Value { get; set; } +}"; + await VerifyCS.VerifyAnalyzerAsync(source, VerifyCS.Diagnostic(NativeGenericTypeMustBeClosedOrMatchArityRule).WithLocation(0).WithArguments("Native<,>", "S")); + } + + [Fact] + public async Task UninstantiatedGenericNativeTypeOnGenericWithArityMatch_DoesNotReportDiagnostic() + { + string source = @" +using System.Runtime.InteropServices; + +[NativeMarshalling(typeof(Native<>))] +struct S +{ + public T t; +} + +struct Native + where T : new() +{ + public Native(S s) + { + Value = 0; + } + + public S ToManaged() => new S(); + + public int Value { get; set; } +}"; + await VerifyCS.VerifyAnalyzerAsync(source); } [Fact] diff --git a/DllImportGenerator/DllImportGenerator/Analyzers/AnalyzerDiagnostics.cs b/DllImportGenerator/DllImportGenerator/Analyzers/AnalyzerDiagnostics.cs index 88a5f269f322..dd6a675d56e6 100644 --- a/DllImportGenerator/DllImportGenerator/Analyzers/AnalyzerDiagnostics.cs +++ b/DllImportGenerator/DllImportGenerator/Analyzers/AnalyzerDiagnostics.cs @@ -25,7 +25,7 @@ public static class Ids public const string StackallocMarshallingShouldSupportAllocatingMarshallingFallback = Prefix + "011"; public const string StackallocConstructorMustHaveStackBufferSizeConstant = Prefix + "012"; public const string RefValuePropertyUnsupported = Prefix + "014"; - public const string NativeGenericTypeMustBeClosed = Prefix + "016"; + public const string NativeGenericTypeMustBeClosedOrMatchArity = Prefix + "016"; // GeneratedDllImport public const string GeneratedDllImportMissingRequiredModifiers = Prefix + "013"; diff --git a/DllImportGenerator/DllImportGenerator/Analyzers/ManualTypeMarshallingAnalyzer.cs b/DllImportGenerator/DllImportGenerator/Analyzers/ManualTypeMarshallingAnalyzer.cs index 90d12505d5ee..00e0079840ec 100644 --- a/DllImportGenerator/DllImportGenerator/Analyzers/ManualTypeMarshallingAnalyzer.cs +++ b/DllImportGenerator/DllImportGenerator/Analyzers/ManualTypeMarshallingAnalyzer.cs @@ -144,15 +144,15 @@ public class ManualTypeMarshallingAnalyzer : DiagnosticAnalyzer isEnabledByDefault: true, description: GetResourceString(nameof(Resources.RefValuePropertyUnsupportedDescription))); - public readonly static DiagnosticDescriptor NativeGenericTypeMustBeClosedRule = + public readonly static DiagnosticDescriptor NativeGenericTypeMustBeClosedOrMatchArityRule = new DiagnosticDescriptor( - Ids.NativeGenericTypeMustBeClosed, - "GenericTypeMustBeClosed", - GetResourceString(nameof(Resources.NativeGenericTypeMustBeClosedMessage)), + Ids.NativeGenericTypeMustBeClosedOrMatchArity, + "NativeGenericTypeMustBeClosedOrMatchArity", + GetResourceString(nameof(Resources.NativeGenericTypeMustBeClosedOrMatchArityMessage)), Category, DiagnosticSeverity.Error, isEnabledByDefault: true, - description: GetResourceString(nameof(Resources.NativeGenericTypeMustBeClosedDescription))); + description: GetResourceString(nameof(Resources.NativeGenericTypeMustBeClosedOrMatchArityDescription))); public override ImmutableArray SupportedDiagnostics => ImmutableArray.Create( @@ -169,7 +169,7 @@ public class ManualTypeMarshallingAnalyzer : DiagnosticAnalyzer StackallocMarshallingShouldSupportAllocatingMarshallingFallbackRule, StackallocConstructorMustHaveStackBufferSizeConstantRule, RefValuePropertyUnsupportedRule, - NativeGenericTypeMustBeClosedRule); + NativeGenericTypeMustBeClosedOrMatchArityRule); public override void Initialize(AnalysisContext context) { @@ -185,12 +185,14 @@ private void PrepareForAnalysis(CompilationStartAnalysisContext context) var blittableTypeAttribute = context.Compilation.GetTypeByMetadataName(TypeNames.BlittableTypeAttribute); var nativeMarshallingAttribute = context.Compilation.GetTypeByMetadataName(TypeNames.NativeMarshallingAttribute); var marshalUsingAttribute = context.Compilation.GetTypeByMetadataName(TypeNames.MarshalUsingAttribute); + var genericContiguousCollectionMarshallerAttribute = context.Compilation.GetTypeByMetadataName(TypeNames.GenericContiguousCollectionMarshallerAttribute); var spanOfByte = context.Compilation.GetTypeByMetadataName(TypeNames.System_Span_Metadata)!.Construct(context.Compilation.GetSpecialType(SpecialType.System_Byte)); if (generatedMarshallingAttribute is not null && blittableTypeAttribute is not null && nativeMarshallingAttribute is not null && marshalUsingAttribute is not null + && genericContiguousCollectionMarshallerAttribute is not null && spanOfByte is not null) { var perCompilationAnalyzer = new PerCompilationAnalyzer( @@ -198,6 +200,7 @@ private void PrepareForAnalysis(CompilationStartAnalysisContext context) blittableTypeAttribute, nativeMarshallingAttribute, marshalUsingAttribute, + genericContiguousCollectionMarshallerAttribute, spanOfByte, context.Compilation.GetTypeByMetadataName(TypeNames.System_Runtime_InteropServices_StructLayoutAttribute)!); context.RegisterSymbolAction(context => perCompilationAnalyzer.AnalyzeTypeDefinition(context), SymbolKind.NamedType); @@ -212,6 +215,7 @@ class PerCompilationAnalyzer private readonly INamedTypeSymbol BlittableTypeAttribute; private readonly INamedTypeSymbol NativeMarshallingAttribute; private readonly INamedTypeSymbol MarshalUsingAttribute; + private readonly INamedTypeSymbol GenericContiguousCollectionMarshallerAttribute; private readonly INamedTypeSymbol SpanOfByte; private readonly INamedTypeSymbol StructLayoutAttribute; @@ -219,6 +223,7 @@ public PerCompilationAnalyzer(INamedTypeSymbol generatedMarshallingAttribute, INamedTypeSymbol blittableTypeAttribute, INamedTypeSymbol nativeMarshallingAttribute, INamedTypeSymbol marshalUsingAttribute, + INamedTypeSymbol genericContiguousCollectionMarshallerAttribute, INamedTypeSymbol spanOfByte, INamedTypeSymbol structLayoutAttribute) { @@ -226,6 +231,7 @@ public PerCompilationAnalyzer(INamedTypeSymbol generatedMarshallingAttribute, BlittableTypeAttribute = blittableTypeAttribute; NativeMarshallingAttribute = nativeMarshallingAttribute; MarshalUsingAttribute = marshalUsingAttribute; + GenericContiguousCollectionMarshallerAttribute = genericContiguousCollectionMarshallerAttribute; SpanOfByte = spanOfByte; StructLayoutAttribute = structLayoutAttribute; } @@ -270,7 +276,7 @@ public void AnalyzeTypeDefinition(SymbolAnalysisContext context) } else if (nativeMarshallingAttributeData is not null) { - AnalyzeNativeMarshalerType(context, type, nativeMarshallingAttributeData, validateManagedGetPinnableReference: true, validateAllScenarioSupport: true); + AnalyzeNativeMarshalerType(context, type, nativeMarshallingAttributeData, defaultMarshaller:true); } } @@ -292,11 +298,11 @@ public void AnalyzeElement(SymbolAnalysisContext context) { if (context.Symbol is IParameterSymbol param) { - AnalyzeNativeMarshalerType(context, param.Type, attrData, false, false); + AnalyzeNativeMarshalerType(context, param.Type, attrData, defaultMarshaller: false); } else if (context.Symbol is IFieldSymbol field) { - AnalyzeNativeMarshalerType(context, field.Type, attrData, false, false); + AnalyzeNativeMarshalerType(context, field.Type, attrData, defaultMarshaller: false); } } } @@ -307,11 +313,11 @@ public void AnalyzeReturnType(SymbolAnalysisContext context) AttributeData? attrData = method.GetReturnTypeAttributes().FirstOrDefault(attr => SymbolEqualityComparer.Default.Equals(MarshalUsingAttribute, attr.AttributeClass)); if (attrData is not null) { - AnalyzeNativeMarshalerType(context, method.ReturnType, attrData, false, false); + AnalyzeNativeMarshalerType(context, method.ReturnType, attrData, defaultMarshaller: false); } } - private void AnalyzeNativeMarshalerType(SymbolAnalysisContext context, ITypeSymbol type, AttributeData nativeMarshalerAttributeData, bool validateManagedGetPinnableReference, bool validateAllScenarioSupport) + private void AnalyzeNativeMarshalerType(SymbolAnalysisContext context, ITypeSymbol type, AttributeData nativeMarshalerAttributeData, bool defaultMarshaller) { if (nativeMarshalerAttributeData.ConstructorArguments.Length == 0) { @@ -353,12 +359,29 @@ private void AnalyzeNativeMarshalerType(SymbolAnalysisContext context, ITypeSymb if (marshalerType.IsUnboundGenericType) { - context.ReportDiagnostic( - nativeMarshalerAttributeData.CreateDiagnostic( - NativeGenericTypeMustBeClosedRule, - nativeType.ToDisplayString(), - type.ToDisplayString())); - return; + if (!defaultMarshaller) + { + context.ReportDiagnostic( + nativeMarshalerAttributeData.CreateDiagnostic( + NativeGenericTypeMustBeClosedOrMatchArityRule, + nativeType.ToDisplayString(), + type.ToDisplayString())); + return; + } + else if (type is not INamedTypeSymbol namedType || marshalerType.TypeArguments.Length != namedType.TypeArguments.Length) + { + context.ReportDiagnostic( + nativeMarshalerAttributeData.CreateDiagnostic( + NativeGenericTypeMustBeClosedOrMatchArityRule, + nativeType.ToDisplayString(), + type.ToDisplayString())); + return; + } + else + { + // Construct the marshaler type around the same type arguments as the managed type. + nativeType = marshalerType = marshalerType.ConstructedFrom.Construct(namedType.TypeArguments, namedType.TypeArgumentNullableAnnotations); + } } bool hasConstructor = false; @@ -399,7 +422,7 @@ private void AnalyzeNativeMarshalerType(SymbolAnalysisContext context, ITypeSymb } // Validate that this type can support marshalling when stackalloc is not usable. - if (validateAllScenarioSupport && hasStackallocConstructor && !hasConstructor) + if (defaultMarshaller && hasStackallocConstructor && !hasConstructor) { context.ReportDiagnostic( marshalerType.CreateDiagnostic( @@ -455,7 +478,7 @@ private void AnalyzeNativeMarshalerType(SymbolAnalysisContext context, ITypeSymb // Use a tuple here instead of an anonymous type so we can do the reassignment and pattern matching below. var getPinnableReferenceMethods = new { - Managed = validateManagedGetPinnableReference? ManualTypeMarshallingHelper.FindGetPinnableReference(type) : null, + Managed = defaultMarshaller ? ManualTypeMarshallingHelper.FindGetPinnableReference(type) : null, Marshaler = ManualTypeMarshallingHelper.FindGetPinnableReference(marshalerType) }; @@ -466,7 +489,7 @@ private void AnalyzeNativeMarshalerType(SymbolAnalysisContext context, ITypeSymb context.ReportDiagnostic(getPinnableReferenceMethods.Managed.CreateDiagnostic(GetPinnableReferenceReturnTypeBlittableRule)); } // Validate that our marshaler supports scenarios where GetPinnableReference cannot be used. - if (validateAllScenarioSupport && (!hasConstructor || valueProperty is { GetMethod: null })) + if (defaultMarshaller && (!hasConstructor || valueProperty is { GetMethod: null })) { context.ReportDiagnostic( nativeMarshalerAttributeData.CreateDiagnostic( diff --git a/DllImportGenerator/DllImportGenerator/Resources.Designer.cs b/DllImportGenerator/DllImportGenerator/Resources.Designer.cs index 3a93516652e5..adc27f852830 100644 --- a/DllImportGenerator/DllImportGenerator/Resources.Designer.cs +++ b/DllImportGenerator/DllImportGenerator/Resources.Designer.cs @@ -394,20 +394,20 @@ internal static string MarshallingStringOrCharAsUndefinedNotSupported { } /// - /// Looks up a localized string similar to The native type '{0}' must be a closed generic so the emitted code can use a specific instantiation.. + /// Looks up a localized string similar to The native type '{0}' must be a closed generic or have the same number of generic parameters as the managed type so the emitted code can use a specific instantiation.. /// - internal static string NativeGenericTypeMustBeClosedDescription { + internal static string NativeGenericTypeMustBeClosedOrMatchArityDescription { get { - return ResourceManager.GetString("NativeGenericTypeMustBeClosedDescription", resourceCulture); + return ResourceManager.GetString("NativeGenericTypeMustBeClosedOrMatchArityDescription", resourceCulture); } } /// - /// Looks up a localized string similar to The native type '{0}' for managed type '{1}' must be a closed generic type.. + /// Looks up a localized string similar to The native type '{0}' for managed type '{1}' must be a closed generic type or have the same arity as the managed type.. /// - internal static string NativeGenericTypeMustBeClosedMessage { + internal static string NativeGenericTypeMustBeClosedOrMatchArityMessage { get { - return ResourceManager.GetString("NativeGenericTypeMustBeClosedMessage", resourceCulture); + return ResourceManager.GetString("NativeGenericTypeMustBeClosedOrMatchArityMessage", resourceCulture); } } diff --git a/DllImportGenerator/DllImportGenerator/Resources.resx b/DllImportGenerator/DllImportGenerator/Resources.resx index 56d1e2c89557..57549952f177 100644 --- a/DllImportGenerator/DllImportGenerator/Resources.resx +++ b/DllImportGenerator/DllImportGenerator/Resources.resx @@ -229,11 +229,11 @@ Marshalling string or char without explicit marshalling information is not supported. Specify either 'GeneratedDllImportAttribute.CharSet' or 'MarshalAsAttribute'. - - The native type '{0}' must be a closed generic so the emitted code can use a specific instantiation. + + The native type '{0}' must be a closed generic or have the same number of generic parameters as the managed type so the emitted code can use a specific instantiation. - - The native type '{0}' for managed type '{1}' must be a closed generic type. + + The native type '{0}' for managed type '{1}' must be a closed generic type or have the same arity as the managed type. A native type for a given type must be blittable. diff --git a/DllImportGenerator/DllImportGenerator/TypeSymbolExtensions.cs b/DllImportGenerator/DllImportGenerator/TypeSymbolExtensions.cs index 2cf28efc9434..a37aa5708efa 100644 --- a/DllImportGenerator/DllImportGenerator/TypeSymbolExtensions.cs +++ b/DllImportGenerator/DllImportGenerator/TypeSymbolExtensions.cs @@ -85,7 +85,7 @@ private static bool IsConsideredBlittable(this ITypeSymbol type, ImmutableHashSe return true; } - if (!type.IsValueType || type.IsReferenceType) + if (type.IsReferenceType) { return false; } From 4abf33468f0ae48fdaaa0e97cd54eb9dbb5cf397 Mon Sep 17 00:00:00 2001 From: Jeremy Koritzinsky Date: Fri, 2 Jul 2021 15:14:09 -0700 Subject: [PATCH 2/3] Add validation of generic collection marshallers. --- .../ManualTypeMarshallingAnalyzerTests.cs | 191 ++++++++++++++++++ .../ManualTypeMarshallingAnalyzer.cs | 46 ++++- .../DllImportGenerator/Resources.Designer.cs | 18 ++ .../DllImportGenerator/Resources.resx | 6 + 4 files changed, 253 insertions(+), 8 deletions(-) diff --git a/DllImportGenerator/DllImportGenerator.UnitTests/ManualTypeMarshallingAnalyzerTests.cs b/DllImportGenerator/DllImportGenerator.UnitTests/ManualTypeMarshallingAnalyzerTests.cs index 61800fd3b035..1d5fe1a7ca1c 100644 --- a/DllImportGenerator/DllImportGenerator.UnitTests/ManualTypeMarshallingAnalyzerTests.cs +++ b/DllImportGenerator/DllImportGenerator.UnitTests/ManualTypeMarshallingAnalyzerTests.cs @@ -613,6 +613,197 @@ await VerifyCS.VerifyAnalyzerAsync(source, VerifyCS.Diagnostic(NativeTypeMustHaveRequiredShapeRule).WithLocation(0).WithArguments("Native", "S")); } + [Fact] + public async Task CollectionNativeTypeWithNoMarshallingMethods_ReportsDiagnostic() + { + string source = @" +using System; +using System.Runtime.InteropServices; + +[NativeMarshalling(typeof(Native))] +class S +{ + public byte c; +} + +[GenericContiguousCollectionMarshaller] +struct {|#0:Native|} +{ +}"; + + await VerifyCS.VerifyAnalyzerAsync(source, + VerifyCS.Diagnostic(CollectionNativeTypeMustHaveRequiredShapeRule).WithLocation(0).WithArguments("Native", "S")); + } + + [Fact] + public async Task CollectionNativeTypeWithWrongConstructor_ReportsDiagnostic() + { + string source = @" +using System; +using System.Runtime.InteropServices; + +[NativeMarshalling(typeof(Native))] +class S +{ + public byte c; +} + +[GenericContiguousCollectionMarshaller] +ref struct {|#0:Native|} +{ + public Native(S s) : this() {} + + public Span ManagedValues { get; set; } + public Span NativeValueStorage { get; set; } + + public IntPtr Value { get; } +}"; + + await VerifyCS.VerifyAnalyzerAsync(source, + VerifyCS.Diagnostic(CollectionNativeTypeMustHaveRequiredShapeRule).WithLocation(0).WithArguments("Native", "S")); + } + + [Fact] + public async Task CollectionNativeTypeWithCorrectConstructor_DoesNotReportDiagnostic() + { + string source = @" +using System; +using System.Runtime.InteropServices; + +[NativeMarshalling(typeof(Native))] +class S +{ + public byte c; +} + +[GenericContiguousCollectionMarshaller] +ref struct Native +{ + public Native(S s, int nativeElementSize) : this() {} + + public Span ManagedValues { get; set; } + public Span NativeValueStorage { get; set; } + + public IntPtr Value { get; } +}"; + + await VerifyCS.VerifyAnalyzerAsync(source); + } + + [Fact] + public async Task CollectionNativeTypeWithIncorrectStackallocConstructor_ReportsDiagnostic() + { + string source = @" +using System; +using System.Runtime.InteropServices; + +[NativeMarshalling(typeof(Native))] +class S +{ + public byte c; +} + +[GenericContiguousCollectionMarshaller] +ref struct {|#0:Native|} +{ + public Native(S s, Span stackSpace) : this() {} + + public const int StackBufferSize = 1; + + public Span ManagedValues { get; set; } + public Span NativeValueStorage { get; set; } + + public IntPtr Value { get; } +}"; + + await VerifyCS.VerifyAnalyzerAsync(source, + VerifyCS.Diagnostic(CollectionNativeTypeMustHaveRequiredShapeRule).WithLocation(0).WithArguments("Native", "S")); + } + + [Fact] + public async Task CollectionNativeTypeWithOnlyStackallocConstructor_ReportsDiagnostic() + { + string source = @" +using System; +using System.Runtime.InteropServices; + +[NativeMarshalling(typeof(Native))] +class S +{ + public byte c; +} + +[GenericContiguousCollectionMarshaller] +ref struct {|#0:Native|} +{ + public Native(S s, Span stackSpace, int nativeElementSize) : this() {} + + public const int StackBufferSize = 1; + + public Span ManagedValues { get; set; } + public Span NativeValueStorage { get; set; } + + public IntPtr Value { get; } +}"; + + await VerifyCS.VerifyAnalyzerAsync(source, + VerifyCS.Diagnostic(StackallocMarshallingShouldSupportAllocatingMarshallingFallbackRule).WithLocation(0).WithArguments("Native", "S")); + } + + [Fact] + public async Task CollectionNativeTypeWithMissingManagedValuesProperty_ReportsDiagnostic() + { + string source = @" +using System; +using System.Runtime.InteropServices; + +[NativeMarshalling(typeof(Native))] +class S +{ + public byte c; +} + +[GenericContiguousCollectionMarshaller] +ref struct {|#0:Native|} +{ + public Native(S s, int nativeElementSize) : this() {} + + public Span NativeValueStorage { get; set; } + + public IntPtr Value { get; } +}"; + + await VerifyCS.VerifyAnalyzerAsync(source, + VerifyCS.Diagnostic(CollectionNativeTypeMustHaveRequiredShapeRule).WithLocation(0).WithArguments("Native", "S")); + } + + [Fact] + public async Task CollectionNativeTypeWithMissingNativeValueStorageProperty_ReportsDiagnostic() + { + string source = @" +using System; +using System.Runtime.InteropServices; + +[NativeMarshalling(typeof(Native))] +class S +{ + public byte c; +} + +[GenericContiguousCollectionMarshaller] +ref struct {|#0:Native|} +{ + public Native(S s, int nativeElementSize) : this() {} + + public Span ManagedValues { get; set; } + + public IntPtr Value { get; } +}"; + + await VerifyCS.VerifyAnalyzerAsync(source, + VerifyCS.Diagnostic(CollectionNativeTypeMustHaveRequiredShapeRule).WithLocation(0).WithArguments("Native", "S")); + } + [Fact] public async Task NativeTypeWithOnlyConstructor_DoesNotReportDiagnostic() { diff --git a/DllImportGenerator/DllImportGenerator/Analyzers/ManualTypeMarshallingAnalyzer.cs b/DllImportGenerator/DllImportGenerator/Analyzers/ManualTypeMarshallingAnalyzer.cs index 00e0079840ec..94b6c607fcff 100644 --- a/DllImportGenerator/DllImportGenerator/Analyzers/ManualTypeMarshallingAnalyzer.cs +++ b/DllImportGenerator/DllImportGenerator/Analyzers/ManualTypeMarshallingAnalyzer.cs @@ -84,6 +84,16 @@ public class ManualTypeMarshallingAnalyzer : DiagnosticAnalyzer isEnabledByDefault: true, description: GetResourceString(nameof(Resources.NativeTypeMustHaveRequiredShapeDescription))); + public readonly static DiagnosticDescriptor CollectionNativeTypeMustHaveRequiredShapeRule = + new DiagnosticDescriptor( + Ids.NativeTypeMustHaveRequiredShape, + "NativeTypeMustHaveRequiredShape", + GetResourceString(nameof(Resources.CollectionNativeTypeMustHaveRequiredShapeMessage)), + Category, + DiagnosticSeverity.Error, + isEnabledByDefault: true, + description: GetResourceString(nameof(Resources.CollectionNativeTypeMustHaveRequiredShapeDescription))); + public readonly static DiagnosticDescriptor ValuePropertyMustHaveSetterRule = new DiagnosticDescriptor( Ids.ValuePropertyMustHaveSetter, @@ -163,6 +173,7 @@ public class ManualTypeMarshallingAnalyzer : DiagnosticAnalyzer GetPinnableReferenceReturnTypeBlittableRule, NativeTypeMustBePointerSizedRule, NativeTypeMustHaveRequiredShapeRule, + CollectionNativeTypeMustHaveRequiredShapeRule, ValuePropertyMustHaveSetterRule, ValuePropertyMustHaveGetterRule, GetPinnableReferenceShouldSupportAllocatingMarshallingFallbackRule, @@ -337,21 +348,40 @@ private void AnalyzeNativeMarshalerType(SymbolAnalysisContext context, ITypeSymb ITypeSymbol nativeType = (ITypeSymbol)nativeMarshalerAttributeData.ConstructorArguments[0].Value!; ISymbol nativeTypeDiagnosticsTargetSymbol = nativeType; - if (!nativeType.IsValueType) + if (nativeType is not INamedTypeSymbol marshalerType) { context.ReportDiagnostic( - nativeType.CreateDiagnostic( + nativeMarshalerAttributeData.CreateDiagnostic( NativeTypeMustHaveRequiredShapeRule, nativeType.ToDisplayString(), type.ToDisplayString())); return; } - if (nativeType is not INamedTypeSymbol marshalerType) + DiagnosticDescriptor requiredShapeRule = NativeTypeMustHaveRequiredShapeRule; + + ManualTypeMarshallingHelper.NativeTypeMarshallingVariant variant = ManualTypeMarshallingHelper.NativeTypeMarshallingVariant.Standard; + if (marshalerType.GetAttributes().Any(a => SymbolEqualityComparer.Default.Equals(GenericContiguousCollectionMarshallerAttribute, a.AttributeClass))) + { + variant = ManualTypeMarshallingHelper.NativeTypeMarshallingVariant.ContiguousCollection; + requiredShapeRule = CollectionNativeTypeMustHaveRequiredShapeRule; + if (!ManualTypeMarshallingHelper.TryGetManagedValuesProperty(marshalerType, out _) + || !ManualTypeMarshallingHelper.HasNativeValueStorageProperty(marshalerType, SpanOfByte)) + { + context.ReportDiagnostic( + marshalerType.CreateDiagnostic( + requiredShapeRule, + nativeType.ToDisplayString(), + type.ToDisplayString())); + return; + } + } + + if (!nativeType.IsValueType) { context.ReportDiagnostic( - nativeMarshalerAttributeData.CreateDiagnostic( - NativeTypeMustHaveRequiredShapeRule, + nativeType.CreateDiagnostic( + requiredShapeRule, nativeType.ToDisplayString(), type.ToDisplayString())); return; @@ -393,9 +423,9 @@ private void AnalyzeNativeMarshalerType(SymbolAnalysisContext context, ITypeSymb continue; } - hasConstructor = hasConstructor || ManualTypeMarshallingHelper.IsManagedToNativeConstructor(ctor, type, ManualTypeMarshallingHelper.NativeTypeMarshallingVariant.Standard); + hasConstructor = hasConstructor || ManualTypeMarshallingHelper.IsManagedToNativeConstructor(ctor, type, variant); - if (!hasStackallocConstructor && ManualTypeMarshallingHelper.IsStackallocConstructor(ctor, type, SpanOfByte, ManualTypeMarshallingHelper.NativeTypeMarshallingVariant.Standard)) + if (!hasStackallocConstructor && ManualTypeMarshallingHelper.IsStackallocConstructor(ctor, type, SpanOfByte, variant)) { hasStackallocConstructor = true; IFieldSymbol stackAllocSizeField = nativeType.GetMembers("StackBufferSize").OfType().FirstOrDefault(); @@ -416,7 +446,7 @@ private void AnalyzeNativeMarshalerType(SymbolAnalysisContext context, ITypeSymb { context.ReportDiagnostic( marshalerType.CreateDiagnostic( - NativeTypeMustHaveRequiredShapeRule, + requiredShapeRule, marshalerType.ToDisplayString(), type.ToDisplayString())); } diff --git a/DllImportGenerator/DllImportGenerator/Resources.Designer.cs b/DllImportGenerator/DllImportGenerator/Resources.Designer.cs index adc27f852830..79d1a4f8c3ec 100644 --- a/DllImportGenerator/DllImportGenerator/Resources.Designer.cs +++ b/DllImportGenerator/DllImportGenerator/Resources.Designer.cs @@ -114,6 +114,24 @@ internal static string CannotHaveMultipleMarshallingAttributesMessage { } } + /// + /// Looks up a localized string similar to A native type with the 'GenericContiguousCollectionMarshallerAttribute' must have at least one of the two marshalling methods as well as a 'ManagedValues' property of type 'Span<T>' for some 'T' and a 'NativeValueStorage' property of type 'Span<byte>' to enable marshalling the managed type.. + /// + internal static string CollectionNativeTypeMustHaveRequiredShapeDescription { + get { + return ResourceManager.GetString("CollectionNativeTypeMustHaveRequiredShapeDescription", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to The native type '{0}' must be a value type and have a constructor that takes two parameters, one of type '{1}' and an 'int', or have a parameterless instance method named 'ToManaged' that returns '{1}' as well as a 'ManagedValues' property of type 'Span<T>' for some 'T' and a 'NativeValueStorage' property of type 'Span<byte>'. + /// + internal static string CollectionNativeTypeMustHaveRequiredShapeMessage { + get { + return ResourceManager.GetString("CollectionNativeTypeMustHaveRequiredShapeMessage", resourceCulture); + } + } + /// /// Looks up a localized string similar to The specified collection size parameter for an collection must be an integer type. If the size information is applied to a nested collection, the size parameter must be a collection of one less level of nesting with an integral element.. /// diff --git a/DllImportGenerator/DllImportGenerator/Resources.resx b/DllImportGenerator/DllImportGenerator/Resources.resx index 57549952f177..25a4e5c8cd5c 100644 --- a/DllImportGenerator/DllImportGenerator/Resources.resx +++ b/DllImportGenerator/DllImportGenerator/Resources.resx @@ -259,6 +259,12 @@ The native type '{0}' must be a value type and have a constructor that takes one parameter of type '{1}' or a parameterless instance method named 'ToManaged' that returns '{1}' + + A native type with the 'GenericContiguousCollectionMarshallerAttribute' must have at least one of the two marshalling methods as well as a 'ManagedValues' property of type 'Span<T>' for some 'T' and a 'NativeValueStorage' property of type 'Span<byte>' to enable marshalling the managed type. + + + The native type '{0}' must be a value type and have a constructor that takes two parameters, one of type '{1}' and an 'int', or have a parameterless instance method named 'ToManaged' that returns '{1}' as well as a 'ManagedValues' property of type 'Span<T>' for some 'T' and a 'NativeValueStorage' property of type 'Span<byte>' + The '[Out]' attribute is only supported on array parameters. From 6dce5d1d93340ca421b1517fbdf1c7a369c0b2ad Mon Sep 17 00:00:00 2001 From: Jeremy Koritzinsky Date: Thu, 8 Jul 2021 14:08:03 -0700 Subject: [PATCH 3/3] PR feedback --- .../ManualTypeMarshallingAnalyzer.cs | 29 +++++++++---------- 1 file changed, 13 insertions(+), 16 deletions(-) diff --git a/DllImportGenerator/DllImportGenerator/Analyzers/ManualTypeMarshallingAnalyzer.cs b/DllImportGenerator/DllImportGenerator/Analyzers/ManualTypeMarshallingAnalyzer.cs index 14984276dccb..351a5e7a6901 100644 --- a/DllImportGenerator/DllImportGenerator/Analyzers/ManualTypeMarshallingAnalyzer.cs +++ b/DllImportGenerator/DllImportGenerator/Analyzers/ManualTypeMarshallingAnalyzer.cs @@ -287,7 +287,7 @@ public void AnalyzeTypeDefinition(SymbolAnalysisContext context) } else if (nativeMarshallingAttributeData is not null) { - AnalyzeNativeMarshalerType(context, type, nativeMarshallingAttributeData, defaultMarshaller:true); + AnalyzeNativeMarshalerType(context, type, nativeMarshallingAttributeData, isNativeMarshallingAttribute:true); } } @@ -309,11 +309,11 @@ public void AnalyzeElement(SymbolAnalysisContext context) { if (context.Symbol is IParameterSymbol param) { - AnalyzeNativeMarshalerType(context, param.Type, attrData, defaultMarshaller: false); + AnalyzeNativeMarshalerType(context, param.Type, attrData, isNativeMarshallingAttribute: false); } else if (context.Symbol is IFieldSymbol field) { - AnalyzeNativeMarshalerType(context, field.Type, attrData, defaultMarshaller: false); + AnalyzeNativeMarshalerType(context, field.Type, attrData, isNativeMarshallingAttribute: false); } } } @@ -324,11 +324,11 @@ public void AnalyzeReturnType(SymbolAnalysisContext context) AttributeData? attrData = method.GetReturnTypeAttributes().FirstOrDefault(attr => SymbolEqualityComparer.Default.Equals(MarshalUsingAttribute, attr.AttributeClass)); if (attrData is not null) { - AnalyzeNativeMarshalerType(context, method.ReturnType, attrData, defaultMarshaller: false); + AnalyzeNativeMarshalerType(context, method.ReturnType, attrData, isNativeMarshallingAttribute: false); } } - private void AnalyzeNativeMarshalerType(SymbolAnalysisContext context, ITypeSymbol type, AttributeData nativeMarshalerAttributeData, bool defaultMarshaller) + private void AnalyzeNativeMarshalerType(SymbolAnalysisContext context, ITypeSymbol type, AttributeData nativeMarshalerAttributeData, bool isNativeMarshallingAttribute) { if (nativeMarshalerAttributeData.ConstructorArguments.Length == 0) { @@ -380,7 +380,7 @@ private void AnalyzeNativeMarshalerType(SymbolAnalysisContext context, ITypeSymb if (!nativeType.IsValueType) { context.ReportDiagnostic( - nativeType.CreateDiagnostic( + GetDiagnosticLocations(context, nativeType, nativeMarshalerAttributeData).CreateDiagnostic( requiredShapeRule, nativeType.ToDisplayString(), type.ToDisplayString())); @@ -389,7 +389,7 @@ private void AnalyzeNativeMarshalerType(SymbolAnalysisContext context, ITypeSymb if (marshalerType.IsUnboundGenericType) { - if (!defaultMarshaller) + if (!isNativeMarshallingAttribute) { context.ReportDiagnostic( nativeMarshalerAttributeData.CreateDiagnostic( @@ -398,7 +398,7 @@ private void AnalyzeNativeMarshalerType(SymbolAnalysisContext context, ITypeSymb type.ToDisplayString())); return; } - else if (type is not INamedTypeSymbol namedType || marshalerType.TypeArguments.Length != namedType.TypeArguments.Length) + if (type is not INamedTypeSymbol namedType || marshalerType.TypeArguments.Length != namedType.TypeArguments.Length) { context.ReportDiagnostic( nativeMarshalerAttributeData.CreateDiagnostic( @@ -407,11 +407,8 @@ private void AnalyzeNativeMarshalerType(SymbolAnalysisContext context, ITypeSymb type.ToDisplayString())); return; } - else - { - // Construct the marshaler type around the same type arguments as the managed type. - nativeType = marshalerType = marshalerType.ConstructedFrom.Construct(namedType.TypeArguments, namedType.TypeArgumentNullableAnnotations); - } + // Construct the marshaler type around the same type arguments as the managed type. + nativeType = marshalerType = marshalerType.ConstructedFrom.Construct(namedType.TypeArguments, namedType.TypeArgumentNullableAnnotations); } bool hasConstructor = false; @@ -452,7 +449,7 @@ private void AnalyzeNativeMarshalerType(SymbolAnalysisContext context, ITypeSymb } // Validate that this type can support marshalling when stackalloc is not usable. - if (defaultMarshaller && hasStackallocConstructor && !hasConstructor) + if (isNativeMarshallingAttribute && hasStackallocConstructor && !hasConstructor) { context.ReportDiagnostic( GetDiagnosticLocations(context, marshalerType, nativeMarshalerAttributeData).CreateDiagnostic( @@ -508,7 +505,7 @@ private void AnalyzeNativeMarshalerType(SymbolAnalysisContext context, ITypeSymb // Use a tuple here instead of an anonymous type so we can do the reassignment and pattern matching below. var getPinnableReferenceMethods = new { - Managed = defaultMarshaller ? ManualTypeMarshallingHelper.FindGetPinnableReference(type) : null, + Managed = isNativeMarshallingAttribute ? ManualTypeMarshallingHelper.FindGetPinnableReference(type) : null, Marshaler = ManualTypeMarshallingHelper.FindGetPinnableReference(marshalerType) }; @@ -519,7 +516,7 @@ private void AnalyzeNativeMarshalerType(SymbolAnalysisContext context, ITypeSymb context.ReportDiagnostic(getPinnableReferenceMethods.Managed.CreateDiagnostic(GetPinnableReferenceReturnTypeBlittableRule)); } // Validate that our marshaler supports scenarios where GetPinnableReference cannot be used. - if (defaultMarshaller && (!hasConstructor || valueProperty is { GetMethod: null })) + if (isNativeMarshallingAttribute && (!hasConstructor || valueProperty is { GetMethod: null })) { context.ReportDiagnostic( nativeMarshalerAttributeData.CreateDiagnostic(