Skip to content

Commit

Permalink
Merge pull request #922 from microsoft/fix918
Browse files Browse the repository at this point in the history
Fix generated code compilation break due to redundant extension methods
  • Loading branch information
AArnott authored Apr 28, 2023
2 parents fb94769 + e9c04f9 commit 477c087
Show file tree
Hide file tree
Showing 4 changed files with 44 additions and 21 deletions.
2 changes: 1 addition & 1 deletion src/Microsoft.Windows.CsWin32/Generator.Handle.cs
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ public partial class Generator
return safeHandleType;
}

if (this.FindTypeSymbolIfAlreadyAvailable($"{this.Namespace}.{safeHandleType}") is object)
if (this.FindTypeSymbolsIfAlreadyAvailable($"{this.Namespace}.{safeHandleType}").Count > 0)
{
return safeHandleType;
}
Expand Down
6 changes: 5 additions & 1 deletion src/Microsoft.Windows.CsWin32/Generator.InlineArrays.cs
Original file line number Diff line number Diff line change
Expand Up @@ -777,8 +777,12 @@ StatementSyntax ClearSlice(ExpressionSyntax span) =>

private ClassDeclarationSyntax DeclareInlineArrayIndexerExtensionsClass()
{
var filteredExtensionMethods =
this.committedCode.InlineArrayIndexerExtensions.Where(e =>
this.FindExtensionMethodIfAlreadyAvailable($"{this.Namespace}.{InlineArrayIndexerExtensionsClassName}", e.Identifier.ValueText) is null).ToArray();

return ClassDeclaration(InlineArrayIndexerExtensionsClassName.Identifier)
.AddMembers(this.committedCode.InlineArrayIndexerExtensions.ToArray())
.AddMembers(filteredExtensionMethods)
.WithModifiers(TokenList(TokenWithSpace(this.Visibility), TokenWithSpace(SyntaxKind.StaticKeyword), TokenWithSpace(SyntaxKind.PartialKeyword)))
.AddAttributeLists(AttributeList().AddAttributes(GeneratedCodeAttribute));
}
Expand Down
53 changes: 34 additions & 19 deletions src/Microsoft.Windows.CsWin32/Generator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ public partial class Generator : IDisposable
private readonly TypeSyntaxSettings functionPointerTypeSettings;
private readonly TypeSyntaxSettings errorMessageTypeSettings;

private readonly Dictionary<string, ISymbol?> findTypeSymbolIfAlreadyAvailableCache = new(StringComparer.Ordinal);
private readonly Dictionary<string, IReadOnlyList<ISymbol>> findTypeSymbolIfAlreadyAvailableCache = new(StringComparer.Ordinal);
private readonly Rental<MetadataReader> metadataReader;
private readonly GeneratorOptions options;
private readonly CSharpCompilation? compilation;
Expand Down Expand Up @@ -953,7 +953,9 @@ 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.FindTypeSymbolIfAlreadyAvailable(fullyQualifiedName) is object)
// But if we have more than one match, the compiler won't be able to resolve our type references.
// In such a case, we'll prefer to just declare our own local symbol.
if (this.FindTypeSymbolsIfAlreadyAvailable(fullyQualifiedName).Count == 1)
{
// The type already exists either in this project or a referenced one.
return null;
Expand Down Expand Up @@ -1064,24 +1066,27 @@ private bool TryGetRenamedMethod(string methodName, [NotNullWhen(true)] out stri
return false;
}

private ISymbol? FindTypeSymbolIfAlreadyAvailable(string fullyQualifiedMetadataName)
private ISymbol? FindTypeSymbolIfAlreadyAvailable(string fullyQualifiedMetadataName) => this.FindTypeSymbolsIfAlreadyAvailable(fullyQualifiedMetadataName).FirstOrDefault();

private IReadOnlyList<ISymbol> FindTypeSymbolsIfAlreadyAvailable(string fullyQualifiedMetadataName)
{
if (this.findTypeSymbolIfAlreadyAvailableCache.TryGetValue(fullyQualifiedMetadataName, out ISymbol? result))
if (this.findTypeSymbolIfAlreadyAvailableCache.TryGetValue(fullyQualifiedMetadataName, out IReadOnlyList<ISymbol>? result))
{
return result;
}

List<ISymbol>? results = null;
if (this.compilation is object)
{
if (this.compilation.Assembly.GetTypeByMetadataName(fullyQualifiedMetadataName) is { } ownSymbol)
{
// This assembly defines it.
// But if it defines it as a partial, we should not consider it as fully defined so we populate our side.
result = ownSymbol.DeclaringSyntaxReferences.Any(sr => sr.GetSyntax() is BaseTypeDeclarationSyntax type && type.Modifiers.Any(SyntaxKind.PartialKeyword))
? null
: ownSymbol;
this.findTypeSymbolIfAlreadyAvailableCache.Add(fullyQualifiedMetadataName, result);
return result;
if (!ownSymbol.DeclaringSyntaxReferences.Any(sr => sr.GetSyntax() is BaseTypeDeclarationSyntax type && type.Modifiers.Any(SyntaxKind.PartialKeyword)))
{
results ??= new();
results.Add(ownSymbol);
}
}

foreach (MetadataReference? reference in this.compilation.References)
Expand All @@ -1099,25 +1104,32 @@ private bool TryGetRenamedMethod(string methodName, [NotNullWhen(true)] out stri
if (this.compilation.IsSymbolAccessibleWithin(externalSymbol, this.compilation.Assembly))
{
// A referenced assembly declares this symbol and it is accessible to our own.
// If we already found a match, then we have multiple matches now and the compiler won't be able to resolve our type references.
// In such a case, we'll prefer to just declare our own local symbol.
if (result is not null)
{
this.findTypeSymbolIfAlreadyAvailableCache.Add(fullyQualifiedMetadataName, null);
return null;
}

result = externalSymbol;
results ??= new();
results.Add(externalSymbol);
}
}
}
}
}

