Skip to content

Fix illink TypeMap proxy retention. Fixes #127004.#127005

Open
rolfbjarne wants to merge 2 commits intodotnet:mainfrom
rolfbjarne:illink-typemapassociation-127004
Open

Fix illink TypeMap proxy retention. Fixes #127004.#127005
rolfbjarne wants to merge 2 commits intodotnet:mainfrom
rolfbjarne:illink-typemapassociation-127004

Conversation

@rolfbjarne
Copy link
Copy Markdown
Member

@rolfbjarne rolfbjarne commented Apr 16, 2026

Keep matching TypeMapAssociation entries when an external TypeMap entry preserves the same source type.

Note: copilot wrote the fix, I have no idea if it's correct or not.

Fixes #127004.

Keep matching TypeMapAssociation entries when an external TypeMap entry preserves the same source type.

Fixes dotnet#127004.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@rolfbjarne rolfbjarne requested a review from sbomer as a code owner April 16, 2026 14:31
Copilot AI review requested due to automatic review settings April 16, 2026 14:31
@github-actions github-actions Bot added the area-Tools-ILLink .NET linker development as well as trimming analyzers label Apr 16, 2026
@dotnet-policy-service dotnet-policy-service Bot added the linkable-framework Issues associated with delivering a linker friendly framework label Apr 16, 2026
@dotnet-policy-service
Copy link
Copy Markdown
Contributor

