Skip to content

illink: Fix TypeMap instantiation not propagating to TypeMapAssociation entries#127452

Draft
Copilot wants to merge 2 commits intomainfrom
copilot/fix-typemap-instantiation-issue
Draft

illink: Fix TypeMap instantiation not propagating to TypeMapAssociation entries#127452
Copilot wants to merge 2 commits intomainfrom
copilot/fix-typemap-instantiation-issue

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Apr 27, 2026

TypeMapHandler.MarkTypeMapAttribute called Annotations.MarkInstantiated(targetTypeDef) directly, bypassing MarkRequirementsForInstantiatedTypes. This meant ProcessInstantiated was never called for the target type from that path — so any TypeMapAssociation entries keyed on that target as source type would remain permanently unprocessed if the type was never independently instantiated by user code.

Description

MarkStep.cs

  • Changed MarkRequirementsForInstantiatedTypes from protected virtual to protected internal virtual to allow TypeMapHandler (same assembly) to call it directly.

TypeMapHandler.cs

  • Replaced _context.Annotations.MarkInstantiated(targetTypeDef) with _markStep.MarkRequirementsForInstantiatedTypes(targetTypeDef) in MarkTypeMapAttribute. This routes through the full instantiation pipeline, including ProcessInstantiated, which correctly enqueues or directly marks associated TypeMapAssociation entries.

TypeMap.cs (test)

  • Added a regression test: a TypeMap<UsedTypeMap> whose target differs from its trim target, paired with a TypeMapAssociation<UsedTypeMap> where the source is that target type. The trim target is newed in Main; the target is only instantiated via TypeMap processing. Without the fix, the TypeMapAssociation attribute is silently dropped.
-  _context.Annotations.MarkInstantiated(targetTypeDef);
+  _markStep.MarkRequirementsForInstantiatedTypes(targetTypeDef);
-  protected virtual void MarkRequirementsForInstantiatedTypes(TypeDefinition type)
+  protected internal virtual void MarkRequirementsForInstantiatedTypes(TypeDefinition type)

Note

This PR description was generated with AI assistance (GitHub Copilot).

Original prompt

Apply the TypeMap instantiation fix on top of main in dotnet/runtime, using the cleaner approach discussed in PR #127005 and comment #127005 (comment).

Required code changes

  1. In src/tools/illink/src/linker/Linker.Steps/MarkStep.cs, change MarkRequirementsForInstantiatedTypes(TypeDefinition type) from protected virtual to protected internal virtual so it remains available to subclasses while also being callable from TypeMapHandler in the same assembly.

  2. In src/tools/illink/src/linker/Linker/TypeMapHandler.cs, replace the direct _context.Annotations.MarkInstantiated(targetTypeDef); call in MarkTypeMapAttribute with _markStep.MarkRequirementsForInstantiatedTypes(targetTypeDef);.

Current intended diff shape against main:

diff --git a/src/tools/illink/src/linker/Linker.Steps/MarkStep.cs b/src/tools/illink/src/linker/Linker.Steps/MarkStep.cs
--- a/src/tools/illink/src/linker/Linker.Steps/MarkStep.cs
+++ b/src/tools/illink/src/linker/Linker.Steps/MarkStep.cs
@@
-        protected virtual void MarkRequirementsForInstantiatedTypes(TypeDefinition type)
+        protected internal virtual void MarkRequirementsForInstantiatedTypes(TypeDefinition type)
         {
diff --git a/src/tools/illink/src/linker/Linker/TypeMapHandler.cs b/src/tools/illink/src/linker/Linker/TypeMapHandler.cs
--- a/src/tools/illink/src/linker/Linker/TypeMapHandler.cs
+++ b/src/tools/illink/src/linker/Linker/TypeMapHandler.cs
@@
             // Mark the target type as instantiated
             if (entry.TargetType is { } targetType
                 && _context.Resolve(UnwrapToResolvableType(targetType)) is TypeDefinition targetTypeDef)
-                _context.Annotations.MarkInstantiated(targetTypeDef);
+                _markStep.MarkRequirementsForInstantiatedTypes(targetTypeDef);

Required tests

Bring over the test coverage from PR #127005 into main. In particular, update src/tools/illink/test/Mono.Linker.Tests.Cases/Reflection/TypeMap.cs to cover a type used by both TypeMapAttribute and TypeMapAssociationAttribute, ensuring both attributes are kept when the external TypeMap entry preserves the same source type.

The PR’s test additions were approximately:

// Verify that a type can be kept if it's used for both TypeMap and TypeMapAssociation
[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))]

and in Main:

// Use BothInExternalAndProxy in a way that preserves any corresponding typemap entries.
Console.WriteLine(new BothInExternalAndProxy());

and near the end of the test file:

[Kept]
[KeptMember(".ctor()")]
class BothInExternalAndProxy;
[Kept]
class BothInExternalAndProxyTarget;

Adjust the exact placement to match current main and add any additional tests that seem necessary.

Validation

Run an appropriate build and test for the changed area, and validate expected linker test outputs. At minimum, run the relevant ILLink test(s) for Mono.Linker.Tests.Cases.Reflection.TypeMap if available in this repo. If full build/test is impractical, run the narrowest reliable commands and document what was run and the result.

Notes

Prefer the protected internal modifier for MarkRequirementsForInstantiatedTypes, as requested.

The following is the prior conversation context from the user's chat exploration (may be truncated):

User: #127005 (comment) Does this comment make sense to you? What would the diff look like if you were to implement it?

Assistant: Yes — the comment makes sense. The PR’s current fix manually compensates for calling _context.Annotations.MarkInstantiated(targetTypeDef) directly, but MarkStep.MarkRequirementsForInstantiatedTypes() is the central path that already does the “a type became instantiated” work, including _typeMapHandler.ProcessInstantiated(type). So exposing that method to TypeMapHandler and calling it should make the extra MarkAssociatedProxyTypeMapAttributes logic unnecessary.

A likely implementation diff would be:

diff --git a/src/tools/illink/src/linker/Linker.Steps/MarkStep.cs b/src/tools/illink/src/linker/Linker.Steps/MarkStep.cs
index a9a5b3d961bd..00000000000 100644
--- a/src/tools/illink/src/linker/Lin...

</details>



<!-- START COPILOT CODING AGENT SUFFIX -->

*This pull request was created from Copilot chat.*
>

Copilot AI self-assigned this Apr 27, 2026
Copilot AI review requested due to automatic review settings April 27, 2026 13:03
Copilot AI review requested due to automatic review settings April 27, 2026 13:03
@github-actions github-actions Bot added the area-Tools-ILLink .NET linker development as well as trimming analyzers label Apr 27, 2026
…uirementsForInstantiatedTypes + test

Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/b98f3157-9724-49b7-92fe-29cfad891f1e

Co-authored-by: jtschuster <36744439+jtschuster@users.noreply.github.com>
Copilot AI requested review from Copilot and removed request for Copilot April 27, 2026 13:56
@dotnet-policy-service dotnet-policy-service Bot added the linkable-framework Issues associated with delivering a linker friendly framework label Apr 27, 2026
Copilot AI changed the title [WIP] Fix TypeMap instantiation access modifier and update usage illink: Fix TypeMap instantiation not propagating to TypeMapAssociation entries Apr 27, 2026
Copilot AI requested a review from jtschuster April 27, 2026 14:00
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

Status: No status

Development

Successfully merging this pull request may close these issues.

2 participants