Skip to content

Fix custom-attribute generic argument dataflow in the trimmer#128776

Open
Copilot wants to merge 9 commits into
mainfrom
copilot/fix-trimmer-parameterless-constructor
Open

Fix custom-attribute generic argument dataflow in the trimmer#128776
Copilot wants to merge 9 commits into
mainfrom
copilot/fix-trimmer-parameterless-constructor

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented May 29, 2026

A generic attribute carrying a new() constraint failed to keep the parameterless constructor of its generic argument, and the trimmer could NRE while emitting IL2091 for generic-argument dataflow on custom attributes.

Description

  • MarkStep: Guard ProcessGenericArgumentDataFlow with RequiresGenericArgumentDataFlow so generic-argument analysis only runs when the argument actually carries requirements.
  • DiagnosticUtilities / MemberReferenceExtensions: Add null-safety (GetGenericParameterDeclaringMemberDisplayName, GetNamespaceDisplayName) so emitting IL2091 for an orphan generic parameter — a custom-attribute generic argument with no resolvable declaring type/method — no longer throws.
  • Tests: Consolidate NewConstrainedGenericAttributeArgumentConstructorIsKept into GenericAttributes, and drop a bogus [KeptMember(".ctor()")] on WithNewConstrainedGenericAttribute (its ctor is correctly trimmed since the attribute is read via GetCustomAttributes, never constructed).
  • Add an IL dependency (GenericAttributesDataFlow.il) for the generic-attribute DAM dataflow scenario, which cannot be expressed in C# (CS8968: a type parameter cannot be used as a generic attribute argument). The test asserts the unannotated argument warns IL2091 without crashing, matched via LogContains because the warning originates in the dependency assembly rather than the test assembly.

Known limitation

The reviewer-requested "no warning when the argument is annotated" half is not currently achievable. A custom-attribute generic argument is encoded as an orphan positional generic parameter (!0, with DeclaringType/DeclaringMethod/Owner all null), so FlowAnnotations.GetGenericParameterAnnotation returns None and annotated/unannotated arguments warn identically. Suppressing the annotated case would require resolving positional generic parameters against the attribute's target type — out of scope for this fix and flagged for reviewer decision.

Co-authored-by: jtschuster <36744439+jtschuster@users.noreply.github.com>
Copilot AI requested review from Copilot and removed request for Copilot May 29, 2026 16:42
@dotnet-policy-service dotnet-policy-service Bot added the linkable-framework Issues associated with delivering a linker friendly framework label May 29, 2026
@github-actions github-actions Bot added the area-Tools-ILLink .NET linker development as well as trimming analyzers label May 29, 2026
…new()-constrained ctor

Co-authored-by: jtschuster <36744439+jtschuster@users.noreply.github.com>
Copilot AI requested review from Copilot and removed request for Copilot May 29, 2026 16:57
@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.

Copilot AI changed the title [WIP] Fix trimmer removal of parameterless constructor for attribute Fix trimmer removing new()-constrained generic argument's parameterless ctor for custom attributes May 29, 2026
Copilot AI requested a review from jtschuster May 29, 2026 17:02
Comment thread src/tools/illink/src/linker/Linker.Steps/MarkStep.cs Outdated
Copilot AI review requested due to automatic review settings May 29, 2026 17:15
Comment thread src/tools/illink/src/linker/Linker.Steps/MarkStep.cs
Comment thread src/tools/illink/src/linker/Linker.Steps/MarkStep.cs Outdated
@jtschuster jtschuster marked this pull request as ready for review May 29, 2026 17:17
@jtschuster jtschuster requested a review from sbomer as a code owner May 29, 2026 17:17
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

This PR updates ILLink’s marking behavior for custom attributes so generic instantiations on attribute types participate in generic argument dataflow, ensuring requirements like new() constraints (and related annotations) are honored when attributes are materialized via reflection.

Changes:

  • Run GenericArgumentDataFlow.ProcessGenericArgumentDataFlow for the attribute constructor’s declaring type during MarkStep.MarkCustomAttribute.
  • Add a regression test that reflects over a type with a new()-constrained generic attribute and asserts the generic argument’s public parameterless constructor is preserved.

Reviewed changes

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

File Description
src/tools/illink/src/linker/Linker.Steps/MarkStep.cs Invokes generic-argument dataflow for attribute type instantiations when marking custom attributes.
src/tools/illink/test/Mono.Linker.Tests.Cases/Attributes/NewConstrainedGenericAttributeArgumentConstructorIsKept.cs Adds a linker test covering new()-constrained generic attribute arguments accessed via reflection.

Comment thread src/tools/illink/src/linker/Linker.Steps/MarkStep.cs Outdated

[Kept]
[KeptBaseType(typeof(Attribute))]
class MyAttribute<[KeptGenericParamAttributes(GenericParameterAttributes.DefaultConstructorConstraint)] T> : Attribute where T : new()
Copy link
Copy Markdown
Member

@sbomer sbomer May 29, 2026

Choose a reason for hiding this comment

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

@copilot Can we move this to the existing GenericAttributes tests? And could we add a test for a generic attribute where the generic parameter has DynamicallyAccessedMembers?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@copilot please move this test case to GenericAttributes tests and add a similar test case where the generic parameter has DynamicallyAccessedMembers. It should warn when the T passed in is unannotated, and not warn when it is annotated. It should look similar to below.

typeof(GenericClass<int, string>.InnerClass).GetCustomAttributes();
typeof(GenericClass<int, string>.InnerClass2).GetCustomAttributes();

class MyAnnotatedAttribute<[DynamicallyAccessedMembers(DAMT.Methods)] T>
{
}

class GenericClass<TUnnotated, [DynamicallyAccessedMembers(DAMT.Methods)]TAnnotated>
{
  [MyAnnotated<TUnannotated>] // Warns
  public class InnerClass { }
  [MyAnnotated<TAnnotated>] // no warning
  public class InnerClass2 { }
}

Comment thread src/tools/illink/src/linker/Linker.Steps/MarkStep.cs Outdated
Comment thread src/tools/illink/src/linker/Linker.Steps/MarkStep.cs Outdated
Copilot AI review requested due to automatic review settings May 29, 2026 18:08
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

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

Comment on lines 1190 to +1192
TypeReference constructor_type = ca.Constructor.DeclaringType;
GenericArgumentDataFlow.ProcessGenericArgumentDataFlow(in origin, this, Context, constructor_type);

Co-authored-by: jtschuster <36744439+jtschuster@users.noreply.github.com>
Copilot AI changed the title Fix trimmer removing new()-constrained generic argument's parameterless ctor for custom attributes Fix custom-attribute generic argument dataflow in the trimmer May 29, 2026
Copilot AI requested a review from jtschuster May 29, 2026 19:09
…ric-attribute case

Co-authored-by: jtschuster <36744439+jtschuster@users.noreply.github.com>
Copilot AI requested review from Copilot and removed request for Copilot June 1, 2026 20:31
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

4 participants