Merge pull request #767 from microsoft/fix752

Emit a stable reference to the interface guid
microsoft · Nov 12, 2022 · 8b9fa8b · 8b9fa8b
2 parents f1fbefd + 1a572bf
commit 8b9fa8b
Show file tree

Hide file tree

Showing 11 changed files with 171 additions and 44 deletions.
diff --git a/.devcontainer/Dockerfile b/.devcontainer/Dockerfile
@@ -1,4 +1,4 @@
-FROM mcr.microsoft.com/dotnet/sdk:6.0.401-focal
+FROM mcr.microsoft.com/dotnet/sdk:7.0.100-jammy
 
 # Installing mono makes `dotnet test` work without errors even for net472.
 # But installing it takes a long time, so it's excluded by default.

diff --git a/Directory.Build.props b/Directory.Build.props
@@ -5,7 +5,7 @@
     <BaseIntermediateOutputPath>$(RepoRootPath)obj\$([MSBuild]::MakeRelative($(RepoRootPath), $(MSBuildProjectDirectory)))\</BaseIntermediateOutputPath>
     <BaseOutputPath Condition=" '$(BaseOutputPath)' == '' ">$(RepoRootPath)bin\$(MSBuildProjectName)\</BaseOutputPath>
     <PackageOutputPath>$(RepoRootPath)bin\Packages\$(Configuration)\</PackageOutputPath>
-    <LangVersion>10.0</LangVersion>
+    <LangVersion>11.0</LangVersion>
     <ImplicitUsings>true</ImplicitUsings>
     <Nullable>enable</Nullable>
     <AnalysisLevel>latest</AnalysisLevel>

diff --git a/Microsoft.Windows.CsWin32.sln b/Microsoft.Windows.CsWin32.sln
@@ -1,7 +1,7 @@
 
 Microsoft Visual Studio Solution File, Format Version 12.00
-# Visual Studio Version 16
-VisualStudioVersion = 16.0.29322.22
+# Visual Studio Version 17
+VisualStudioVersion = 17.5.33110.383
 MinimumVisualStudioVersion = 15.0.26124.0
 Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "Solution Items", "Solution Items", "{1CE9670B-D5FF-46A7-9D00-24E70E6ED48B}"
 	ProjectSection(SolutionItems) = preProject
@@ -27,6 +27,7 @@ Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "test", "test", "{36CCE840-6
 		test\.editorconfig = test\.editorconfig
 		test\Directory.Build.props = test\Directory.Build.props
 		test\Directory.Build.targets = test\Directory.Build.targets
+		test\GenerationSandbox.props = test\GenerationSandbox.props
 	EndProjectSection
 EndProject
 Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Microsoft.Windows.CsWin32", "src\Microsoft.Windows.CsWin32\Microsoft.Windows.CsWin32.csproj", "{E3E96466-44B6-41AF-BBC8-9D30183ED8A9}"
@@ -41,6 +42,8 @@ Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "SpellChecker", "test\SpellC
 EndProject
 Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "WinRTInteropTest", "test\WinRTInteropTest\WinRTInteropTest.csproj", "{0E067B66-C2EC-4106-87D2-5310CFCDC5B8}"
 EndProject
+Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "GenerationSandbox.Unmarshalled.Tests", "test\GenerationSandbox.Unmarshalled.Tests\GenerationSandbox.Unmarshalled.Tests.csproj", "{3D303454-7DB0-4F9F-BD9E-07F09D3C70E3}"
+EndProject
 Global
 	GlobalSection(SolutionConfigurationPlatforms) = preSolution
 		Debug|Any CPU = Debug|Any CPU
@@ -95,6 +98,14 @@ Global
 		{0E067B66-C2EC-4106-87D2-5310CFCDC5B8}.Release|Any CPU.ActiveCfg = Release|Any CPU
 		{0E067B66-C2EC-4106-87D2-5310CFCDC5B8}.Release|Any CPU.Build.0 = Release|Any CPU
 		{0E067B66-C2EC-4106-87D2-5310CFCDC5B8}.Release|NonWindows.ActiveCfg = Release|Any CPU
