Make [In] the default for blittable arrays that are pinned (#90054)
authorJackson Schuster <36744439+jtschuster@users.noreply.github.com>
Tue, 8 Aug 2023 00:01:45 +0000 (19:01 -0500)
committerGitHub <noreply@github.com>
Tue, 8 Aug 2023 00:01:45 +0000 (17:01 -0700)
* Make [In] the default for blittable arrays that are pinned

src/libraries/System.Runtime.InteropServices/gen/ComInterfaceGenerator/Analyzers/ConvertComImportToGeneratedComInterfaceAnalyzer.cs
src/libraries/System.Runtime.InteropServices/gen/Microsoft.Interop.SourceGeneration/Marshalling/ByValueMarshalKindSupportDescriptor.cs
src/libraries/System.Runtime.InteropServices/gen/Microsoft.Interop.SourceGeneration/Marshalling/CharMarshaller.cs
src/libraries/System.Runtime.InteropServices/gen/Microsoft.Interop.SourceGeneration/Marshalling/CustomTypeMarshallingGenerator.cs
src/libraries/System.Runtime.InteropServices/gen/Microsoft.Interop.SourceGeneration/Marshalling/IMarshallingGeneratorFactory.cs
src/libraries/System.Runtime.InteropServices/gen/Microsoft.Interop.SourceGeneration/Marshalling/StaticPinnableManagedValueMarshaller.cs
src/libraries/System.Runtime.InteropServices/tests/ComInterfaceGenerator.Tests/RcwAroundCcwTests.cs
src/libraries/System.Runtime.InteropServices/tests/ComInterfaceGenerator.Unit.Tests/CompileFails.cs
src/libraries/System.Runtime.InteropServices/tests/LibraryImportGenerator.UnitTests/CompileFails.cs
src/libraries/System.Runtime.InteropServices/tests/LibraryImportGenerator.UnitTests/Compiles.cs
src/libraries/System.Runtime.InteropServices/tests/TestAssets/SharedTypes/ComInterfaces/IStatelessPinnableCollectionBlittableElements.cs [new file with mode: 0644]

index d264dcf..d476547 100644 (file)
@@ -206,7 +206,7 @@ namespace Microsoft.Interop.Analyzers
             String
         }
 
-        private sealed record TrackedMarshallingInfo(TrackedMarshallingInfoAnnotation TrackingAnnotation, MarshallingInfo InnerInfo): MarshallingInfo;
+        private sealed record TrackedMarshallingInfo(TrackedMarshallingInfoAnnotation TrackingAnnotation, MarshallingInfo InnerInfo) : MarshallingInfo;
 
         private sealed class TrackingStringMarshallingInfoProvider : ITypeBasedMarshallingInfoProvider
         {
index b7c8051..1f859d3 100644 (file)
@@ -31,12 +31,12 @@ namespace Microsoft.Interop
             InOutSupport: ByValueMarshalKindSupport.Supported, InOutSupportDetails: null);
 
         /// <summary>
-        /// A default <see cref="ByValueMarshalKindSupportDescriptor"/> for pinned by value parameters. [In] is not allowed. [In, Out] is the default and unnecessary. [Out] is allowed.
+        /// A default <see cref="ByValueMarshalKindSupportDescriptor"/> for pinned parameters. [In] is allowed, but unnecessary. Out is allowed.
         /// </summary>
         public static readonly ByValueMarshalKindSupportDescriptor PinnedParameter = new ByValueMarshalKindSupportDescriptor(
-            InSupport: ByValueMarshalKindSupport.NotSupported, InSupportDetails: SR.InAttributeOnlyNotSupportedOnPinnedParameters,
+            InSupport: ByValueMarshalKindSupport.Unnecessary, InSupportDetails: SR.InAttributeOnlyIsDefault,
             OutSupport: ByValueMarshalKindSupport.Supported, OutSupportDetails: null,
-            InOutSupport: ByValueMarshalKindSupport.Unnecessary, InOutSupportDetails: SR.PinnedMarshallingIsInOutByDefault);
+            InOutSupport: ByValueMarshalKindSupport.Supported, InOutSupportDetails: null);
 
         /// <summary>
         /// Returns the support for the ByValueContentsMarshalKind, and if it is not <see cref="ByValueMarshalKindSupport.Supported"/>, diagnostic is not null
