Add an option to FriendlyOverloads to request previous pointer overloads (#1524)

Author: jevansaks

src/Microsoft.Windows.CsWin32/Generator.FriendlyOverloads.cs

@@ -81,14 +81,33 @@ private IEnumerable<MethodDeclarationSyntax> DeclareFriendlyOverloads(MethodDefi
             }
         }
 
-        bool useSpansForPointers = this.canUseSpan;
-        foreach (MethodDeclarationSyntax method in this.DeclareFriendlyOverload(methodDefinition, externMethodDeclaration, declaringTypeName, overloadOf, helperMethodsAdded, avoidWinmdRootAlias, useSpansForPointers, omitOptionalParams: false))
+        bool improvePointersToSpansAndRefs = this.canUseSpan;
+        FriendlyMethodBookkeeping bookkeeping = new();
+        foreach (MethodDeclarationSyntax method in this.DeclareFriendlyOverload(methodDefinition, externMethodDeclaration, declaringTypeName, overloadOf, helperMethodsAdded, avoidWinmdRootAlias, improvePointersToSpansAndRefs, omitOptionalParams: false, bookkeeping))
         {
             yield return method;
         }
+
+        if (this.Options.FriendlyOverloads.IncludePointerOverloads && improvePointersToSpansAndRefs && bookkeeping.NumSpanByteParameters > 0)
+        {
+            // If we could use Span and _did_ use span Span and the pointer overloads were requested, then Generate overloads that use pointer types instead of Span<byte>/ReadOnlySpan<byte>.
+            foreach (MethodDeclarationSyntax method in this.DeclareFriendlyOverload(methodDefinition, externMethodDeclaration, declaringTypeName, overloadOf, helperMethodsAdded, avoidWinmdRootAlias, improvePointersToSpansAndRefs: false, omitOptionalParams: false))
+            {
+                yield return method;
+            }
+        }
     }
 