+		{3D303454-7DB0-4F9F-BD9E-07F09D3C70E3}.Debug|Any CPU.ActiveCfg = Debug|Any CPU
+		{3D303454-7DB0-4F9F-BD9E-07F09D3C70E3}.Debug|Any CPU.Build.0 = Debug|Any CPU
+		{3D303454-7DB0-4F9F-BD9E-07F09D3C70E3}.Debug|NonWindows.ActiveCfg = Debug|Any CPU
+		{3D303454-7DB0-4F9F-BD9E-07F09D3C70E3}.Debug|NonWindows.Build.0 = Debug|Any CPU
+		{3D303454-7DB0-4F9F-BD9E-07F09D3C70E3}.Release|Any CPU.ActiveCfg = Release|Any CPU
+		{3D303454-7DB0-4F9F-BD9E-07F09D3C70E3}.Release|Any CPU.Build.0 = Release|Any CPU
+		{3D303454-7DB0-4F9F-BD9E-07F09D3C70E3}.Release|NonWindows.ActiveCfg = Release|Any CPU
+		{3D303454-7DB0-4F9F-BD9E-07F09D3C70E3}.Release|NonWindows.Build.0 = Release|Any CPU
 	EndGlobalSection
 	GlobalSection(SolutionProperties) = preSolution
 		HideSolutionNode = FALSE
@@ -106,6 +117,7 @@ Global
 		{7E8A5179-F94C-410F-8BBE-FDAAA95A19C3} = {36CCE840-6FE5-4DB9-A8D5-8CF3CB6D342A}
 		{744BE74F-8C4A-49E8-9683-52D987224285} = {36CCE840-6FE5-4DB9-A8D5-8CF3CB6D342A}
 		{0E067B66-C2EC-4106-87D2-5310CFCDC5B8} = {36CCE840-6FE5-4DB9-A8D5-8CF3CB6D342A}
+		{3D303454-7DB0-4F9F-BD9E-07F09D3C70E3} = {36CCE840-6FE5-4DB9-A8D5-8CF3CB6D342A}
 	EndGlobalSection
 	GlobalSection(ExtensibilityGlobals) = postSolution
 		SolutionGuid = {E3944F6A-384B-4B0F-B93F-3BD513DC57BD}