index ecf0380..3357dc8 100644 (file)
@@ -142,15 +142,6 @@ namespace Microsoft.Interop
         private static string PinnedIdentifier(string identifier) => $"{identifier}__pinned";
         public ByValueMarshalKindSupport SupportsByValueMarshalKind(ByValueContentsMarshalKind marshalKind, TypePositionInfo info, StubCodeContext context, out GeneratorDiagnostic? diagnostic)
         {
-            // Maybe should be done in the CharMarshallerFactory, but complexity and interdependence bleeds in if you do that
-            if (IsPinningPathSupported(info, context) && info.RefKind == RefKind.In)
-            {
-                diagnostic = new GeneratorDiagnostic.NotSupported(info, context)
-                {
-                    NotSupportedDetails = SR.InRefKindIsNotSupportedOnPinnedParameters
-                };
-                return ByValueMarshalKindSupport.NotSupported;
-            }
             return ByValueMarshalKindSupportDescriptor.Default.GetSupport(marshalKind, info, context, out diagnostic);
         }
 
index 7873781..c992f63 100644 (file)
@@ -110,7 +110,7 @@ namespace Microsoft.Interop
         {
             return
             info.ByValueContentsMarshalKind.HasFlag(ByValueContentsMarshalKind.Out)
-                && _byValueContentsMarshallingSupport.GetSupport(info.ByValueContentsMarshalKind, info, context, out _) == ByValueMarshalKindSupport.Supported
+                && _byValueContentsMarshallingSupport.GetSupport(info.ByValueContentsMarshalKind, info, context, out _) != ByValueMarshalKindSupport.NotSupported
                 && !info.IsByRef
                 && !_isPinned;
         }
index e5be70f..45ef61f 100644 (file)
@@ -105,14 +105,6 @@ namespace Microsoft.Interop
 
         public ByValueMarshalKindSupport SupportsByValueMarshalKind(ByValueContentsMarshalKind marshalKind, TypePositionInfo info, StubCodeContext context, out GeneratorDiagnostic? diagnostic)
         {
-            if (marshalKind is ByValueContentsMarshalKind.In)
-            {
-                diagnostic = new GeneratorDiagnostic.NotSupported(info, context)
-                {
-                    NotSupportedDetails = SR.InAttributeOnlyNotSupportedOnPinnedParameters
-                };
-                return ByValueMarshalKindSupport.NotSupported;
-            }
             return ByValueMarshalKindSupportDescriptor.PinnedParameter.GetSupport(marshalKind, info, context, out diagnostic);
         }
     }
index 75e9525..5157e3c 100644 (file)
@@ -79,14 +79,7 @@ namespace ComInterfaceGenerator.Tests
             Assert.Equal(data, value);
             obj.SwapArray(ref data, data.Length);
             obj.PassIn(in data, data.Length);
-        }
-
-        [Fact]
-        [ActiveIssue("https://github.com/dotnet/runtime/issues/89265")]
-        public void IIntArray_Failing()
-        {
-            var obj = CreateWrapper<IIntArrayImpl, IIntArray>();
-            int[] data = new int[] { 1, 2, 3 };
+            data = new int[] { 1, 2, 3 };
             obj.Double(data, data.Length);
             Assert.True(data is [2, 4, 6]);
         }
index ef9d76f..ee765a1 100644 (file)
@@ -746,14 +746,12 @@ namespace ComInterfaceGenerator.Unit.Tests
                     .WithLocation(0)
                     .WithArguments(SR.InAttributeOnlyNotSupportedOnPinnedParameters, paramName);
             yield return new object[] { ID(), codeSnippets.ByValueMarshallingOfType(inAttribute + constElementCount, "int[]", paramNameWithLocation), new DiagnosticResult[] {
-                inAttributeNotSupportedOnPinnedParameter
+                inAttributeIsDefaultDiagnostic
             }};
