@copilot please address the feedback from @copilot and validate test changes

@copilot please address the feedback from @copilot and validate test changes

Done in af0ac93:

1. Added explicit RemovedTypeInAssembly("test", typeof(ArrayTypeTrimTargetUnusedClass)) and RemovedTypeInAssembly("test", typeof(ArrayTypeTrimTargetUnusedTarget)) assertions on the TypeMap class so the expected removal is explicitly verified by the test runner.
2. Fixed the misleading comment in AddProxyTypeMapEntry — it now says "two constructor type arguments (source and proxy)" instead of "single type argument".

Both Reflection.TypeMap and Reflection.TypeMapEntryAssembly tests still pass.

TypeMapHandler PR Review

✅ Core fix is correct and well-targeted

The while (trimTarget is TypeSpecification and not GenericInstanceType) loop in both AddExternalTypeMapEntry and AddProxyTypeMapEntry correctly mirrors the pattern already established in:

• MarkStep.GetOriginalType (MarkStep.cs:2880–2904) — same while + TypeSpecification + ElementType unwinding
• LinkContext.TryResolve (LinkContext.cs:955–966) — same fallthrough to GetElementType() for non-GenericInstanceType specs

The loop terminates safely because each iteration removes one wrapper layer from a finite chain. Excluding GenericInstanceType is correct since those can be resolved by LinkContext.Resolve.

⚠️ MarkTypeMapAttribute has the same latent vulnerability (pre-existing)

At TypeMapHandler.cs:110:

if (entry.TargetType is { } targetType && _context.Resolve(targetType) is TypeDefinition targetTypeDef)

TargetType (line 330) returns the raw ConstructorArguments[1] — the target/proxy type, not the trim target/source. If a user passes typeof(Foo[]) in that position, _context.Resolve() (the throwing variant, line 928-929) will throw NotSupportedException. This path is not covered by the new stripping logic.

This is pre-existing and low-probability (target/proxy types are meant to be instantiated, so array types are semantically unusual there), but it's the exact same class of bug this PR fixes. Consider either applying the same stripping to the TargetType property or switching to TryResolve at line 110.

✅ Test coverage is correct and complete

• Used case: ArrayTypeTrimTargetClass[] as trim target, class instantiated via new ArrayTypeTrimTargetClass() at line 214 → triggers ProcessType → entry kept. Matching KeptAttributeAttribute at line 46 ✓, [Kept] on both target class (line 565) and trim-target class (line 568) ✓.
• Unused case: ArrayTypeTrimTargetUnusedClass[] as trim target, class never instantiated → entry removed. No KeptAttributeAttribute ✓, RemovedTypeInAssembly for both types (lines 143-144) ✓.

✅ No infinite-loop risk

The while loop terminates because each ElementType access peels exactly one TypeSpecification layer. The chain of wrappers is finite (e.g., int[][] → int[] → int).

💡 Minor style: could use pattern-matching cast

// Current
while (trimTarget is TypeSpecification and not GenericInstanceType)
    trimTarget = ((TypeSpecification)trimTarget).ElementType;

// Alternative (slightly more idiomatic)
while (trimTarget is TypeSpecification { ElementType: var el } and not GenericInstanceType)
    trimTarget = el;

Not blocking — the current form is perfectly readable.

✅ NativeAOT changes are sound and separate

The ExternalTypeMapNode / ProxyTypeMapNode changes add a GenericTypesTemplateMap.IsEligibleToHaveATemplate(canon) guard before emitting conditional dependencies on template type layouts. This is a correctness tightening for the NativeAOT type-loader path and is independent of the ILLink stripping fix.

Summary

The core fix is correct and surgical. The one item to consider is the ⚠️ on MarkTypeMapAttribute's Resolve call, which has the same class of vulnerability but on a different argument position. The test additions properly validate both the kept and removed cases.

Generated by Code Review for issue #126380 · ◷

@copilot please apply the same fix in MarkTypeMapAttribute as the review identified and use the pattern matching style it recommends throughout your changes.

@copilot please apply the same fix in MarkTypeMapAttribute as the review identified and use the pattern matching style it recommends throughout your changes.

Done in 7ecb95a:

1. Applied the same TypeSpecification-stripping fix in MarkTypeMapAttribute — it now strips array/pointer/byref wrappers before calling _context.Resolve(targetType).
2. Updated all three stripping loops to use the property pattern matching style: while (targetType is TypeSpecification { ElementType: var elementType } and not GenericInstanceType), avoiding the explicit ((TypeSpecification)...) cast throughout.

/azp run runtime,runtime-dev-innerloop,dotnet-linker-tests