-    private IEnumerable<MethodDeclarationSyntax> DeclareFriendlyOverload(MethodDefinition methodDefinition, MethodDeclarationSyntax externMethodDeclaration, NameSyntax declaringTypeName, FriendlyOverloadOf overloadOf, HashSet<string> helperMethodsAdded, bool avoidWinmdRootAlias, bool useSpansForPointers, bool omitOptionalParams)
+    private IEnumerable<MethodDeclarationSyntax> DeclareFriendlyOverload(
+        MethodDefinition methodDefinition,
+        MethodDeclarationSyntax externMethodDeclaration,
+        NameSyntax declaringTypeName,
+        FriendlyOverloadOf overloadOf,
+        HashSet<string> helperMethodsAdded,
+        bool avoidWinmdRootAlias,
+        bool improvePointersToSpansAndRefs,
+        bool omitOptionalParams,
+        FriendlyMethodBookkeeping? bookkeeping = null)
     {
 #pragma warning disable SA1114 // Parameter list should follow declaration
         bool isReleaseMethod = this.MetadataIndex.ReleaseMethods.Contains(externMethodDeclaration.Identifier.ValueText);
@@ -123,6 +142,7 @@ private IEnumerable<MethodDeclarationSyntax> DeclareFriendlyOverload(MethodDefin
         bool minorSignatureChange = false; // Did the signature change but not enough that overload resolution would be confused?
         List<Parameter>? countOfBytesStructParameters = null;
         int numOptionalParams = 0;
+        int numSpanByteParameters = 0;
 
         foreach (ParameterHandle paramHandle in methodDefinition.GetParameters())
         {
@@ -177,12 +197,18 @@ private IEnumerable<MethodDeclarationSyntax> DeclareFriendlyOverload(MethodDefin
                 // If there's no MemorySize attribute, we may still need to keep this parameter as a pointer if it's a struct with a flexible array.
                 mustRemainAsPointer = parameterTypeInfo is PointerTypeHandleInfo { ElementType: HandleTypeHandleInfo pointedElement } && pointedElement.Generator.IsStructWithFlexibleArray(pointedElement);
             }
-            else if (!useSpansForPointers)
+            else if (!improvePointersToSpansAndRefs)
             {
                 // If we are generating the overload with pointers for memory sized params then also force them to pointers.
                 mustRemainAsPointer = true;
             }
 
+            // For compat with how out/ref parameters used to be generated, leave out/ref parameters as pointers if we're not tring to improve them.
+            if (isOptional && isOut && !isComOutPtr && !improvePointersToSpansAndRefs)
+            {
+                mustRemainAsPointer = true;
+            }
+
             IdentifierNameSyntax origName = IdentifierName(externParam.Identifier.ValueText);
 
             bool isArray = false;
@@ -220,7 +246,7 @@ private IEnumerable<MethodDeclarationSyntax> DeclareFriendlyOverload(MethodDefin
             }
 
             bool projectAsSpanBytes = false;
-            if (useSpansForPointers && IsVoidPtrOrPtrPtr(externParam.Type))
+            if (improvePointersToSpansAndRefs && IsVoidPtrOrPtrPtr(externParam.Type))
             {
                 // if it's memory-sized project as Span<byte>
                 if (memorySize is not null)
@@ -401,7 +427,7 @@ private IEnumerable<MethodDeclarationSyntax> DeclareFriendlyOverload(MethodDefin
                 arguments[param.SequenceNumber - 1] = Argument(typeDefHandleName);
             }
             else if ((externParam.Type is PointerTypeSyntax { ElementType: TypeSyntax ptrElementType }
-                && (!IsVoid(ptrElementType) || (useSpansForPointers && isArray))
+                && (!IsVoid(ptrElementType) || (improvePointersToSpansAndRefs && isArray))
                 && !this.IsInterface(parameterTypeInfo)) ||
                 externParam.Type is ArrayTypeSyntax)
             {
@@ -476,6 +502,11 @@ private IEnumerable<MethodDeclarationSyntax> DeclareFriendlyOverload(MethodDefin
                         fixedBlocks.Add(VariableDeclaration(PointerType(elementType)).AddVariables(
                             VariableDeclarator(localName.Identifier).WithInitializer(EqualsValueClause(origName))));
                         arguments[param.SequenceNumber - 1] = projectAsSpanBytes ? Argument(CastExpression(externParam.Type, localName)) : Argument(localName);
+
+                        if (projectAsSpanBytes)
+                        {
+                            numSpanByteParameters++;
+                        }
                     }
                     else if (isNullTerminated && isConst && parameters[param.SequenceNumber - 1].Type is PointerTypeSyntax { ElementType: PredefinedTypeSyntax { Keyword: { RawKind: (int)SyntaxKind.CharKeyword } } })
                     {
@@ -826,6 +857,7 @@ bool TryHandleCountParam(TypeSyntax elementType, bool nullableSource)
                             fixedBlocks.Add(VariableDeclaration(PointerType(byteSyntax)).AddVariables(
                                 VariableDeclarator(localName.Identifier).WithInitializer(EqualsValueClause(origName))));
                             arguments[param.SequenceNumber - 1] = Argument(CastExpression(externParam.Type, localName));
+                            numSpanByteParameters++;
                         }
                         else
                         {
@@ -898,7 +930,7 @@ bool TryHandleCountParam(TypeSyntax elementType, bool nullableSource)
                 : IdentifierName(externMethodDeclaration.Identifier);
             SyntaxTrivia leadingTrivia = Trivia(
                 DocumentationCommentTrivia(SyntaxKind.SingleLineDocumentationCommentTrivia).AddContent(
-                    XmlText("/// "),
+                    XmlText($"/// "),
                     XmlEmptyElement("inheritdoc").AddAttributes(XmlCrefAttribute(NameMemberCref(docRefExternName, ToCref(externMethodDeclaration.ParameterList)))),
                     XmlText().AddTextTokens(XmlTextNewLine("\n", continueXmlDocumentationComment: false))));
             InvocationExpressionSyntax externInvocation = InvocationExpression(
@@ -1003,12 +1035,17 @@ bool TryHandleCountParam(TypeSyntax elementType, bool nullableSource)
             friendlyDeclaration = friendlyDeclaration
                 .WithLeadingTrivia(leadingTrivia);
 
+            if (bookkeeping is not null)
+            {
+                bookkeeping.NumSpanByteParameters = numSpanByteParameters;
+            }
+
             yield return friendlyDeclaration;
 
             // We generated the main overload, but now see if we should generate another helper for things like SHGetFileInfo where
             // there is a parameter that's sized in bytes and for convenience you want to just use the struct and not cast between Span<byte>.
             // To avoid an explosion of overloads, just do this if there's one parameter of this kind.
-            if (useSpansForPointers && countOfBytesStructParameters?.Count == 1)
+            if (improvePointersToSpansAndRefs && countOfBytesStructParameters?.Count == 1)
             {
                 MethodDeclarationSyntax? structOverload = this.DeclareStructCountOfBytesFriendlyOverload(externMethodDeclaration, countOfBytesStructParameters, friendlyDeclaration);
                 if (structOverload is not null)
@@ -1018,10 +1055,10 @@ bool TryHandleCountParam(TypeSyntax elementType, bool nullableSource)
             }
         }
 
-        if (numOptionalParams > 0 && !omitOptionalParams)
+        if (numOptionalParams > 0 && !omitOptionalParams && improvePointersToSpansAndRefs)
         {
             // Generate overloads for optional parameters.
-            foreach (MethodDeclarationSyntax method in this.DeclareFriendlyOverload(methodDefinition, externMethodDeclaration, declaringTypeName, overloadOf, helperMethodsAdded, avoidWinmdRootAlias, useSpansForPointers, omitOptionalParams: true))
+            foreach (MethodDeclarationSyntax method in this.DeclareFriendlyOverload(methodDefinition, externMethodDeclaration, declaringTypeName, overloadOf, helperMethodsAdded, avoidWinmdRootAlias, improvePointersToSpansAndRefs, omitOptionalParams: true))
             {
                 yield return method;
             }
@@ -1163,4 +1200,9 @@ bool TryHandleCountParam(TypeSyntax elementType, bool nullableSource)
 
         return helper;
     }
+
+    private class FriendlyMethodBookkeeping
+    {
+        public int NumSpanByteParameters { get; set; } = 0;
+    }
 }

src/Microsoft.Windows.CsWin32/GeneratorOptions.cs

@@ -106,5 +106,11 @@ public record FriendlyOverloadOptions
         /// </summary>
         /// <value>The default value is <see langword="true" />.</value>
         public bool Enabled { get; set; } = true;
+
+        /// <summary>
+        /// Gets or sets a value indicating whether to also generate overloads that use pointer types for parameters that are [MemorySize] annotated buffers
+        /// which normally appear as spans.
+        /// </summary>
+        public bool IncludePointerOverloads { get; set; } = false;
     }
 }

src/Microsoft.Windows.CsWin32/settings.schema.json

@@ -50,6 +50,11 @@
           "description": "A value indicating whether to generate method overloads that may be easier to consume or be more idiomatic C#. These may use fewer pointers, accept or return SafeHandles, etc.",
           "type": "boolean",
           "default": true
+        },
+        "includePointerOverloads": {
+          "description": "A value indicating whether to also generate overloads that use pointer types for parameters that are [MemorySize] annotated buffers which normally appear as spans.",
+          "type": "boolean",
+          "default": false
         }
       }
     },