-            // blittable arrays don't support [In] only. Different diagnostics are issued because we can pin in one direction (managed->unmanaged)
-            // but not the other direction.
             yield return new object[] {
                 ID(),
                 codeSnippets.ByValueMarshallingOfType(inAttribute + constElementCount, "char[]", paramNameWithLocation, (StringMarshalling.Utf16, null)),
-                new DiagnosticResult[] { inAttributeNotSupportedOnPinnedParameter, inAttributeIsDefaultDiagnostic }
+                new DiagnosticResult[] { inAttributeIsDefaultDiagnostic }
             };
 
             // bools that are marshalled into a new array are in by default
@@ -782,12 +780,12 @@ namespace ComInterfaceGenerator.Unit.Tests
             yield return new object[] {
                 ID(),
                 codeSnippets.ByValueMarshallingOfType(inAttribute + outAttribute + constElementCount, "int[]", paramNameWithLocation),
-                new DiagnosticResult[] { inOutAttributeIsDefaultDiagnostic }
+                new DiagnosticResult[] { }
             };
             yield return new object[] {
                 ID(),
                 codeSnippets.ByValueMarshallingOfType(inAttribute + outAttribute + constElementCount, "char[]", paramNameWithLocation, (StringMarshalling.Utf16, null)),
-                new DiagnosticResult[] { inOutAttributeIsDefaultDiagnostic }
+                new DiagnosticResult[] { }
             };
 
             yield return new object[] {
index 1d3e9ac..ffb3fb5 100644 (file)
@@ -832,11 +832,12 @@ namespace LibraryImportGenerator.UnitTests
                     .WithLocation(1)
                     .WithArguments("ref return", "Basic.RefReadonlyReturn()"),
             } };
-            yield return new object[] { ID(), CodeSnippets.ByValueParameterWithModifier<int[]>("In"), new[]
+            yield return new object[] { ID(), CodeSnippets.ByValueParameterWithModifier<int[]>("{|#10:In|}"), new[]
             {
-                VerifyCS.Diagnostic(GeneratorDiagnostics.ParameterTypeNotSupportedWithDetails)
+                VerifyCS.Diagnostic(GeneratorDiagnostics.UnnecessaryParameterMarshallingInfo)
                     .WithLocation(0)
-                    .WithArguments(SR.InAttributeOnlyNotSupportedOnPinnedParameters, "p")
+                    .WithLocation(10)
+                    .WithArguments("[In] and [Out] attributes", "p", SR.InAttributeOnlyIsDefault)
             } };
         }
 
index a124086..438d589 100644 (file)
@@ -750,13 +750,13 @@ namespace LibraryImportGenerator.UnitTests
             // Blittable array
             yield return new object[] { ID(), CodeSnippets.ByValueParameterWithModifier<int[]>("{|#10:Out|}"), new DiagnosticResult[] { } };
 