diff --git a/global.json b/global.json
@@ -1,6 +1,6 @@
 {
   "sdk": {
-    "version": "6.0.401",
+    "version": "7.0.100",
     "rollForward": "patch",
     "allowPrerelease": false
   }

diff --git a/src/Microsoft.Windows.CsWin32/Generator.cs b/src/Microsoft.Windows.CsWin32/Generator.cs
@@ -2299,6 +2299,18 @@ private static AttributeSyntax StructLayout(TypeAttributes typeAttributes, TypeL
         return structLayoutAttribute;
     }
 
+    private static AttributeSyntax MethodImpl(MethodImplOptions options)
+    {
+        if (options != MethodImplOptions.AggressiveInlining)
+        {
+            throw new NotImplementedException();
+        }
+
+        AttributeSyntax attribute = Attribute(IdentifierName("MethodImpl"))
+            .AddArgumentListArguments(AttributeArgument(MemberAccessExpression(SyntaxKind.SimpleMemberAccessExpression, IdentifierName(nameof(MethodImplOptions)), IdentifierName(nameof(MethodImplOptions.AggressiveInlining)))));
+        return attribute;
+    }
+
     private static AttributeSyntax GUID(Guid guid)
     {
         return Attribute(IdentifierName("Guid")).AddArgumentListArguments(
@@ -2473,6 +2485,25 @@ private static bool IsAnsiFunction(string methodName)
         return false;
     }
 
+    /// <summary>
+    /// Creates the syntax for creating a new byte array populated with the specified data.
+    /// e.g. <c>new byte[] { 0x01, 0x02 }</c>.
+    /// </summary>
+    /// <param name="bytes">The content of the array.</param>
+    /// <returns>The array creation syntax.</returns>
+    private static ArrayCreationExpressionSyntax NewByteArray(ReadOnlySpan<byte> bytes)
+    {
+        ExpressionSyntax[] elements = new ExpressionSyntax[bytes.Length];
+        for (int i = 0; i < bytes.Length; i++)
+        {
+            elements[i] = LiteralExpression(SyntaxKind.NumericLiteralExpression, Literal(ToHex(bytes[i]), bytes[i]));
+        }
+
+        return ArrayCreationExpression(
+            ArrayType(PredefinedType(Token(SyntaxKind.ByteKeyword))).AddRankSpecifiers(ArrayRankSpecifier()),
+            InitializerExpression(SyntaxKind.ArrayInitializerExpression, SeparatedList(elements)));
+    }
+
     private static unsafe string ToHex<T>(T value)
         where T : unmanaged
     {
@@ -3894,7 +3925,7 @@ StatementSyntax ThrowOnHRFailure(ExpressionSyntax hrExpression) => ExpressionSta
         return ifaceDeclaration;
     }
 
-    private (List<MemberDeclarationSyntax> Members, List<BaseTypeSyntax> BaseTypes) DeclareStaticCOMInterfaceMembers(CustomAttribute? guidAttribute)
+    private unsafe (List<MemberDeclarationSyntax> Members, List<BaseTypeSyntax> BaseTypes) DeclareStaticCOMInterfaceMembers(CustomAttribute? guidAttribute)
     {
         List<MemberDeclarationSyntax> members = new();
         List<BaseTypeSyntax> baseTypes = new();
@@ -3917,12 +3948,50 @@ StatementSyntax ThrowOnHRFailure(ExpressionSyntax hrExpression) => ExpressionSta
             {
                 baseTypes.Add(SimpleBaseType(IComIIDGuidInterfaceName));
 
-                // static ref readonly Guid IComIID.Guid => ref IID_Guid;
+                IdentifierNameSyntax dataLocal = IdentifierName("data");
+
+                // Rather than just `return ref IID_Guid`, which returns a pointer to a 'movable' field,
+                // We leverage C# syntax that we know the modern C# compiler will turn into a pointer directly into the dll image,
+                // so that the pointer does not move.
+                // This does rely on at least the generated code running on a little endian machine, since we're laying raw bytes on top of integer fields.
+                // But at the moment, we also assume this source generator is running on little endian for convenience for the reverse operation.
+                if (!BitConverter.IsLittleEndian)
+                {
+                    throw new NotSupportedException("Conversion from big endian to little endian is not implemented.");
+                }
+
+                // ReadOnlySpan<byte> data = new byte[] { ... };
+                ReadOnlySpan<byte> guidBytes = new((byte*)&guidAttributeValue, sizeof(Guid));
+                LocalDeclarationStatementSyntax dataDecl = LocalDeclarationStatement(
+                    VariableDeclaration(MakeReadOnlySpanOfT(PredefinedType(Token(SyntaxKind.ByteKeyword)))).AddVariables(
+                        VariableDeclarator(dataLocal.Identifier).WithInitializer(EqualsValueClause(NewByteArray(guidBytes)))));
+
+                // return ref Unsafe.As<byte, Guid>(ref MemoryMarshal.GetReference(data));
+                ReturnStatementSyntax returnStatement = ReturnStatement(RefExpression(
+                    InvocationExpression(
+                        MemberAccessExpression(
+                            SyntaxKind.SimpleMemberAccessExpression,
+                            IdentifierName(nameof(Unsafe)),
+                            GenericName(nameof(Unsafe.As)).AddTypeArgumentListArguments(PredefinedType(Token(SyntaxKind.ByteKeyword)), IdentifierName(nameof(Guid)))),
+                        ArgumentList().AddArguments(
+                            Argument(
+                                InvocationExpression(
+                                    MemberAccessExpression(SyntaxKind.SimpleMemberAccessExpression, IdentifierName(nameof(MemoryMarshal)), IdentifierName(nameof(MemoryMarshal.GetReference))),
+                                    ArgumentList(SingletonSeparatedList(Argument(dataLocal)))))
+                            .WithRefOrOutKeyword(Token(SyntaxKind.RefKeyword))))));
+
+                // The native assembly code for this property getter is just a `mov` and a `ret.
+                // For our callers to also enjoy just the `mov` instruction, we have to attribute for aggressive inlining.
+                // [MethodImpl(MethodImplOptions.AggressiveInlining)]
+                AttributeListSyntax methodImplAttr = AttributeList().AddAttributes(MethodImpl(MethodImplOptions.AggressiveInlining));
+
+                BlockSyntax getBody = Block(dataDecl, returnStatement);
+
+                // static ref readonly Guid IComIID.Guid { get { ... } }
                 PropertyDeclarationSyntax guidProperty = PropertyDeclaration(IdentifierName(nameof(Guid)).WithTrailingTrivia(Space), ComIIDGuidPropertyName.Identifier)
                     .WithExplicitInterfaceSpecifier(ExplicitInterfaceSpecifier(IComIIDGuidInterfaceName))
                     .AddModifiers(TokenWithSpace(SyntaxKind.StaticKeyword), TokenWithSpace(SyntaxKind.RefKeyword), TokenWithSpace(SyntaxKind.ReadOnlyKeyword))
-                    .WithExpressionBody(ArrowExpressionClause(RefExpression(iidGuidFieldName)))
-                    .WithSemicolonToken(Semicolon);
+                    .WithAccessorList(AccessorList().AddAccessors(AccessorDeclaration(SyntaxKind.GetAccessorDeclaration, getBody).AddAttributeLists(methodImplAttr)));
                 members.Add(guidProperty);
             }
         }
@@ -6225,7 +6294,8 @@ private bool TryDeclareCOMGuidInterfaceIfNecessary()
                     TokenWithSpace(SyntaxKind.AbstractKeyword),
                     TokenWithSpace(SyntaxKind.RefKeyword),
                     TokenWithSpace(SyntaxKind.ReadOnlyKeyword))
-                .WithAccessorList(AccessorList().AddAccessors(AccessorDeclaration(SyntaxKind.GetAccessorDeclaration).WithSemicolonToken(Semicolon)));
+                .WithAccessorList(AccessorList().AddAccessors(AccessorDeclaration(SyntaxKind.GetAccessorDeclaration).WithSemicolonToken(Semicolon)))
+                .WithLeadingTrivia(ParseLeadingTrivia($"/// <summary>The IID guid for this interface.</summary>\n/// <remarks>The <see cref=\"Guid\" /> reference that is returned comes from a permanent memory address, and is therefore safe to convert to a pointer and pass around or hold long-term.</remarks>\n"));
 
             // internal interface IComIID { ... }
             InterfaceDeclarationSyntax ifaceDecl = InterfaceDeclaration(IComIIDGuidInterfaceName.Identifier)

diff --git a/test/GenerationSandbox.Tests/GenerationSandbox.Tests.csproj b/test/GenerationSandbox.Tests/GenerationSandbox.Tests.csproj
@@ -1,41 +1,8 @@
 <Project Sdk="Microsoft.NET.Sdk">
-  <Import Project="..\..\src\Microsoft.Windows.CsWin32\build\Microsoft.Windows.CsWin32.props" />
-
-  <PropertyGroup>
-    <TargetFrameworks>net6.0-windows7.0;net472</TargetFrameworks>
-    <RootNamespace />
-    <PlatformTarget>x64</PlatformTarget>
-  </PropertyGroup>
-
-  <ItemGroup>
-    <Using Include="Xunit" />
-    <Using Include="Xunit.Abstractions" />
-  </ItemGroup>
-
-  <ItemGroup>
-    <ProjectReference Include="..\..\src\Microsoft.Windows.CsWin32\Microsoft.Windows.CsWin32.csproj" OutputItemType="Analyzer" ReferenceOutputAssembly="false" />
-    <Analyzer Include="$(RepoRootPath)\bin\Microsoft.Windows.CsWin32\$(Configuration)\netstandard2.0\System.Text.Json.dll" />
-    <Analyzer Include="$(RepoRootPath)\bin\Microsoft.Windows.CsWin32\$(Configuration)\netstandard2.0\Microsoft.Bcl.AsyncInterfaces.dll" />
-    <Analyzer Include="$(RepoRootPath)\bin\Microsoft.Windows.CsWin32\$(Configuration)\netstandard2.0\System.Text.Encodings.Web.dll" />
-    <Analyzer Include="$(RepoRootPath)\bin\Microsoft.Windows.CsWin32\$(Configuration)\netstandard2.0\Microsoft.Windows.SDK.Win32Docs.dll" />
-    <Analyzer Include="$(RepoRootPath)\bin\Microsoft.Windows.CsWin32\$(Configuration)\netstandard2.0\MessagePack.dll" />
-    <Analyzer Include="$(RepoRootPath)\bin\Microsoft.Windows.CsWin32\$(Configuration)\netstandard2.0\MessagePack.Annotations.dll" />
-  </ItemGroup>
+  <Import Project="..\GenerationSandbox.props" />
 
   <ProjectExtensions>
     <VisualStudio><UserProperties nativemethods_1json__JsonSchema="..\..\src\Microsoft.Windows.CsWin32\settings.schema.json" /></VisualStudio>
   </ProjectExtensions>
 
-  <ItemGroup>
-    <PackageReference Include="Microsoft.Win32.Registry" Version="5.0.0" />
-    <PackageReference Include="Microsoft.Windows.SDK.Win32Metadata" Version="$(MetadataVersion)" GeneratePathProperty="true" PrivateAssets="none" />
-    <!-- <PackageReference Include="Microsoft.Dia.Win32Metadata" Version="$(DiaMetadataVersion)" PrivateAssets="none" /> -->
-    <PackageReference Include="coverlet.msbuild" Version="3.2.0" />
-    <PackageReference Include="Microsoft.NET.Test.Sdk" Version="17.3.2" />
-    <PackageReference Include="System.Memory" Version="4.5.5" />
-    <PackageReference Include="System.Runtime.CompilerServices.Unsafe" Version="6.0.0" />
-    <PackageReference Include="xunit" Version="2.4.2" />
-    <PackageReference Include="xunit.runner.visualstudio" Version="2.4.5" />
-  </ItemGroup>
-
 </Project>
diff --git a/test/GenerationSandbox.Unmarshalled.Tests/COMTests.cs b/test/GenerationSandbox.Unmarshalled.Tests/COMTests.cs
@@ -0,0 +1,23 @@
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// Licensed under the MIT license. See LICENSE file in the project root for full license information.
+
+#pragma warning disable IDE0005
+
+using Windows.Win32;
+using Windows.Win32.System.Com;
+
+public class COMTests
+{
+#if NET7_0_OR_GREATER
+    [Fact]
+    public void COMStaticGuid()
+    {
+        Assert.Equal(typeof(IPersistFile).GUID, IPersistFile.IID_Guid);
+        Assert.Equal(typeof(IPersistFile).GUID, GetGuid<IPersistFile>());
+    }
+
+    private static Guid GetGuid<T>()
+        where T : IComIID
+        => T.Guid;
+#endif
+}
diff --git a/test/GenerationSandbox.Unmarshalled.Tests/GenerationSandbox.Unmarshalled.Tests.csproj b/test/GenerationSandbox.Unmarshalled.Tests/GenerationSandbox.Unmarshalled.Tests.csproj
@@ -0,0 +1,11 @@
+<Project Sdk="Microsoft.NET.Sdk">
+  <Import Project="..\GenerationSandbox.props" />
+  <ItemGroup>
+    <None Remove="NativeMethods.json" />
+    <None Remove="NativeMethods.txt" />
+  </ItemGroup>
+  <ItemGroup>
+    <AdditionalFiles Include="NativeMethods.json" />
+    <AdditionalFiles Include="NativeMethods.txt" />
+  </ItemGroup>
+</Project>
diff --git a/test/GenerationSandbox.Unmarshalled.Tests/NativeMethods.json b/test/GenerationSandbox.Unmarshalled.Tests/NativeMethods.json
@@ -0,0 +1,6 @@
+{
+  "$schema": "..\\..\\src\\Microsoft.Windows.CsWin32\\settings.schema.json",
+  "emitSingleFile": true,
+  "multiTargetingFriendlyAPIs": true,
+  "allowMarshaling": false
+}
diff --git a/test/GenerationSandbox.Unmarshalled.Tests/NativeMethods.txt b/test/GenerationSandbox.Unmarshalled.Tests/NativeMethods.txt
@@ -0,0 +1 @@
+IPersistFile
diff --git a/test/GenerationSandbox.props b/test/GenerationSandbox.props
@@ -0,0 +1,37 @@
+<Project>
+  <Import Project="$(RepoRootPath)src\Microsoft.Windows.CsWin32\build\Microsoft.Windows.CsWin32.props" />
+
+  <PropertyGroup>
+    <TargetFrameworks>net7.0-windows7.0;net6.0-windows7.0;net472</TargetFrameworks>
+    <RootNamespace />
+    <PlatformTarget>x64</PlatformTarget>
+  </PropertyGroup>
+
+  <ItemGroup>
+    <Using Include="Xunit" />
+    <Using Include="Xunit.Abstractions" />
+  </ItemGroup>
+
+  <ItemGroup>
+    <ProjectReference Include="$(RepoRootPath)src\Microsoft.Windows.CsWin32\Microsoft.Windows.CsWin32.csproj" OutputItemType="Analyzer" ReferenceOutputAssembly="false" />
+    <Analyzer Include="$(RepoRootPath)bin\Microsoft.Windows.CsWin32\$(Configuration)\netstandard2.0\System.Text.Json.dll" />
+    <Analyzer Include="$(RepoRootPath)bin\Microsoft.Windows.CsWin32\$(Configuration)\netstandard2.0\Microsoft.Bcl.AsyncInterfaces.dll" />
+    <Analyzer Include="$(RepoRootPath)bin\Microsoft.Windows.CsWin32\$(Configuration)\netstandard2.0\System.Text.Encodings.Web.dll" />
+    <Analyzer Include="$(RepoRootPath)bin\Microsoft.Windows.CsWin32\$(Configuration)\netstandard2.0\Microsoft.Windows.SDK.Win32Docs.dll" />
+    <Analyzer Include="$(RepoRootPath)bin\Microsoft.Windows.CsWin32\$(Configuration)\netstandard2.0\MessagePack.dll" />
+    <Analyzer Include="$(RepoRootPath)bin\Microsoft.Windows.CsWin32\$(Configuration)\netstandard2.0\MessagePack.Annotations.dll" />
+  </ItemGroup>
+
+  <ItemGroup>
+    <PackageReference Include="Microsoft.Win32.Registry" Version="5.0.0" />
+    <PackageReference Include="Microsoft.Windows.SDK.Win32Metadata" Version="$(MetadataVersion)" GeneratePathProperty="true" PrivateAssets="none" />
+    <!-- <PackageReference Include="Microsoft.Dia.Win32Metadata" Version="$(DiaMetadataVersion)" PrivateAssets="none" /> -->
+    <PackageReference Include="coverlet.msbuild" Version="3.2.0" />
+    <PackageReference Include="Microsoft.NET.Test.Sdk" Version="17.3.2" />
+    <PackageReference Include="System.Memory" Version="4.5.5" />
+    <PackageReference Include="System.Runtime.CompilerServices.Unsafe" Version="6.0.0" />
+    <PackageReference Include="xunit" Version="2.4.2" />
+    <PackageReference Include="xunit.runner.visualstudio" Version="2.4.5" />
+  </ItemGroup>
+
+</Project>