test/CsWin32Generator.Tests/CsWin32GeneratorFullTests.cs

@@ -17,12 +17,13 @@ public CsWin32GeneratorFullTests(ITestOutputHelper logger)
     [Trait("TestCategory", "FailsInCloudTest")] // these take ~4GB of memory to run.
     [InlineData("net8.0", LanguageVersion.CSharp12)]
     [InlineData("net9.0", LanguageVersion.CSharp13)]
-    public async Task FullGeneration(string tfm, LanguageVersion langVersion)
+    [InlineData("net9.0", LanguageVersion.CSharp13, true)]
+    public async Task FullGeneration(string tfm, LanguageVersion langVersion, bool includePointerOverloads = false)
     {
         this.fullGeneration = true;
         this.compilation = this.starterCompilations[tfm];
         this.parseOptions = this.parseOptions.WithLanguageVersion(langVersion);
-        this.nativeMethodsJson = "NativeMethods.EmitSingleFile.json";
+        this.nativeMethodsJson = includePointerOverloads ? "NativeMethods.IncludePointerOverloads.json" : "NativeMethods.EmitSingleFile.json";
         await this.InvokeGeneratorAndCompile($"FullGeneration_{tfm}_{langVersion}", TestOptions.None);
     }
 }

test/CsWin32Generator.Tests/CsWin32GeneratorTests.cs