-            yield return new object[] { ID(), CodeSnippets.ByValueParameterWithModifier<int[]>("{|#10:In|}, {|#11:Out|}"), new[]
-            {
+            yield return new object[] { ID(), CodeSnippets.ByValueParameterWithModifier<int[]>("{|#10:In|}, {|#11:Out|}"), new DiagnosticResult[] { } };
+
+            yield return new object[] { ID(), CodeSnippets.ByValueParameterWithModifier<int[]>("{|#10:In|}"), new DiagnosticResult[] {
                 VerifyCS.Diagnostic(GeneratorDiagnostics.UnnecessaryParameterMarshallingInfo)
                     .WithLocation(0)
                     .WithLocation(10)
-                    .WithLocation(11)
-                    .WithArguments("[In] and [Out] attributes", "p", SR.PinnedMarshallingIsInOutByDefault)
+                    .WithArguments("[In] and [Out] attributes", "p", SR.InAttributeOnlyIsDefault)
             } };
         }
 
diff --git a/src/libraries/System.Runtime.InteropServices/tests/TestAssets/SharedTypes/ComInterfaces/IStatelessPinnableCollectionBlittableElements.cs b/src/libraries/System.Runtime.InteropServices/tests/TestAssets/SharedTypes/ComInterfaces/IStatelessPinnableCollectionBlittableElements.cs
new file mode 100644 (file)
index 0000000..26dcc22
--- /dev/null
@@ -0,0 +1,156 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+using System;
+using System.Runtime.InteropServices;
+using System.Runtime.InteropServices.Marshalling;
+
+namespace SharedTypes.ComInterfaces
+{
+    [GeneratedComInterface]
+    [Guid("3BBB0C99-7D6C-4AD1-BE4C-ACB4C2127F02")]
+    internal partial interface IStatelessPinnableCollectionBlittableElements
+    {
+        void Method(
+            [MarshalUsing(CountElementName = nameof(size))] StatelessPinnableCollection<int> p,
+            int size);
+
+        void MethodIn(
+            [MarshalUsing(CountElementName = nameof(size))] in StatelessPinnableCollection<int> pIn,
+            in int size);
+
+        void MethodRef(
+            [MarshalUsing(CountElementName = nameof(size))] ref StatelessPinnableCollection<int> pRef,
+            int size);
+
+        void MethodOut(
+            [MarshalUsing(CountElementName = nameof(size))] out StatelessPinnableCollection<int> pOut,
+            out int size);
+
+        [return: MarshalUsing(CountElementName = nameof(size))]
+        StatelessPinnableCollection<int> Return(int size);
+
+        [PreserveSig]
+        [return: MarshalUsing(CountElementName = nameof(size))]
+        StatelessPinnableCollection<int> ReturnPreserveSig(int size);
+    }
+
+    [NativeMarshalling(typeof(StatelessPinnableCollectionMarshaller<,>))]
+    internal class StatelessPinnableCollection<T> where T : unmanaged
+    {
+    }
+
+    internal unsafe struct StatelessPinnableCollectionNative<T> where T : unmanaged
+    {
+        public static explicit operator StatelessPinnableCollectionNative<T>(void* ptr) => new StatelessPinnableCollectionNative<T>();
+        public static explicit operator void*(StatelessPinnableCollectionNative<T> ptr) => (void*)null;
+    }
+
+
+    [ContiguousCollectionMarshaller]
+    [CustomMarshaller(typeof(StatelessPinnableCollection<>), MarshalMode.ManagedToUnmanagedIn, typeof(StatelessPinnableCollectionMarshaller<,>.ManagedToUnmanaged))]
+    [CustomMarshaller(typeof(StatelessPinnableCollection<>), MarshalMode.UnmanagedToManagedOut, typeof(StatelessPinnableCollectionMarshaller<,>.ManagedToUnmanaged))]
+    [CustomMarshaller(typeof(StatelessPinnableCollection<>), MarshalMode.ElementIn, typeof(StatelessPinnableCollectionMarshaller<,>.ManagedToUnmanaged))]
+    [CustomMarshaller(typeof(StatelessPinnableCollection<>), MarshalMode.ManagedToUnmanagedOut, typeof(StatelessPinnableCollectionMarshaller<,>.UnmanagedToManaged))]
+    [CustomMarshaller(typeof(StatelessPinnableCollection<>), MarshalMode.UnmanagedToManagedIn, typeof(StatelessPinnableCollectionMarshaller<,>.UnmanagedToManaged))]
+    [CustomMarshaller(typeof(StatelessPinnableCollection<>), MarshalMode.ElementOut, typeof(StatelessPinnableCollectionMarshaller<,>.UnmanagedToManaged))]
+    [CustomMarshaller(typeof(StatelessPinnableCollection<>), MarshalMode.UnmanagedToManagedRef, typeof(StatelessPinnableCollectionMarshaller<,>.Bidirectional))]
+    [CustomMarshaller(typeof(StatelessPinnableCollection<>), MarshalMode.ManagedToUnmanagedRef, typeof(StatelessPinnableCollectionMarshaller<,>.Bidirectional))]
+    [CustomMarshaller(typeof(StatelessPinnableCollection<>), MarshalMode.ElementRef, typeof(StatelessPinnableCollectionMarshaller<,>.Bidirectional))]
+    internal static unsafe class StatelessPinnableCollectionMarshaller<T, TUnmanagedElement>
+        where T : unmanaged
+        where TUnmanagedElement : unmanaged
+    {
+        internal static class Bidirectional
+        {
+            public static StatelessPinnableCollectionNative<T> AllocateContainerForUnmanagedElements(StatelessPinnableCollection<T> managed, out int numElements)
+            {
+                throw new NotImplementedException();
+            }
+
+            public static StatelessPinnableCollection<T> AllocateContainerForManagedElements(StatelessPinnableCollectionNative<T> unmanaged, int numElements)
+            {
+                throw new NotImplementedException();
+            }
+
+            public static ReadOnlySpan<T> GetManagedValuesSource(StatelessPinnableCollection<T> managed)
+            {
+                throw new NotImplementedException();
+            }
+
+            public static Span<TUnmanagedElement> GetUnmanagedValuesDestination(StatelessPinnableCollectionNative<T> unmanaged, int numElements)
+            {
+                throw new NotImplementedException();
+            }
+
+            public static ReadOnlySpan<TUnmanagedElement> GetUnmanagedValuesSource(StatelessPinnableCollectionNative<T> unmanaged, int numElements)
+            {
+                throw new NotImplementedException();
+            }
+
+            public static Span<T> GetManagedValuesDestination(StatelessPinnableCollection<T> managed)
+            {
+                throw new NotImplementedException();
+            }
+
+            public static ref StatelessPinnableCollectionNative<T> GetPinnableReference(StatelessPinnableCollection<T> managed)
+            {
+                throw new NotImplementedException();
+            }
+
+            public static void Free(StatelessPinnableCollectionNative<T> unmanaged) { }
+        }
+
+        internal static class ManagedToUnmanaged
+        {
+            public static StatelessPinnableCollectionNative<T> AllocateContainerForUnmanagedElements(StatelessPinnableCollection<T> managed, out int numElements)
+            {
+                throw new NotImplementedException();
+            }
+
+            public static ReadOnlySpan<T> GetManagedValuesSource(StatelessPinnableCollection<T> managed)
+            {
+                throw new NotImplementedException();
+            }
+
+            public static Span<TUnmanagedElement> GetUnmanagedValuesDestination(StatelessPinnableCollectionNative<T> unmanaged, int numElements)
+            {
+                throw new NotImplementedException();
+            }
+
+            public static ref StatelessPinnableCollectionNative<T> GetPinnableReference(StatelessPinnableCollection<T> managed)
+            {
+                throw new NotImplementedException();
+            }
+
+            public static void Free(StatelessPinnableCollectionNative<T> unmanaged) => throw new NotImplementedException();
+        }
+
+        internal static class UnmanagedToManaged
+        {
+            public static StatelessPinnableCollection<T> AllocateContainerForManagedElements(StatelessPinnableCollectionNative<T> unmanaged, int numElements)
+            {
+                throw new NotImplementedException();
+            }
+
+            public static ReadOnlySpan<TUnmanagedElement> GetUnmanagedValuesSource(StatelessPinnableCollectionNative<T> unmanaged, int numElements)
+            {
+                throw new NotImplementedException();
+            }
+
+            // Should be removed: https://github.com/dotnet/runtime/issues/89885
+            public static Span<TUnmanagedElement> GetUnmanagedValuesDestination(StatelessPinnableCollectionNative<T> unmanaged, int numElements)
+            {
+                throw new NotImplementedException();
+            }
+
+            public static Span<T> GetManagedValuesDestination(StatelessPinnableCollection<T> managed)
+            {
+                throw new NotImplementedException();
+            }
+
+            public static void Free(StatelessPinnableCollectionNative<T> unmanaged) => throw new NotImplementedException();
+
+        }
+    }
+}