From ff7a75790e92a45243e16d51f72cd869a2fbbdaa Mon Sep 17 00:00:00 2001 From: Andrew Arnott Date: Mon, 16 May 2022 10:42:04 -0600 Subject: [PATCH 1/3] Move `TryGetTypeDefHandle` to `MetadataIndex` This should improve perf since the lookup cache can be shared across generator runs. --- src/Microsoft.Windows.CsWin32/Generator.cs | 51 +------------------ .../MetadataIndex.cs | 49 ++++++++++++++++++ 2 files changed, 51 insertions(+), 49 deletions(-) diff --git a/src/Microsoft.Windows.CsWin32/Generator.cs b/src/Microsoft.Windows.CsWin32/Generator.cs index e71c276e..3f116b51 100644 --- a/src/Microsoft.Windows.CsWin32/Generator.cs +++ b/src/Microsoft.Windows.CsWin32/Generator.cs @@ -293,8 +293,6 @@ public class Generator : IDisposable private readonly TypeSyntaxSettings functionPointerTypeSettings; private readonly TypeSyntaxSettings errorMessageTypeSettings; - private readonly Dictionary refToDefCache = new(); - private readonly GeneratorOptions options; private readonly CSharpCompilation? compilation; private readonly CSharpParseOptions? parseOptions; @@ -1816,7 +1814,7 @@ internal bool TryGetTypeDefHandle(TypeReferenceHandle typeRefHandle, out Qualifi return this.SuperGenerator.TryGetTypeDefinitionHandle(new QualifiedTypeReferenceHandle(this, typeRefHandle), out typeDefHandle); } - if (this.TryGetTypeDefHandle(typeRefHandle, out TypeDefinitionHandle localTypeDefHandle)) + if (this.MetadataIndex.TryGetTypeDefHandle(typeRefHandle, out TypeDefinitionHandle localTypeDefHandle)) { typeDefHandle = new QualifiedTypeDefinitionHandle(this, localTypeDefHandle); return true; @@ -1826,52 +1824,7 @@ internal bool TryGetTypeDefHandle(TypeReferenceHandle typeRefHandle, out Qualifi return false; } - /// - /// Attempts to translate a to a . - /// - /// The reference handle. - /// Receives the type def handle, if one was discovered. - /// if a TypeDefinition was found; otherwise . - internal bool TryGetTypeDefHandle(TypeReferenceHandle typeRefHandle, out TypeDefinitionHandle typeDefHandle) - { - if (this.refToDefCache.TryGetValue(typeRefHandle, out typeDefHandle)) - { - return !typeDefHandle.IsNil; - } - - TypeReference typeRef = this.Reader.GetTypeReference(typeRefHandle); - if (typeRef.ResolutionScope.Kind != HandleKind.AssemblyReference) - { - foreach (TypeDefinitionHandle tdh in this.Reader.TypeDefinitions) - { - TypeDefinition typeDef = this.Reader.GetTypeDefinition(tdh); - if (typeDef.Name == typeRef.Name && typeDef.Namespace == typeRef.Namespace) - { - if (typeRef.ResolutionScope.Kind == HandleKind.TypeReference) - { - // The ref is nested. Verify that the type we found is nested in the same type as well. - if (this.TryGetTypeDefHandle((TypeReferenceHandle)typeRef.ResolutionScope, out TypeDefinitionHandle nestingTypeDef) && nestingTypeDef == typeDef.GetDeclaringType()) - { - typeDefHandle = tdh; - break; - } - } - else if (typeRef.ResolutionScope.Kind == HandleKind.ModuleDefinition && typeDef.GetDeclaringType().IsNil) - { - typeDefHandle = tdh; - break; - } - else - { - throw new NotSupportedException("Unrecognized ResolutionScope: " + typeRef.ResolutionScope); - } - } - } - } - - this.refToDefCache.Add(typeRefHandle, typeDefHandle); - return !typeDefHandle.IsNil; - } + internal bool TryGetTypeDefHandle(TypeReferenceHandle typeRefHandle, out TypeDefinitionHandle typeDefHandle) => this.MetadataIndex.TryGetTypeDefHandle(typeRefHandle, out typeDefHandle); internal bool TryGetTypeDefHandle(TypeReference typeRef, out TypeDefinitionHandle typeDefHandle) => this.TryGetTypeDefHandle(typeRef.Namespace, typeRef.Name, out typeDefHandle); diff --git a/src/Microsoft.Windows.CsWin32/MetadataIndex.cs b/src/Microsoft.Windows.CsWin32/MetadataIndex.cs index 6b718822..e21c1b98 100644 --- a/src/Microsoft.Windows.CsWin32/MetadataIndex.cs +++ b/src/Microsoft.Windows.CsWin32/MetadataIndex.cs @@ -37,6 +37,8 @@ internal class MetadataIndex : IDisposable private readonly HashSet releaseMethods = new HashSet(StringComparer.Ordinal); + private readonly Dictionary refToDefCache = new(); + /// /// The set of names of typedef structs that represent handles where the handle has length of /// and is therefore appropriate to wrap in a . @@ -274,6 +276,53 @@ internal static void Return(MetadataIndex index) } } + /// + /// Attempts to translate a to a . + /// + /// The reference handle. + /// Receives the type def handle, if one was discovered. + /// if a TypeDefinition was found; otherwise . + internal bool TryGetTypeDefHandle(TypeReferenceHandle typeRefHandle, out TypeDefinitionHandle typeDefHandle) + { + if (this.refToDefCache.TryGetValue(typeRefHandle, out typeDefHandle)) + { + return !typeDefHandle.IsNil; + } + + TypeReference typeRef = this.Reader.GetTypeReference(typeRefHandle); + if (typeRef.ResolutionScope.Kind != HandleKind.AssemblyReference) + { + foreach (TypeDefinitionHandle tdh in this.Reader.TypeDefinitions) + { + TypeDefinition typeDef = this.Reader.GetTypeDefinition(tdh); + if (typeDef.Name == typeRef.Name && typeDef.Namespace == typeRef.Namespace) + { + if (typeRef.ResolutionScope.Kind == HandleKind.TypeReference) + { + // The ref is nested. Verify that the type we found is nested in the same type as well. + if (this.TryGetTypeDefHandle((TypeReferenceHandle)typeRef.ResolutionScope, out TypeDefinitionHandle nestingTypeDef) && nestingTypeDef == typeDef.GetDeclaringType()) + { + typeDefHandle = tdh; + break; + } + } + else if (typeRef.ResolutionScope.Kind == HandleKind.ModuleDefinition && typeDef.GetDeclaringType().IsNil) + { + typeDefHandle = tdh; + break; + } + else + { + throw new NotSupportedException("Unrecognized ResolutionScope: " + typeRef.ResolutionScope); + } + } + } + } + + this.refToDefCache.Add(typeRefHandle, typeDefHandle); + return !typeDefHandle.IsNil; + } + private static string CommonPrefix(IReadOnlyList ss) { if (ss.Count == 0) From 1e8a81ef37a8f6946aed52c31652a5387552fb79 Mon Sep 17 00:00:00 2001 From: Andrew Arnott Date: Mon, 16 May 2022 10:46:01 -0600 Subject: [PATCH 2/3] Rename `FindSymbolIfAlreadyAvailable` to clarify it's only for types --- src/Microsoft.Windows.CsWin32/Generator.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/Microsoft.Windows.CsWin32/Generator.cs b/src/Microsoft.Windows.CsWin32/Generator.cs index 3f116b51..3b324341 100644 --- a/src/Microsoft.Windows.CsWin32/Generator.cs +++ b/src/Microsoft.Windows.CsWin32/Generator.cs @@ -1492,7 +1492,7 @@ internal void RequestConstant(FieldDefinitionHandle fieldDefHandle) return safeHandleType; } - if (this.FindSymbolIfAlreadyAvailable($"{this.Namespace}.{safeHandleType}") is object) + if (this.FindTypeSymbolIfAlreadyAvailable($"{this.Namespace}.{safeHandleType}") is object) { return safeHandleType; } @@ -1715,7 +1715,7 @@ internal void GetBaseTypeInfo(TypeDefinition typeDef, out StringHandle baseTypeN fullyQualifiedName = $"{ns}.{specialName}"; // Skip if the compilation already defines this type or can access it from elsewhere. - if (this.FindSymbolIfAlreadyAvailable(fullyQualifiedName) is object) + if (this.FindTypeSymbolIfAlreadyAvailable(fullyQualifiedName) is object) { // The type already exists either in this project or a referenced one. return null; @@ -2615,7 +2615,7 @@ private bool HasObsoleteAttribute(CustomAttributeHandleCollection attributes) return false; } - private ISymbol? FindSymbolIfAlreadyAvailable(string fullyQualifiedMetadataName) + private ISymbol? FindTypeSymbolIfAlreadyAvailable(string fullyQualifiedMetadataName) { if (this.compilation is object) { @@ -2659,7 +2659,7 @@ private bool HasObsoleteAttribute(CustomAttributeHandleCollection attributes) string name = this.Reader.GetString(typeDef.Name); string ns = this.Reader.GetString(typeDef.Namespace); string fullyQualifiedName = ns + "." + name; - if (this.FindSymbolIfAlreadyAvailable(fullyQualifiedName) is object) + if (this.FindTypeSymbolIfAlreadyAvailable(fullyQualifiedName) is object) { // The type already exists either in this project or a referenced one. return null; From ee42a7cbb0016d89b86183d99570c92e8dcab567 Mon Sep 17 00:00:00 2001 From: Andrew Arnott Date: Mon, 16 May 2022 10:50:37 -0600 Subject: [PATCH 3/3] Move constants to the typedef struct that declares their type For example, this moves the `HRESULT PInvoke.S_OK` static field to the `HRESULT` struct. Closes #340 --- src/Microsoft.Windows.CsWin32/Generator.cs | 79 +++++++++++++++---- .../GeneratorTests.cs | 48 +++++++++++ 2 files changed, 111 insertions(+), 16 deletions(-) diff --git a/src/Microsoft.Windows.CsWin32/Generator.cs b/src/Microsoft.Windows.CsWin32/Generator.cs index 3b324341..d869b1df 100644 --- a/src/Microsoft.Windows.CsWin32/Generator.cs +++ b/src/Microsoft.Windows.CsWin32/Generator.cs @@ -149,6 +149,11 @@ public class Generator : IDisposable "PCSTR", }; + private static readonly HashSet TypeDefsThatDoNotNestTheirConstants = new HashSet(SpecialTypeDefNames, StringComparer.Ordinal) + { + "PWSTR", + }; + /// /// This is the preferred capitalizations for modules and class names. /// If they are not in this list, the capitalization will come from the metadata assembly. @@ -425,7 +430,7 @@ select ClassDeclaration(Identifier(this.options.ClassName)) result = result.Concat(new MemberDeclarationSyntax[] { comInterfaceFriendlyExtensionsClass }); } - if (this.committedCode.Fields.Any()) + if (this.committedCode.TopLevelFields.Any()) { result = result.Concat(new MemberDeclarationSyntax[] { this.DeclareConstantDefiningClass() }); } @@ -1443,9 +1448,22 @@ internal void RequestConstant(FieldDefinitionHandle fieldDefHandle) { this.volatileCode.GenerateConstant(fieldDefHandle, delegate { - FieldDeclarationSyntax constantDeclaration = this.DeclareConstant(fieldDefHandle); - constantDeclaration = this.AddApiDocumentation(constantDeclaration.Declaration.Variables[0].Identifier.ValueText, constantDeclaration); - this.volatileCode.AddConstant(fieldDefHandle, constantDeclaration); + FieldDefinition fieldDef = this.Reader.GetFieldDefinition(fieldDefHandle); + FieldDeclarationSyntax constantDeclaration = this.DeclareConstant(fieldDef); + + TypeHandleInfo fieldTypeInfo = fieldDef.DecodeSignature(SignatureHandleProvider.Instance, null) with { IsConstantField = true }; + TypeDefinitionHandle? fieldType = null; + if (fieldTypeInfo is HandleTypeHandleInfo handleInfo && this.IsTypeDefStruct(handleInfo) && handleInfo.Handle.Kind == HandleKind.TypeReference) + { + TypeReference tr = this.Reader.GetTypeReference((TypeReferenceHandle)handleInfo.Handle); + string fieldTypeName = this.Reader.GetString(tr.Name); + if (!TypeDefsThatDoNotNestTheirConstants.Contains(fieldTypeName) && this.TryGetTypeDefHandle(tr, out TypeDefinitionHandle candidate)) + { + fieldType = candidate; + } + } + + this.volatileCode.AddConstant(fieldDefHandle, constantDeclaration, fieldType); }); } @@ -1605,12 +1623,12 @@ internal void RequestConstant(FieldDefinitionHandle fieldDefHandle) { case "NTSTATUS": this.TryGenerateConstantOrThrow("STATUS_SUCCESS"); - ExpressionSyntax statusSuccess = MemberAccessExpression(SyntaxKind.SimpleMemberAccessExpression, this.methodsAndConstantsClassName, IdentifierName("STATUS_SUCCESS")); + ExpressionSyntax statusSuccess = MemberAccessExpression(SyntaxKind.SimpleMemberAccessExpression, ParseName("winmdroot.Foundation.NTSTATUS"), IdentifierName("STATUS_SUCCESS")); releaseInvocation = BinaryExpression(SyntaxKind.EqualsExpression, releaseInvocation, statusSuccess); break; case "HRESULT": this.TryGenerateConstantOrThrow("S_OK"); - ExpressionSyntax ok = MemberAccessExpression(SyntaxKind.SimpleMemberAccessExpression, this.methodsAndConstantsClassName, IdentifierName("S_OK")); + ExpressionSyntax ok = MemberAccessExpression(SyntaxKind.SimpleMemberAccessExpression, ParseName("winmdroot.Foundation.HRESULT"), IdentifierName("S_OK")); releaseInvocation = BinaryExpression(SyntaxKind.EqualsExpression, releaseInvocation, ok); break; default: @@ -2681,7 +2699,7 @@ private bool HasObsoleteAttribute(CustomAttributeHandleCollection attributes) // Is this a special typedef struct? if (this.IsTypeDefStruct(typeDef)) { - typeDeclaration = this.DeclareTypeDefStruct(typeDef); + typeDeclaration = this.DeclareTypeDefStruct(typeDef, typeDefHandle); } else if (this.IsEmptyStructWithGuid(typeDef)) { @@ -2923,9 +2941,8 @@ private void TryGenerateConstantOrThrow(string possiblyQualifiedName) } } - private FieldDeclarationSyntax DeclareConstant(FieldDefinitionHandle fieldDefHandle) + private FieldDeclarationSyntax DeclareConstant(FieldDefinition fieldDef) { - FieldDefinition fieldDef = this.Reader.GetFieldDefinition(fieldDefHandle); string name = this.Reader.GetString(fieldDef.Name); try { @@ -2980,6 +2997,8 @@ private FieldDeclarationSyntax DeclareConstant(FieldDefinitionHandle fieldDefHan VariableDeclarator(Identifier(name)).WithInitializer(EqualsValueClause(value)))) .WithModifiers(modifiers); result = fieldType.AddMarshalAs(result); + result = this.AddApiDocumentation(result.Declaration.Variables[0].Identifier.ValueText, result); + return result; } catch (Exception ex) @@ -2994,7 +3013,7 @@ private FieldDeclarationSyntax DeclareConstant(FieldDefinitionHandle fieldDefHan private ClassDeclarationSyntax DeclareConstantDefiningClass() { return ClassDeclaration(this.methodsAndConstantsClassName.Identifier) - .AddMembers(this.committedCode.Fields.ToArray()) + .AddMembers(this.committedCode.TopLevelFields.ToArray()) .WithModifiers(TokenList(TokenWithSpace(this.Visibility), TokenWithSpace(SyntaxKind.StaticKeyword), TokenWithSpace(SyntaxKind.PartialKeyword))); } @@ -3564,7 +3583,7 @@ private ClassDeclarationSyntax DeclareCocreatableClass(TypeDefinition typeDef) /// /// Creates a struct that emulates a typedef in the C language headers. /// - private StructDeclarationSyntax DeclareTypeDefStruct(TypeDefinition typeDef) + private StructDeclarationSyntax DeclareTypeDefStruct(TypeDefinition typeDef, TypeDefinitionHandle typeDefHandle) { IdentifierNameSyntax name = IdentifierName(this.Reader.GetString(typeDef.Name)); if (name.Identifier.ValueText == "BOOL") @@ -5539,7 +5558,7 @@ private class GeneratedCode /// private readonly Dictionary types = new(); - private readonly Dictionary fieldsToSyntax = new(); + private readonly Dictionary fieldsToSyntax = new(); private readonly List safeHandleTypes = new(); @@ -5577,13 +5596,15 @@ internal GeneratedCode(GeneratedCode parent) this.parent = parent; } - internal IEnumerable GeneratedTypes => this.types.Values.Concat(this.specialTypes.Values).Concat(this.safeHandleTypes); + internal IEnumerable GeneratedTypes => this.GetTypesWithInjectedFields().Concat(this.specialTypes.Values).Concat(this.safeHandleTypes); internal IEnumerable ComInterfaceExtensions => this.comInterfaceFriendlyExtensionsMembers; internal IEnumerable InlineArrayIndexerExtensions => this.inlineArrayIndexerExtensionsMembers; - internal IEnumerable Fields => this.fieldsToSyntax.Values; + internal IEnumerable TopLevelFields => from field in this.fieldsToSyntax.Values + where field.FieldType is null || !this.types.ContainsKey(field.FieldType.Value) + select field.FieldDeclaration; internal IEnumerable> MembersByModule { @@ -5627,10 +5648,10 @@ internal void AddMemberToModule(string moduleName, IEnumerable GetTypesWithInjectedFields() + { + var fieldsByType = + (from field in this.fieldsToSyntax + where field.Value.FieldType is not null + group field.Value.FieldDeclaration by field.Value.FieldType into typeGroup + select typeGroup).ToDictionary(k => k.Key!, k => k.ToArray()); + foreach (KeyValuePair pair in this.types) + { + MemberDeclarationSyntax type = pair.Value; + if (fieldsByType.TryGetValue(pair.Key, out var extraFields)) + { + switch (type) + { + case StructDeclarationSyntax structType: + type = structType.AddMembers(extraFields); + break; + default: + throw new NotSupportedException(); + } + } + + yield return type; + } + } + private void ThrowIfNotGenerating() { if (!this.generating) diff --git a/test/Microsoft.Windows.CsWin32.Tests/GeneratorTests.cs b/test/Microsoft.Windows.CsWin32.Tests/GeneratorTests.cs index f78fae36..8af0df7e 100644 --- a/test/Microsoft.Windows.CsWin32.Tests/GeneratorTests.cs +++ b/test/Microsoft.Windows.CsWin32.Tests/GeneratorTests.cs @@ -310,6 +310,54 @@ public void WildcardForConstants_NoMatch() Assert.Empty(preciseApi); } + [Fact] + public void TypeDefConstantsDeclaredWithinTypeDef() + { + const string constant = "S_OK"; + this.generator = this.CreateGenerator(); + Assert.True(this.generator.TryGenerate(constant, CancellationToken.None)); + this.CollectGeneratedCode(this.generator); + this.AssertNoDiagnostics(); + FieldDeclarationSyntax field = this.FindGeneratedConstant(constant).Single(); + StructDeclarationSyntax declaringStruct = Assert.IsType(field.Parent); + Assert.Equal("HRESULT", declaringStruct.Identifier.ToString()); + } + + [Fact] + public void TypeDefConstantsRedirectedToPInvokeWhenTypeDefAlreadyInRefAssembly() + { + CSharpCompilation referencedProject = this.compilation.WithAssemblyName("refdProj"); + + using var referencedGenerator = this.CreateGenerator(new GeneratorOptions { Public = true }, referencedProject); + Assert.True(referencedGenerator.TryGenerate("HRESULT", CancellationToken.None)); + referencedProject = this.AddGeneratedCode(referencedProject, referencedGenerator); + this.AssertNoDiagnostics(referencedProject); + + // Now produce more code in a referencing project that includes at least one of the same types as generated in the referenced project. + const string constant = "S_OK"; + this.compilation = this.compilation.AddReferences(referencedProject.ToMetadataReference()); + this.generator = this.CreateGenerator(); + Assert.True(this.generator.TryGenerate(constant, CancellationToken.None)); + this.CollectGeneratedCode(this.generator); + this.AssertNoDiagnostics(); + FieldDeclarationSyntax field = this.FindGeneratedConstant(constant).Single(); + ClassDeclarationSyntax declaringClass = Assert.IsType(field.Parent); + Assert.Equal("PInvoke", declaringClass.Identifier.ToString()); + } + + [Theory] + [InlineData("IDC_ARROW")] // PCWSTR / PWSTR + public void SpecialTypeDefsDoNotContainTheirOwnConstants(string constantName) + { + this.generator = this.CreateGenerator(); + Assert.True(this.generator.TryGenerate(constantName, CancellationToken.None)); + this.CollectGeneratedCode(this.generator); + this.AssertNoDiagnostics(); + FieldDeclarationSyntax field = this.FindGeneratedConstant(constantName).Single(); + ClassDeclarationSyntax declaringClass = Assert.IsType(field.Parent); + Assert.Equal("PInvoke", declaringClass.Identifier.ToString()); + } + [Fact] public void FriendlyOverloadOfCOMInterfaceRemovesParameter() {