@@ -184,27 +184,30 @@ public async Task TestGenerateCoCreateableClass()
         ["IEnumString", "Next", "this winmdroot.System.Com.IEnumString @this, Span<winmdroot.Foundation.PWSTR> rgelt, out uint pceltFetched"],
         ["PSCreateMemoryPropertyStore", "PSCreateMemoryPropertyStore", "in global::System.Guid riid, out void* ppv"],
         ["DeviceIoControl", "DeviceIoControl", "SafeHandle hDevice, uint dwIoControlCode, ReadOnlySpan<byte> lpInBuffer, Span<byte> lpOutBuffer, out uint lpBytesReturned, global::System.Threading.NativeOverlapped* lpOverlapped"],
+        ["DeviceIoControl", "DeviceIoControl", "SafeHandle hDevice, uint dwIoControlCode, ReadOnlySpan<byte> lpInBuffer, Span<byte> lpOutBuffer, out uint lpBytesReturned, global::System.Threading.NativeOverlapped* lpOverlapped", true, "NativeMethods.IncludePointerOverloads.json"],
+        ["NtQueryObject", "NtQueryObject", "global::Windows.Win32.Foundation.HANDLE Handle, winmdroot.Foundation.OBJECT_INFORMATION_CLASS ObjectInformationClass, Span<byte> ObjectInformation, out uint ReturnLength"],
     ];
 
     [Theory]
     [MemberData(nameof(TestSignatureData))]
-    public async Task VerifySignature(string api, string member, string signature, bool assertPresent = true)
+    public async Task VerifySignature(string api, string member, string signature, bool assertPresent = true, string? nativeMethodsJson = null)
     {
-        await this.VerifySignatureWorker(api, member, signature, assertPresent, "net9.0");
+        await this.VerifySignatureWorker(api, member, signature, assertPresent, "net9.0", nativeMethodsJson);
     }
 
     [Theory]
     [InlineData("InitializeAcl", "InitializeAcl", "out winmdroot.Security.ACL pAcl, winmdroot.Security.ACE_REVISION dwAclRevision", false)]
-    public async Task VerifySignatureNet472(string api, string member, string signature, bool assertPresent = true)
+    public async Task VerifySignatureNet472(string api, string member, string signature, bool assertPresent = true, string? nativeMethodsJson = null)
     {
-        await this.VerifySignatureWorker(api, member, signature, assertPresent, "net472");
+        await this.VerifySignatureWorker(api, member, signature, assertPresent, "net472", nativeMethodsJson);
     }
 