Tagging subscribers to this area: @agocke, @dotnet/illink
See info in area-owners.md if you want to be subscribed.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Fixes an ILLink trimming bug where TypeMapAssociationAttribute<T> entries can be dropped when the same type is also preserved/instantiated via a TypeMapAttribute<T> entry (issue #127004).

Changes:

  • Adds a new trimming test scenario intended to validate retention of both TypeMap and TypeMapAssociation entries for the same type.
  • Updates TypeMapHandler.MarkTypeMapAttribute to additionally mark associated proxy TypeMapAssociation entries when a TypeMapAttribute marks a type as instantiated.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
src/tools/illink/test/Mono.Linker.Tests.Cases/Reflection/TypeMap.cs Adds a new test scenario asserting both TypeMap and TypeMapAssociation attributes are kept for a shared type.
src/tools/illink/src/linker/Linker/TypeMapHandler.cs Adds logic to mark proxy association attributes when a TypeMapAttribute instantiates the same type, to avoid missing proxy processing due to early exit in MarkStep.

Comment on lines +118 to +130
void MarkAssociatedProxyTypeMapAttributes(TypeDefinition sourceType, TypeReference typeMapGroup)
{
if (_unmarkedProxyTypeMapEntries.TryGetValue(sourceType, out Dictionary<TypeReference, List<CustomAttributeWithOrigin>>? entriesByGroup) &&
entriesByGroup.Remove(typeMapGroup, out List<CustomAttributeWithOrigin>? unmarkedAttributes))
{
foreach (var attr in unmarkedAttributes)
MarkTypeMapAttribute(attr, new DependencyInfo(DependencyKind.TypeMapEntry, sourceType));

if (entriesByGroup.Count == 0)
_unmarkedProxyTypeMapEntries.Remove(sourceType);
}

if (_pendingProxyTypeMapEntries.TryGetValue(typeMapGroup, out List<CustomAttributeWithOrigin>? pendingAttributes))
Copy link

Copilot AI Apr 16, 2026

Choose a reason for hiding this comment

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

MarkAssociatedProxyTypeMapAttributes directly calls MarkTypeMapAttribute on TypeMapAssociation entries as soon as a TypeMapAttribute marks the same type instantiated. This bypasses the existing universe separation logic (proxy entries are supposed to be kept only when the proxy type-map group is referenced via GetOrCreateProxyTypeMapping), and can cause TypeMapAssociation attributes to be preserved even when the proxy map group is never seen. Consider instead triggering the same logic as MarkStep would (e.g., invoke ProcessInstantiated/RecordTargetTypeSeen for the instantiated type) so entries are moved to the pending/marked sets respecting _referencedProxyTypeMaps, rather than being unconditionally marked here.

Suggested change
void MarkAssociatedProxyTypeMapAttributes(TypeDefinition sourceType, TypeReference typeMapGroup)
{
if (_unmarkedProxyTypeMapEntries.TryGetValue(sourceType, out Dictionary<TypeReference, List<CustomAttributeWithOrigin>>? entriesByGroup) &&
entriesByGroup.Remove(typeMapGroup, out List<CustomAttributeWithOrigin>? unmarkedAttributes))
{
foreach (var attr in unmarkedAttributes)
MarkTypeMapAttribute(attr, new DependencyInfo(DependencyKind.TypeMapEntry, sourceType));
if (entriesByGroup.Count == 0)
_unmarkedProxyTypeMapEntries.Remove(sourceType);
}
if (_pendingProxyTypeMapEntries.TryGetValue(typeMapGroup, out List<CustomAttributeWithOrigin>? pendingAttributes))
void MarkOrQueueProxyTypeMapAttributes(List<CustomAttributeWithOrigin> attributes, TypeReference typeMapGroup, DependencyInfo info)
{
if (_referencedProxyTypeMaps.Contains(typeMapGroup))
{
foreach (CustomAttributeWithOrigin attr in attributes)
MarkTypeMapAttribute(attr, info);
return;
}
if (!_pendingProxyTypeMapEntries.TryGetValue(typeMapGroup, out List<CustomAttributeWithOrigin>? pendingAttributes))
{
pendingAttributes = new List<CustomAttributeWithOrigin>();
_pendingProxyTypeMapEntries.Add(typeMapGroup, pendingAttributes);
}
pendingAttributes.AddRange(attributes);
}
void MarkAssociatedProxyTypeMapAttributes(TypeDefinition sourceType, TypeReference typeMapGroup)
{
if (_unmarkedProxyTypeMapEntries.TryGetValue(sourceType, out Dictionary<TypeReference, List<CustomAttributeWithOrigin>>? entriesByGroup) &&
entriesByGroup.Remove(typeMapGroup, out List<CustomAttributeWithOrigin>? unmarkedAttributes))
{
MarkOrQueueProxyTypeMapAttributes(unmarkedAttributes, typeMapGroup, new DependencyInfo(DependencyKind.TypeMapEntry, sourceType));
if (entriesByGroup.Count == 0)
_unmarkedProxyTypeMapEntries.Remove(sourceType);
}
if (_referencedProxyTypeMaps.Contains(typeMapGroup) &&
_pendingProxyTypeMapEntries.TryGetValue(typeMapGroup, out List<CustomAttributeWithOrigin>? pendingAttributes))

Copilot uses AI. Check for mistakes.
Comment on lines +86 to +89
[assembly: TypeMap<UsedExternalTypeMap>("BothInExternalAndProxy", typeof(BothInExternalAndProxy), typeof(BothInExternalAndProxy))] // Kept
[assembly: TypeMapAssociation<UsedExternalTypeMap>(typeof(BothInExternalAndProxy), typeof(BothInExternalAndProxyTarget))] // Kept
[assembly: KeptAttributeAttribute(typeof(TypeMapAttribute<UsedExternalTypeMap>), "BothInExternalAndProxy", typeof(BothInExternalAndProxy), typeof(BothInExternalAndProxy))]
[assembly: KeptAttributeAttribute(typeof(TypeMapAssociationAttribute<UsedExternalTypeMap>), typeof(BothInExternalAndProxy), typeof(BothInExternalAndProxyTarget))]
Copy link

Copilot AI Apr 16, 2026

Choose a reason for hiding this comment

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

This new scenario asserts that TypeMapAssociation is kept, but the test body never calls TypeMapping.GetOrCreateProxyTypeMapping(). That conflicts with the earlier test intent/comments that proxy-universe attributes should be trimmed when only the external map is used for a universe. To make this a valid regression for #127004, consider using a universe that actually exercises both external and proxy type maps (e.g., UsedTypeMap), or explicitly reference the proxy type map group under test and adjust the other "proxy attrs removed" expectations accordingly.

Suggested change
[assembly: TypeMap<UsedExternalTypeMap>("BothInExternalAndProxy", typeof(BothInExternalAndProxy), typeof(BothInExternalAndProxy))] // Kept
[assembly: TypeMapAssociation<UsedExternalTypeMap>(typeof(BothInExternalAndProxy), typeof(BothInExternalAndProxyTarget))] // Kept
[assembly: KeptAttributeAttribute(typeof(TypeMapAttribute<UsedExternalTypeMap>), "BothInExternalAndProxy", typeof(BothInExternalAndProxy), typeof(BothInExternalAndProxy))]
[assembly: KeptAttributeAttribute(typeof(TypeMapAssociationAttribute<UsedExternalTypeMap>), typeof(BothInExternalAndProxy), typeof(BothInExternalAndProxyTarget))]
[assembly: TypeMap<UsedTypeMap>("BothInExternalAndProxy", typeof(BothInExternalAndProxy), typeof(BothInExternalAndProxy))] // Kept
[assembly: TypeMapAssociation<UsedTypeMap>(typeof(BothInExternalAndProxy), typeof(BothInExternalAndProxyTarget))] // Kept
[assembly: KeptAttributeAttribute(typeof(TypeMapAttribute<UsedTypeMap>), "BothInExternalAndProxy", typeof(BothInExternalAndProxy), typeof(BothInExternalAndProxy))]
[assembly: KeptAttributeAttribute(typeof(TypeMapAssociationAttribute<UsedTypeMap>), typeof(BothInExternalAndProxy), typeof(BothInExternalAndProxyTarget))]

Copilot uses AI. Check for mistakes.
_ = TypeMapping.GetOrCreateExternalTypeMapping<string>();
_ = TypeMapping.GetOrCreateProxyTypeMapping<string>();

// Use BothInExternalAndProxy in a way that preserves any corresponding typemap entries.
Copy link

Copilot AI Apr 16, 2026

Choose a reason for hiding this comment

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

Instantiating BothInExternalAndProxy here roots the type, but it still doesn't ensure the proxy type-map group for the added TypeMapAssociation attribute is referenced. If the intent is to cover the bug where the proxy association is missed because the type was already marked instantiated by an external TypeMap entry, the test should also ensure the relevant proxy type-map universe is "seen" (via GetOrCreateProxyTypeMapping for the same group) so the association is expected to be kept.

Suggested change
// Use BothInExternalAndProxy in a way that preserves any corresponding typemap entries.
// Use BothInExternalAndProxy in a way that preserves any corresponding typemap entries.
// Explicitly reference the proxy type-map universe for the same group as the external entry,
// so the test validates that the proxy association is kept even when the type was already
// marked instantiated by the external type map.
_ = TypeMapping.GetOrCreateProxyTypeMapping<UsedExternalTypeMap>();

Copilot uses AI. Check for mistakes.
@jtschuster
Copy link
Copy Markdown
Member

I think a cleaner fix (if it works) would be to just make MarkStep.MarkRequirementsForInstantiatedTypes() internal and call that instead of _annotations.MarkInstantiated()

Use MarkRequirementsForInstantiatedTypes instead of directly calling
Annotations.MarkInstantiated in TypeMapHandler.MarkTypeMapAttribute.
This ensures ProcessInstantiated is called for the target type, which
properly triggers proxy TypeMapAssociation entry processing through
the existing RecordTargetTypeSeen logic, removing the need for the
custom MarkAssociatedProxyTypeMapAttributes method.

Also fix the test to use UsedTypeMap (which has both external and proxy
references) instead of UsedExternalTypeMap (which only has external).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-Tools-ILLink .NET linker development as well as trimming analyzers linkable-framework Issues associated with delivering a linker friendly framework

Projects

None yet

Development

Successfully merging this pull request may close these issues.

illink: TypeMapAssociation attribute always trimmed if there's a TypeMap attribute for the same type.

3 participants