result = (IReadOnlyList<ISymbol>?)results ?? Array.Empty<ISymbol>();
this.findTypeSymbolIfAlreadyAvailableCache.Add(fullyQualifiedMetadataName, result);
return result;
}

private ISymbol? FindExtensionMethodIfAlreadyAvailable(string fullyQualifiedTypeMetadataName, string methodName)
{
foreach (INamedTypeSymbol typeSymbol in this.FindTypeSymbolsIfAlreadyAvailable(fullyQualifiedTypeMetadataName).OfType<INamedTypeSymbol>())
{
if (typeSymbol.GetMembers(methodName) is { Length: > 0 } members)
{
return members[0];
}
}

return null;
}

private MemberDeclarationSyntax? RequestInteropTypeHelper(TypeDefinitionHandle typeDefHandle, Context context)
{
TypeDefinition typeDef = this.Reader.GetTypeDefinition(typeDefHandle);
Expand All @@ -1132,7 +1144,10 @@ private bool TryGetRenamedMethod(string methodName, [NotNullWhen(true)] out stri
bool isManagedType = this.IsManagedType(typeDefHandle);
string fullyQualifiedName = this.GetMangledIdentifier(ns + "." + name, context.AllowMarshaling, isManagedType);

if (this.FindTypeSymbolIfAlreadyAvailable(fullyQualifiedName) is object)
// Skip if the compilation already defines this type or can access it from elsewhere.
// But if we have more than one match, the compiler won't be able to resolve our type references.
// In such a case, we'll prefer to just declare our own local symbol.
if (this.FindTypeSymbolsIfAlreadyAvailable(fullyQualifiedName).Count == 1)
{
// The type already exists either in this project or a referenced one.
return null;
Expand Down
4 changes: 4 additions & 0 deletions test/Microsoft.Windows.CsWin32.Tests/GeneratorTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -828,6 +828,8 @@ public void TBButton([CombinatorialMemberData(nameof(SpecificCpuArchitectures))]
[Theory, PairwiseData]
public void ProjectReferenceBetweenTwoGeneratingProjects(bool internalsVisibleTo)
{
this.compilation = this.compilation.WithOptions(this.compilation.Options.WithPlatform(Platform.X64));

CSharpCompilation referencedProject = this.compilation
.WithAssemblyName("refdProj");
if (internalsVisibleTo)
Expand All @@ -840,6 +842,7 @@ public void ProjectReferenceBetweenTwoGeneratingProjects(bool internalsVisibleTo
Assert.True(referencedGenerator.TryGenerate("LockWorkStation", CancellationToken.None));
Assert.True(referencedGenerator.TryGenerate("CreateFile", CancellationToken.None));
Assert.True(referencedGenerator.TryGenerate("RAWHID", CancellationToken.None));
Assert.True(referencedGenerator.TryGenerate("SHFILEINFOW", CancellationToken.None)); // generates inline arrays + extension methods
referencedProject = this.AddGeneratedCode(referencedProject, referencedGenerator);
this.AssertNoDiagnostics(referencedProject);

Expand All @@ -848,6 +851,7 @@ public void ProjectReferenceBetweenTwoGeneratingProjects(bool internalsVisibleTo
this.generator = this.CreateGenerator(new GeneratorOptions { ClassName = "P2" });
Assert.True(this.generator.TryGenerate("HidD_GetAttributes", CancellationToken.None));
Assert.True(this.generator.TryGenerate("RAWHID", CancellationToken.None));
Assert.True(this.generator.TryGenerate("DROPDESCRIPTION", CancellationToken.None)); // reuses the same inline array and extension methods as SHFILEINFOW.
this.CollectGeneratedCode(this.generator);
this.AssertNoDiagnostics();
}
Expand Down

0 comments on commit 477c087

Please sign in to comment.