-    private async Task VerifySignatureWorker(string api, string member, string signature, bool assertPresent, string tfm)
+    private async Task VerifySignatureWorker(string api, string member, string signature, bool assertPresent, string tfm, string? nativeMethodsJson)
     {
         this.tfm = tfm;
         this.compilation = this.starterCompilations[tfm];
         this.nativeMethods.Add(api);
+        this.nativeMethodsJson = nativeMethodsJson;
 
         // Make a unique name based on the signature
         await this.InvokeGeneratorAndCompile($"{api}_{member}_{tfm}_{signature.Select(x => (int)x).Aggregate((x, y) => x + y).ToString("X")}");
@@ -272,24 +275,25 @@ private async Task VerifySignatureWorker(string api, string member, string signa
         ["GetModuleFileName", "Should have a friendly Span overload"],
         ["PdhGetCounterInfo", "Optional out parameter omission conflicts with other overload"],
         ["RtlUpcaseUnicodeChar", "char parameter should not get CharSet marshalling in AOT"],
-        ["CryptGetAsyncParam", "Has optional unmanaged delegate out param"]
+        ["CryptGetAsyncParam", "Has optional unmanaged delegate out param"],
+        ["NtQueryObject", "Verify pointer overloads and optional parameters", TestOptions.None, "NativeMethods.IncludePointerOverloads.json"],
     ];
 
     [Theory]
     [MemberData(nameof(TestApiData))]
-    public async Task TestGenerateApi(string api, string purpose, TestOptions options = TestOptions.None)
+    public async Task TestGenerateApi(string api, string purpose, TestOptions options = TestOptions.None, string? nativeMethodsJson = null)
     {
-        await this.TestGenerateApiWorker(api, purpose, options, "net9.0");
+        await this.TestGenerateApiWorker(api, purpose, options, "net9.0", nativeMethodsJson);
     }
 
     [Theory]
     [MemberData(nameof(TestApiData))]
-    public async Task TestGenerateApiNet8(string api, string purpose, TestOptions options = TestOptions.None)
+    public async Task TestGenerateApiNet8(string api, string purpose, TestOptions options = TestOptions.None, string? nativeMethodsJson = null)
     {
-        await this.TestGenerateApiWorker(api, purpose, options, "net8.0");
+        await this.TestGenerateApiWorker(api, purpose, options, "net8.0", nativeMethodsJson);
     }
 
-    private async Task TestGenerateApiWorker(string api, string purpose, TestOptions options, string tfm)
+    private async Task TestGenerateApiWorker(string api, string purpose, TestOptions options, string tfm, string? nativeMethodsJson)
     {
         LanguageVersion langVersion = (tfm == "net8.0") ? LanguageVersion.CSharp12 : LanguageVersion.CSharp13;
 
@@ -298,6 +302,7 @@ private async Task TestGenerateApiWorker(string api, string purpose, TestOptions
         this.parseOptions = this.parseOptions.WithLanguageVersion(langVersion);
         this.Logger.WriteLine($"Testing {api} - {tfm} - {purpose}");
         this.nativeMethods.Add(api);
+        this.nativeMethodsJson = nativeMethodsJson;
         await this.InvokeGeneratorAndCompile($"Test_{api}_{tfm}", options);
     }
 

test/CsWin32Generator.Tests/TestContent/NativeMethods.IncludePointerOverloads.json

@@ -0,0 +1,7 @@
+{
+  "$schema": "..\\..\\..\\src\\Microsoft.Windows.CsWin32\\settings.schema.json",
+  "emitSingleFile": true,
+  "friendlyOverloads": {
+    "includePointerOverloads": true
+  }
+}

test/Microsoft.Windows.CsWin32.Tests/FullGenerationTests.cs

@@ -56,10 +56,11 @@ public void Constants()
     public void ExternMethods(
         MarshalingOptions marshaling,
         bool useIntPtrForComOutPtr,
+        bool includePointerOverloads,
         [CombinatorialMemberData(nameof(SpecificCpuArchitectures))] Platform platform,
         [CombinatorialMemberData(nameof(TFMDataNoNetFx35))] string tfm)
     {
-        this.TestHelper(OptionsForMarshaling(marshaling, useIntPtrForComOutPtr), platform, tfm, generator => generator.GenerateAllExternMethods(CancellationToken.None));
+        this.TestHelper(OptionsForMarshaling(marshaling, useIntPtrForComOutPtr, includePointerOverloads), platform, tfm, generator => generator.GenerateAllExternMethods(CancellationToken.None));
     }
 
     [Fact]
@@ -68,14 +69,18 @@ public void Macros()
         this.TestHelper(new GeneratorOptions(), Platform.X64, DefaultTFM, generator => generator.GenerateAllMacros(CancellationToken.None));
     }
 
-    private static GeneratorOptions OptionsForMarshaling(MarshalingOptions marshaling, bool useIntPtrForComOutPtr) => new()
+    private static GeneratorOptions OptionsForMarshaling(MarshalingOptions marshaling, bool useIntPtrForComOutPtr, bool includePointerOverloads = false) => new()
     {
         AllowMarshaling = marshaling >= MarshalingOptions.MarshalingWithoutSafeHandles,
         UseSafeHandles = marshaling == MarshalingOptions.FullMarshaling,
         ComInterop = new()
         {
             UseIntPtrForComOutPointers = useIntPtrForComOutPtr,
         },
+        FriendlyOverloads = new()
+        {
+            IncludePointerOverloads = includePointerOverloads,
+        },
     };
 
     private void TestHelper(GeneratorOptions generatorOptions, Platform platform, string targetFramework, Action<IGenerator> generationCommands)