Skip to content

Properly populate ExportedType metadata table for forwarded extension blocks. #80361

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Sep 22, 2025
Merged

Properly populate ExportedType metadata table for forwarded extension blocks. #80361

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c2f730e
Properly populate ExportedType metadata table for forwarded extension…
AlekseyTs Sep 18, 2025
6967e34
Remove redundant nullable directive
AlekseyTs Sep 19, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
206 changes: 156 additions & 50 deletions src/Compilers/CSharp/Portable/Emitter/Model/PEModuleBuilder.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
using Microsoft.CodeAnalysis.Collections;
using Microsoft.CodeAnalysis.CSharp.Symbols;
using Microsoft.CodeAnalysis.CSharp.Symbols.Metadata.PE;
using Microsoft.CodeAnalysis.CSharp.Symbols.Retargeting;
using Microsoft.CodeAnalysis.Emit;
using Microsoft.CodeAnalysis.PooledObjects;
using Roslyn.Utilities;
Expand Down Expand Up @@ -613,25 +614,8 @@ private static void GetExportedTypes(NamespaceOrTypeSymbol symbol, int parentInd

if (haveExtensions)
{
var seenGroupingTypes = PooledHashSet<PENamedTypeSymbol>.GetInstance();
var groupingTypes = ArrayBuilder<PENamedTypeSymbol>.GetInstance();

foreach (var type in symbol.GetTypeMembers(""))
{
if (!type.IsExtension)
{
continue;
}

var groupingType = ((PENamedTypeSymbol)type).ExtensionGroupingType;
if (seenGroupingTypes.Add(groupingType))
{
groupingTypes.Add(groupingType);
}
}

seenGroupingTypes.Free();
groupingTypes.Sort((x, y) => x.MetadataToken.CompareTo(y.MetadataToken));
GetNestedExtensionGroupingTypes((PENamedTypeSymbol)symbol, groupingTypes);
Debug.Assert(!groupingTypes.IsEmpty);

foreach (var groupingType in groupingTypes)
Expand All @@ -643,17 +627,40 @@ private static void GetExportedTypes(NamespaceOrTypeSymbol symbol, int parentInd
}
}

public sealed override ImmutableArray<Cci.ExportedType> GetExportedTypes(DiagnosticBag diagnostics)
private static ArrayBuilder<PENamedTypeSymbol> GetNestedExtensionGroupingTypes(PENamedTypeSymbol symbol, ArrayBuilder<PENamedTypeSymbol> groupingTypes)
{
var seenGroupingTypes = PooledHashSet<PENamedTypeSymbol>.GetInstance();

foreach (var type in symbol.GetTypeMembers(""))
{
if (!type.IsExtension)
{
continue;
}

var groupingType = ((PENamedTypeSymbol)type).ExtensionGroupingType;
if (seenGroupingTypes.Add(groupingType))
{
groupingTypes.Add(groupingType);
}
}

seenGroupingTypes.Free();
groupingTypes.Sort((x, y) => x.MetadataToken.CompareTo(y.MetadataToken));
return groupingTypes;
}

public sealed override ImmutableArray<Cci.ExportedType> GetExportedTypes(EmitContext context)
Copy link
Member

@jjonescz jjonescz Sep 19, 2025 •
edited by AlekseyTs
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider passing EmitContext as in since it's a large structure. #Resolved

Copy link
Contributor Author

@AlekseyTs AlekseyTs Sep 19, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider passing EmitContext as in since it's a large structure.

I'll stick to the current way of passing. As far as I can see in existing code, it is passed everywhere by value. If that becomes a real problem we would need to change all of them. Also, VB doesn't have a concept of in, but this method is also overridden in VB.

{
Debug.Assert(HaveDeterminedTopLevelTypes);

if (_lazyExportedTypes.IsDefault)
{
var initialized = ImmutableInterlocked.InterlockedInitialize(ref _lazyExportedTypes, CalculateExportedTypes());
var initialized = ImmutableInterlocked.InterlockedInitialize(ref _lazyExportedTypes, CalculateExportedTypes(context));

if (initialized && _lazyExportedTypes.Length > 0)
{
ReportExportedTypeNameCollisions(_lazyExportedTypes, diagnostics);
ReportExportedTypeNameCollisions(_lazyExportedTypes, context.Diagnostics);
}
}

Expand All @@ -664,7 +671,7 @@ private static void GetExportedTypes(NamespaceOrTypeSymbol symbol, int parentInd
/// Builds an array of public type symbols defined in netmodules included in the compilation
/// and type forwarders defined in this compilation or any included netmodule (in this order).
/// </summary>
private ImmutableArray<Cci.ExportedType> CalculateExportedTypes()
private ImmutableArray<Cci.ExportedType> CalculateExportedTypes(EmitContext context)
{
SourceAssemblySymbol sourceAssembly = SourceModule.ContainingSourceAssembly;
var builder = ArrayBuilder<Cci.ExportedType>.GetInstance();
Expand All @@ -679,7 +686,7 @@ private static void GetExportedTypes(NamespaceOrTypeSymbol symbol, int parentInd
}

Debug.Assert(OutputKind.IsNetModule() == sourceAssembly.DeclaringCompilation.Options.OutputKind.IsNetModule());
GetForwardedTypes(sourceAssembly, builder);
GetForwardedTypes(sourceAssembly, builder, context);

return builder.ToImmutableAndFree();
}
Expand All @@ -688,14 +695,14 @@ private static void GetExportedTypes(NamespaceOrTypeSymbol symbol, int parentInd
/// <summary>
/// Returns a set of top-level forwarded types
/// </summary>
internal static HashSet<NamedTypeSymbol> GetForwardedTypes(SourceAssemblySymbol sourceAssembly, ArrayBuilder<Cci.ExportedType>? builder)
internal static HashSet<NamedTypeSymbol> GetForwardedTypes(SourceAssemblySymbol sourceAssembly, ArrayBuilder<Cci.ExportedType>? builder, EmitContext? context)
{
var seenTopLevelForwardedTypes = new HashSet<NamedTypeSymbol>();
GetForwardedTypes(seenTopLevelForwardedTypes, sourceAssembly.GetSourceDecodedWellKnownAttributeData(), builder);
GetForwardedTypes(seenTopLevelForwardedTypes, sourceAssembly.GetSourceDecodedWellKnownAttributeData(), builder, context);

if (!sourceAssembly.DeclaringCompilation.Options.OutputKind.IsNetModule())
{
GetForwardedTypes(seenTopLevelForwardedTypes, sourceAssembly.GetNetModuleDecodedWellKnownAttributeData(), builder);
GetForwardedTypes(seenTopLevelForwardedTypes, sourceAssembly.GetNetModuleDecodedWellKnownAttributeData(), builder, context);
}

return seenTopLevelForwardedTypes;
Expand All @@ -709,15 +716,20 @@ private void ReportExportedTypeNameCollisions(ImmutableArray<Cci.ExportedType> e

foreach (var exportedType in exportedTypes)
{
var type = (NamedTypeSymbol)exportedType.Type.GetInternalSymbol();
Debug.Assert(exportedType.Type.AsGenericTypeInstanceReference is null);
Debug.Assert(exportedType.Type.AsSpecializedNestedTypeReference is null);

Debug.Assert(type.IsDefinition);

if (!type.IsTopLevelType())
if (exportedType.Type.AsNestedTypeReference is not null)
{
continue;
}

// Other types are expected to be top-level types backed by regular C# type symbols.
var type = (NamedTypeSymbol)exportedType.Type.GetInternalSymbol();

Debug.Assert(type.IsDefinition);
Debug.Assert(type.IsTopLevelType());

// exported types are not emitted in EnC deltas (hence generation 0):
string fullEmittedName = MetadataHelpers.BuildQualifiedName(
((Cci.INamespaceTypeReference)type.GetCciAdapter()).NamespaceName,
Expand Down Expand Up @@ -772,8 +784,11 @@ private void ReportExportedTypeNameCollisions(ImmutableArray<Cci.ExportedType> e
private static void GetForwardedTypes(
HashSet<NamedTypeSymbol> seenTopLevelTypes,
CommonAssemblyWellKnownAttributeData<NamedTypeSymbol> wellKnownAttributeData,
ArrayBuilder<Cci.ExportedType>? builder)
ArrayBuilder<Cci.ExportedType>? builder,
EmitContext? contextOpt)
{
Debug.Assert(builder is null || contextOpt is not null);

if (wellKnownAttributeData?.ForwardedTypes?.Count > 0)
{
// (type, index of the parent exported type in builder, or -1 if the type is a top-level type)
Expand All @@ -798,45 +813,136 @@ private static void GetForwardedTypes(

if (builder is object)
{
Debug.Assert(contextOpt is not null);
var context = contextOpt.GetValueOrDefault();

// Return all nested types.
// Note the order: depth first, children in reverse order (to match dev10, not a requirement).
Debug.Assert(stack.Count == 0);
stack.Push((originalDefinition, -1));

while (stack.Count > 0)
{
var (type, parentIndex) = stack.Pop();
processTopItemFromStack(stack, context, builder);
}
}
}

stack.Free();
}

static void processTopItemFromStack(ArrayBuilder<(NamedTypeSymbol type, int parentIndex)> stack, EmitContext context, ArrayBuilder<Cci.ExportedType> builder)
{
var (type, parentIndex) = stack.Pop();

Debug.Assert(type is { ContainingModule: SourceModuleSymbol } or PENamedTypeSymbol or RetargetingNamedTypeSymbol);

// In general, we don't want private types to appear in the ExportedTypes table.
// BREAK: dev11 emits these types. The problem was discovered in dev10, but failed
// to meet the bar Bug: Dev10/258038 and was left as-is.
if (type.DeclaredAccessibility == Accessibility.Private)
{
// NOTE: this will also exclude nested types of type
return;
}

// NOTE: not bothering to put nested types in seenTypes - the top-level type is adequate protection.

int index = builder.Count;
builder.Add(new Cci.ExportedType(type.GetCciAdapter(), parentIndex, isForwarder: true));

ImmutableArray<NamedTypeSymbol> nested = type.GetTypeMembers(); // Ordered.

// In general, we don't want private types to appear in the ExportedTypes table.
// BREAK: dev11 emits these types. The problem was discovered in dev10, but failed
// to meet the bar Bug: Dev10/258038 and was left as-is.
if (type.DeclaredAccessibility == Accessibility.Private)
if (nested.Any(n => n.IsExtension))
{
switch (type)
{
case PENamedTypeSymbol peType:
{
// NOTE: this will also exclude nested types of type
continue;
}
var groupingTypes = ArrayBuilder<PENamedTypeSymbol>.GetInstance();
GetNestedExtensionGroupingTypes(peType, groupingTypes);
Debug.Assert(!groupingTypes.IsEmpty);

// NOTE: not bothering to put nested types in seenTypes - the top-level type is adequate protection.
// Iterate backwards so they get popped in forward order.
for (int i = groupingTypes.Count - 1; i >= 0; i--)
{
stack.Push((groupingTypes[i], index));
}

groupingTypes.Free();

int index = builder.Count;
builder.Add(new Cci.ExportedType(type.GetCciAdapter(), parentIndex, isForwarder: true));
pushNestedTypes(stack, index, nested);
}
break;

// Iterate backwards so they get popped in forward order.
ImmutableArray<NamedTypeSymbol> nested = type.GetTypeMembers(); // Ordered.
for (int i = nested.Length - 1; i >= 0; i--)
case SourceMemberContainerTypeSymbol sourceType:
{
if (nested[i].IsExtension)
pushAndProcessNestedTypes(stack, context, index, nested, builder);

foreach (var groupingType in sourceType.GetExtensionGroupingInfo().GetGroupingTypes())
{
continue; // https://github.com/dotnet/roslyn/issues/78963 - This is a temporary handling, we should get grouping and marker types processed instead.
int groupingIndex = builder.Count;
builder.Add(new Cci.ExportedType(groupingType, index, isForwarder: true));

foreach (var markerType in groupingType.GetNestedTypes(context))
{
builder.Add(new Cci.ExportedType(markerType, groupingIndex, isForwarder: true));
}
}
}
break;

case RetargetingNamedTypeSymbol retargetingType:
{
pushAndProcessNestedTypes(stack, context, index, nested, builder);

foreach ((Cci.INestedTypeReference GroupingType, ImmutableArray<Cci.INestedTypeReference> MarkerTypes) item in retargetingType.GetExtensionGroupingAndMarkerTypesForTypeForwarding(context))
{
int groupingIndex = builder.Count;
builder.Add(new Cci.ExportedType(item.GroupingType, index, isForwarder: true));

stack.Push((nested[i], index));
foreach (var markerType in item.MarkerTypes)
{
builder.Add(new Cci.ExportedType(markerType, groupingIndex, isForwarder: true));
}
}
}
}
break;

default:
throw ExceptionUtilities.UnexpectedValue(type);
}
}
else
{
pushNestedTypes(stack, index, nested);
}
}

stack.Free();
static void pushNestedTypes(ArrayBuilder<(NamedTypeSymbol type, int parentIndex)> stack, int index, ImmutableArray<NamedTypeSymbol> nested)
{
// Iterate backwards so they get popped in forward order.
for (int i = nested.Length - 1; i >= 0; i--)
{
if (nested[i].IsExtension)
{
continue; // Extension blocks are handled separately
}

stack.Push((nested[i], index));
}
}

static void pushAndProcessNestedTypes(ArrayBuilder<(NamedTypeSymbol type, int parentIndex)> stack, EmitContext context, int index, ImmutableArray<NamedTypeSymbol> nested, ArrayBuilder<Cci.ExportedType> builder)
{
int currentStackSize = stack.Count;

pushNestedTypes(stack, index, nested);

while (stack.Count > currentStackSize)
{
processTopItemFromStack(stack, context, builder);
Copy link
Member

@jjonescz jjonescz Sep 19, 2025 •
edited by AlekseyTs
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are we processing the newly pushed types here immediately instead of processing them in the top-level loop? #Resolved

Copy link
Contributor Author

@AlekseyTs AlekseyTs Sep 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are we processing the newly pushed types here immediately instead of processing them in the top-level loop?

Because we want to process them before we append grouping and marker types. And we want to append grouping and marker types before processing anything else from the stack. That ensures consistent ordering across all different forms of extensions representation.

}
}
}
#nullable disable
Expand Down
Loading