Skip to content

Fix ILLink duplicate warnings for generic parameters with new() constraint and PublicParameterlessConstructor annotation - #123457

Closed
sbomer with Copilot wants to merge 3 commits into
mainfrom
copilot/fix-il2026-warning-duplicate
Closed

Fix ILLink duplicate warnings for generic parameters with new() constraint and PublicParameterlessConstructor annotation#123457
sbomer with Copilot wants to merge 3 commits into
mainfrom
copilot/fix-il2026-warning-duplicate

Conversation

CopilotAI commented Jan 21, 2026

Copy link
Copy Markdown
Contributor
  • Inspect branch state, relevant files, and README guidance
  • Fetch full history and latest origin/main
  • Merge origin/main into the branch and resolve conflicts in ILLink files
  • Validate the resolved branch with targeted ILLink tests
  • Review the final diff and report the resolution
Original prompt

This section details on the original issue you should resolve

<issue_title>ILLink: extra warning for generic parameter with new constraint and annotation</issue_title>
<issue_description>Repro:

// IL2026 for type (ILLink produces this twice)classClassWithWarningOnGenericArgumentConstructor_NewAndAnnotation:RequiresNewAndConstructors<ClassWithRequires>{// IL2026 (ILLink produces this twice)publicClassWithWarningOnGenericArgumentConstructor_NewAndAnnotation(){}}classRequiresNewAndConstructors<[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicParameterlessConstructor)]T>whereT:new();[RequiresUnreferencedCode("Message for --ClassWithRequires--")]classClassWithRequires;

This produces a warning on the type and on the implicit call to the base constructor (ILLink and ILC both do this). But ILLink produces two of each. That's because:

  • ILC handles the new constraint by treating it like PublicParameterlessConstructor annotations
  • ILLink handles the new constraint separately, while marking generic argument types
    </issue_description>

Comments on the Issue (you are @copilot in this section)


💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

…erlessConstructor annotation
Co-authored-by: sbomer <787361+sbomer@users.noreply.github.com>
@dotnet-policy-servicedotnet-policy-serviceBot added the linkable-framework Issues associated with delivering a linker friendly framework label Jan 21, 2026
CopilotAI changed the title [WIP] Fix ILLink extra warning for generic parameterFix ILLink duplicate warnings for generic parameters with new() constraint and PublicParameterlessConstructor annotationJan 22, 2026
CopilotAI requested a review from sbomerJanuary 22, 2026 00:10
@sbomersbomer added area-Tools-ILLink .NET linker development as well as trimming analyzers and removed linkable-framework Issues associated with delivering a linker friendly framework labels Feb 2, 2026
@sbomer
sbomer requested a review from a teamFebruary 2, 2026 20:25
@sbomer
sbomer marked this pull request as ready for review February 2, 2026 20:25
CopilotAI review requested due to automatic review settings February 2, 2026 20:25
@dotnet-policy-servicedotnet-policy-serviceBot added the linkable-framework Issues associated with delivering a linker friendly framework label Feb 2, 2026

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

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 fixes duplicate IL2026 warnings in ILLink when generic parameters have both a new() constraint and DynamicallyAccessedMembers(PublicParameterlessConstructor) annotation. The issue occurred because both MarkStep.cs and GenericArgumentDataFlow.cs independently marked the default constructor, each producing warnings.

Changes:

  • Modified MarkStep.cs to skip marking default constructors for new() constraints when the parameter already has PublicParameterlessConstructor annotation
  • Updated test expectations in RequiresOnClass.cs to remove duplicate warning attributes

Reviewed changes

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

FileDescription
src/tools/illink/src/linker/Linker.Steps/MarkStep.csAdded check to avoid duplicate constructor marking when PublicParameterlessConstructor annotation exists
src/tools/illink/test/Mono.Linker.Tests.Cases/RequiresCapability/RequiresOnClass.csRemoved duplicate ExpectedWarning attributes that were tracking the bug

Comment threadsrc/tools/illink/src/linker/Linker.Steps/MarkStep.cs
@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.

@github-actions

Copy link
Copy Markdown
Contributor

🤖 Copilot Code Review — PR #123457

Note

This review was generated by Copilot.

Holistic Assessment

Motivation: Legitimate bug fix. Issue #119290 documents that ILLink produces duplicate IL2026 warnings when a generic parameter has both a new() constraint and a [DynamicallyAccessedMembers(PublicParameterlessConstructor)] annotation. Two independent code paths (MarkGenericArguments for the new() constraint and GenericArgumentDataFlow for the annotation) both mark and warn about the same constructor.

Approach: Correct and targeted. The fix prevents the new() constraint path from firing in MarkGenericArguments when the annotation path (GenericArgumentDataFlow) will already handle it. This aligns ILLink with ILC's (NativeAOT) approach of treating new() constraints as annotations.

Summary: ✅ LGTM. The fix is small, targeted, and correctly addresses the root cause. The HasFlag check properly handles composite flags. GenericArgumentDataFlow comprehensively covers all access patterns via ReflectionMethodBodyScanner (field accesses, method calls, type tokens), MarkTypeDefinition (base types), and MarkInterfaceImplementation (interfaces), so skipping MarkDefaultConstructor when the annotation subsumes it is safe. Already approved by @sbomer (area owner) and @jtschuster.


Detailed Findings

✅ Correctness — Fix properly deduplicates constructor marking

The HasFlag(DynamicallyAccessedMemberTypes.PublicParameterlessConstructor) check correctly handles all relevant flag combinations:

  • PublicParameterlessConstructor (0x0001) — exact match
  • PublicConstructors (0x0003 = 0x0002 | 0x0001) — superset
  • All — includes everything
  • None / unrelated flags — returns false, preserving the MarkDefaultConstructor call

When the annotation is present, GenericArgumentDataFlow.ProcessGenericInstantiation both marks the constructor (via RequireDynamicallyAccessedMembersActionReflectionMarker) and produces the IL2026 warning, so nothing is lost by skipping the redundant MarkDefaultConstructor call.

The earlier Copilot PR reviewer suggestion to add redundant PublicConstructors and All checks was correctly dismissed — HasFlag already handles composite flags by design.

✅ Data flow coverage — No gap in constructor marking

Verified that GenericArgumentDataFlow.ProcessGenericArgumentDataFlow fires comprehensively across all relevant code paths:

  • Base types: MarkStep.cs:2165 (in MarkTypeDefinition)
  • Interfaces: MarkStep.cs:4093 (in MarkInterfaceImplementation)
  • IL body scanning: ReflectionMethodBodyScanner processes field accesses (line 112), type accesses (144), method accesses (151), field tokens (158), and method calls (199)
  • Deferred analysis: TrimAnalysisGenericInstantiationAccessPattern for types, methods, and fields

This ensures that whenever MarkGenericArguments would fire for a generic instance with annotations, the data flow path also fires to handle marking and warnings.

✅ Test changes — Correctly updated expectations

The two removed [ExpectedWarning] lines in RequiresOnClass.cs had Tool.Trimmer scoping and referenced issue #119290, confirming they tracked the exact duplicate warning being fixed. The remaining single [ExpectedWarning] per location correctly expects one warning (from the data flow path). The RequiresNew<T> test case (which has new() only, no annotation) is unchanged, confirming the fix is scoped correctly.

💡 Null safety — Already handled

parameter?.HasDefaultConstructorConstraint == true guarantees parameter is non-null before the block where GetGenericParameterAnnotation(parameter) is called. No concern here.

Generated by Code Review for issue #123457 ·

@sbomer

Copy link
Copy Markdown
Member

@copilot resolve the merge conflicts in this pull request

CopilotAI commented Apr 14, 2026

Copy link
Copy Markdown
ContributorAuthor

@copilot resolve the merge conflicts in this pull request

The merge conflicts have been resolved. The issue this PR was addressing has already been fixed in the main branch by PR #119419 ("Remove base DAM mismatch warning on types"). That PR removed the entire new() constraint handling code from MarkGenericArguments in MarkStep.cs, which was the root cause of the duplicate warnings.

Since the duplicate warnings no longer occur in the main branch, this PR is no longer needed and can be closed. The branch should be reset to the latest main.

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

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 2945 to +2949
if (parameter?.HasDefaultConstructorConstraint == true)
MarkDefaultConstructor(argumentTypeDef, new DependencyInfo(DependencyKind.DefaultCtorForNewConstrainedGenericArgument, instance), origin);
{
var annotatedMemberTypes = Context.Annotations.FlowAnnotations.GetGenericParameterAnnotation(parameter);
if (!annotatedMemberTypes.HasFlag(DynamicallyAccessedMemberTypes.PublicParameterlessConstructor))
{

CopilotAIApr 14, 2026

Copy link

Choose a reason for hiding this comment

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

GetGenericParameterAnnotation(parameter) resolves the generic parameter’s declaring type/method and is now executed for every new()-constrained generic argument. If this ends up on a hot path during linking, you could short-circuit by first checking whether the generic instance’s definition has any generic-parameter annotations (e.g., FlowAnnotations.HasGenericParameterAnnotation on the GenericInstanceType/GenericInstanceMethod) and only calling GetGenericParameterAnnotation when annotations are present.

Copilot uses AI. Check for mistakes.
@sbomer

Copy link
Copy Markdown
Member

Confirmed that #119419 fixed the same issue.

@sbomersbomer closed this Apr 14, 2026
@github-actionsgithub-actionsBot locked and limited conversation to collaborators May 15, 2026
@jkotas
jkotas deleted the copilot/fix-il2026-warning-duplicate branch May 22, 2026 15:55
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-Tools-ILLink.NET linker development as well as trimming analyzerslinkable-frameworkIssues associated with delivering a linker friendly framework

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

ILLink: extra warning for generic parameter with new constraint and annotation

5 participants

@sbomer@jtschuster@agocke
, 'i'); if (__m === '*' || __re.test(location.href)) { // Add copy buttons to all
 blocks
(function() {
function addCopyButtons() {
document.querySelectorAll('pre code').forEach(function(codeBlock) {
if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;
codeBlock.parentElement.setAttribute('data-copy-added', 'true');
var btn = document.createElement('button');
btn.textContent = 'Copy';
btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';
btn.onmouseover = function() { this.style.opacity = '1'; };
btn.onmouseout = function() { this.style.opacity = '0.7'; };
btn.onclick = function() {
navigator.clipboard.writeText(codeBlock.textContent).then(function() {
btn.textContent = 'Copied!';
setTimeout(function() { btn.textContent = 'Copy'; }, 1500);
});
};
codeBlock.parentElement.style.position = 'relative';
codeBlock.parentElement.appendChild(btn);
});
}
addCopyButtons();
// Re-run on dynamic content
var observer = new MutationObserver(addCopyButtons);
observer.observe(document.body, { childList: true, subtree: true });
})();
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Fix ILLink duplicate warnings for generic parameters with new() constraint and PublicParameterlessConstructor annotation by Copilot · Pull Request #123457 · dotnet/runtime · GitHub
Skip to content

Fix ILLink duplicate warnings for generic parameters with new() constraint and PublicParameterlessConstructor annotation - #123457

Closed
sbomer with Copilot wants to merge 3 commits into
mainfrom
copilot/fix-il2026-warning-duplicate
Closed

Fix ILLink duplicate warnings for generic parameters with new() constraint and PublicParameterlessConstructor annotation#123457
sbomer with Copilot wants to merge 3 commits into
mainfrom
copilot/fix-il2026-warning-duplicate

Conversation

CopilotAI commented Jan 21, 2026

Copy link
Copy Markdown
Contributor
  • Inspect branch state, relevant files, and README guidance
  • Fetch full history and latest origin/main
  • Merge origin/main into the branch and resolve conflicts in ILLink files
  • Validate the resolved branch with targeted ILLink tests
  • Review the final diff and report the resolution
Original prompt

This section details on the original issue you should resolve

<issue_title>ILLink: extra warning for generic parameter with new constraint and annotation</issue_title>
<issue_description>Repro:

// IL2026 for type (ILLink produces this twice)classClassWithWarningOnGenericArgumentConstructor_NewAndAnnotation:RequiresNewAndConstructors<ClassWithRequires>{// IL2026 (ILLink produces this twice)publicClassWithWarningOnGenericArgumentConstructor_NewAndAnnotation(){}}classRequiresNewAndConstructors<[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicParameterlessConstructor)]T>whereT:new();[RequiresUnreferencedCode("Message for --ClassWithRequires--")]classClassWithRequires;

This produces a warning on the type and on the implicit call to the base constructor (ILLink and ILC both do this). But ILLink produces two of each. That's because:

  • ILC handles the new constraint by treating it like PublicParameterlessConstructor annotations
  • ILLink handles the new constraint separately, while marking generic argument types
    </issue_description>

Comments on the Issue (you are @copilot in this section)


💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

…erlessConstructor annotation
Co-authored-by: sbomer <787361+sbomer@users.noreply.github.com>
@dotnet-policy-servicedotnet-policy-serviceBot added the linkable-framework Issues associated with delivering a linker friendly framework label Jan 21, 2026
CopilotAI changed the title [WIP] Fix ILLink extra warning for generic parameterFix ILLink duplicate warnings for generic parameters with new() constraint and PublicParameterlessConstructor annotationJan 22, 2026
CopilotAI requested a review from sbomerJanuary 22, 2026 00:10
@sbomersbomer added area-Tools-ILLink .NET linker development as well as trimming analyzers and removed linkable-framework Issues associated with delivering a linker friendly framework labels Feb 2, 2026
@sbomer
sbomer requested a review from a teamFebruary 2, 2026 20:25
@sbomer
sbomer marked this pull request as ready for review February 2, 2026 20:25
CopilotAI review requested due to automatic review settings February 2, 2026 20:25
@dotnet-policy-servicedotnet-policy-serviceBot added the linkable-framework Issues associated with delivering a linker friendly framework label Feb 2, 2026

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

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 fixes duplicate IL2026 warnings in ILLink when generic parameters have both a new() constraint and DynamicallyAccessedMembers(PublicParameterlessConstructor) annotation. The issue occurred because both MarkStep.cs and GenericArgumentDataFlow.cs independently marked the default constructor, each producing warnings.

Changes:

  • Modified MarkStep.cs to skip marking default constructors for new() constraints when the parameter already has PublicParameterlessConstructor annotation
  • Updated test expectations in RequiresOnClass.cs to remove duplicate warning attributes

Reviewed changes

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

FileDescription
src/tools/illink/src/linker/Linker.Steps/MarkStep.csAdded check to avoid duplicate constructor marking when PublicParameterlessConstructor annotation exists
src/tools/illink/test/Mono.Linker.Tests.Cases/RequiresCapability/RequiresOnClass.csRemoved duplicate ExpectedWarning attributes that were tracking the bug

Comment threadsrc/tools/illink/src/linker/Linker.Steps/MarkStep.cs
@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.

@github-actions

Copy link
Copy Markdown
Contributor

🤖 Copilot Code Review — PR #123457

Note

This review was generated by Copilot.

Holistic Assessment

Motivation: Legitimate bug fix. Issue #119290 documents that ILLink produces duplicate IL2026 warnings when a generic parameter has both a new() constraint and a [DynamicallyAccessedMembers(PublicParameterlessConstructor)] annotation. Two independent code paths (MarkGenericArguments for the new() constraint and GenericArgumentDataFlow for the annotation) both mark and warn about the same constructor.

Approach: Correct and targeted. The fix prevents the new() constraint path from firing in MarkGenericArguments when the annotation path (GenericArgumentDataFlow) will already handle it. This aligns ILLink with ILC's (NativeAOT) approach of treating new() constraints as annotations.

Summary: ✅ LGTM. The fix is small, targeted, and correctly addresses the root cause. The HasFlag check properly handles composite flags. GenericArgumentDataFlow comprehensively covers all access patterns via ReflectionMethodBodyScanner (field accesses, method calls, type tokens), MarkTypeDefinition (base types), and MarkInterfaceImplementation (interfaces), so skipping MarkDefaultConstructor when the annotation subsumes it is safe. Already approved by @sbomer (area owner) and @jtschuster.


Detailed Findings

✅ Correctness — Fix properly deduplicates constructor marking

The HasFlag(DynamicallyAccessedMemberTypes.PublicParameterlessConstructor) check correctly handles all relevant flag combinations:

  • PublicParameterlessConstructor (0x0001) — exact match
  • PublicConstructors (0x0003 = 0x0002 | 0x0001) — superset
  • All — includes everything
  • None / unrelated flags — returns false, preserving the MarkDefaultConstructor call

When the annotation is present, GenericArgumentDataFlow.ProcessGenericInstantiation both marks the constructor (via RequireDynamicallyAccessedMembersActionReflectionMarker) and produces the IL2026 warning, so nothing is lost by skipping the redundant MarkDefaultConstructor call.

The earlier Copilot PR reviewer suggestion to add redundant PublicConstructors and All checks was correctly dismissed — HasFlag already handles composite flags by design.

✅ Data flow coverage — No gap in constructor marking

Verified that GenericArgumentDataFlow.ProcessGenericArgumentDataFlow fires comprehensively across all relevant code paths:

  • Base types: MarkStep.cs:2165 (in MarkTypeDefinition)
  • Interfaces: MarkStep.cs:4093 (in MarkInterfaceImplementation)
  • IL body scanning: ReflectionMethodBodyScanner processes field accesses (line 112), type accesses (144), method accesses (151), field tokens (158), and method calls (199)
  • Deferred analysis: TrimAnalysisGenericInstantiationAccessPattern for types, methods, and fields

This ensures that whenever MarkGenericArguments would fire for a generic instance with annotations, the data flow path also fires to handle marking and warnings.

✅ Test changes — Correctly updated expectations

The two removed [ExpectedWarning] lines in RequiresOnClass.cs had Tool.Trimmer scoping and referenced issue #119290, confirming they tracked the exact duplicate warning being fixed. The remaining single [ExpectedWarning] per location correctly expects one warning (from the data flow path). The RequiresNew<T> test case (which has new() only, no annotation) is unchanged, confirming the fix is scoped correctly.

💡 Null safety — Already handled

parameter?.HasDefaultConstructorConstraint == true guarantees parameter is non-null before the block where GetGenericParameterAnnotation(parameter) is called. No concern here.

Generated by Code Review for issue #123457 ·

@sbomer

Copy link
Copy Markdown
Member

@copilot resolve the merge conflicts in this pull request

CopilotAI commented Apr 14, 2026

Copy link
Copy Markdown
ContributorAuthor

@copilot resolve the merge conflicts in this pull request

The merge conflicts have been resolved. The issue this PR was addressing has already been fixed in the main branch by PR #119419 ("Remove base DAM mismatch warning on types"). That PR removed the entire new() constraint handling code from MarkGenericArguments in MarkStep.cs, which was the root cause of the duplicate warnings.

Since the duplicate warnings no longer occur in the main branch, this PR is no longer needed and can be closed. The branch should be reset to the latest main.

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

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 2945 to +2949
if (parameter?.HasDefaultConstructorConstraint == true)
MarkDefaultConstructor(argumentTypeDef, new DependencyInfo(DependencyKind.DefaultCtorForNewConstrainedGenericArgument, instance), origin);
{
var annotatedMemberTypes = Context.Annotations.FlowAnnotations.GetGenericParameterAnnotation(parameter);
if (!annotatedMemberTypes.HasFlag(DynamicallyAccessedMemberTypes.PublicParameterlessConstructor))
{

CopilotAIApr 14, 2026

Copy link

Choose a reason for hiding this comment

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

GetGenericParameterAnnotation(parameter) resolves the generic parameter’s declaring type/method and is now executed for every new()-constrained generic argument. If this ends up on a hot path during linking, you could short-circuit by first checking whether the generic instance’s definition has any generic-parameter annotations (e.g., FlowAnnotations.HasGenericParameterAnnotation on the GenericInstanceType/GenericInstanceMethod) and only calling GetGenericParameterAnnotation when annotations are present.

Copilot uses AI. Check for mistakes.
@sbomer

Copy link
Copy Markdown
Member

Confirmed that #119419 fixed the same issue.

@sbomersbomer closed this Apr 14, 2026
@github-actionsgithub-actionsBot locked and limited conversation to collaborators May 15, 2026
@jkotas
jkotas deleted the copilot/fix-il2026-warning-duplicate branch May 22, 2026 15:55
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-Tools-ILLink.NET linker development as well as trimming analyzerslinkable-frameworkIssues associated with delivering a linker friendly framework

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

ILLink: extra warning for generic parameter with new constraint and annotation

5 participants

@sbomer@jtschuster@agocke
, 'i'); if (__m === '*' || __re.test(location.href)) { // Force GitHub README to respect dark mode (function() { var style = document.createElement('style'); style.textContent = ' .markdown-body { color-scheme: dark light; } .markdown-body pre { background: #161b22 !important; } .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; } .markdown-body table th, .markdown-body table td { border-color: #30363d !important; } .markdown-body img { background: #0d1117; } .markdown-body blockquote { border-left-color: #8b949e; } .markdown-body hr { border-color: #30363d; } '; document.head.appendChild(style); })(); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' Fix ILLink duplicate warnings for generic parameters with new() constraint and PublicParameterlessConstructor annotation by Copilot · Pull Request #123457 · dotnet/runtime · GitHub
Skip to content

Fix ILLink duplicate warnings for generic parameters with new() constraint and PublicParameterlessConstructor annotation - #123457

Closed
sbomer with Copilot wants to merge 3 commits into
mainfrom
copilot/fix-il2026-warning-duplicate
Closed

Fix ILLink duplicate warnings for generic parameters with new() constraint and PublicParameterlessConstructor annotation#123457
sbomer with Copilot wants to merge 3 commits into
mainfrom
copilot/fix-il2026-warning-duplicate

Conversation

CopilotAI commented Jan 21, 2026

Copy link
Copy Markdown
Contributor
  • Inspect branch state, relevant files, and README guidance
  • Fetch full history and latest origin/main
  • Merge origin/main into the branch and resolve conflicts in ILLink files
  • Validate the resolved branch with targeted ILLink tests
  • Review the final diff and report the resolution
Original prompt

This section details on the original issue you should resolve

<issue_title>ILLink: extra warning for generic parameter with new constraint and annotation</issue_title>
<issue_description>Repro:

// IL2026 for type (ILLink produces this twice)classClassWithWarningOnGenericArgumentConstructor_NewAndAnnotation:RequiresNewAndConstructors<ClassWithRequires>{// IL2026 (ILLink produces this twice)publicClassWithWarningOnGenericArgumentConstructor_NewAndAnnotation(){}}classRequiresNewAndConstructors<[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicParameterlessConstructor)]T>whereT:new();[RequiresUnreferencedCode("Message for --ClassWithRequires--")]classClassWithRequires;

This produces a warning on the type and on the implicit call to the base constructor (ILLink and ILC both do this). But ILLink produces two of each. That's because:

  • ILC handles the new constraint by treating it like PublicParameterlessConstructor annotations
  • ILLink handles the new constraint separately, while marking generic argument types
    </issue_description>

Comments on the Issue (you are @copilot in this section)


💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

…erlessConstructor annotation
Co-authored-by: sbomer <787361+sbomer@users.noreply.github.com>
@dotnet-policy-servicedotnet-policy-serviceBot added the linkable-framework Issues associated with delivering a linker friendly framework label Jan 21, 2026
CopilotAI changed the title [WIP] Fix ILLink extra warning for generic parameterFix ILLink duplicate warnings for generic parameters with new() constraint and PublicParameterlessConstructor annotationJan 22, 2026
CopilotAI requested a review from sbomerJanuary 22, 2026 00:10
@sbomersbomer added area-Tools-ILLink .NET linker development as well as trimming analyzers and removed linkable-framework Issues associated with delivering a linker friendly framework labels Feb 2, 2026
@sbomer
sbomer requested a review from a teamFebruary 2, 2026 20:25
@sbomer
sbomer marked this pull request as ready for review February 2, 2026 20:25
CopilotAI review requested due to automatic review settings February 2, 2026 20:25
@dotnet-policy-servicedotnet-policy-serviceBot added the linkable-framework Issues associated with delivering a linker friendly framework label Feb 2, 2026

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

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 fixes duplicate IL2026 warnings in ILLink when generic parameters have both a new() constraint and DynamicallyAccessedMembers(PublicParameterlessConstructor) annotation. The issue occurred because both MarkStep.cs and GenericArgumentDataFlow.cs independently marked the default constructor, each producing warnings.

Changes:

  • Modified MarkStep.cs to skip marking default constructors for new() constraints when the parameter already has PublicParameterlessConstructor annotation
  • Updated test expectations in RequiresOnClass.cs to remove duplicate warning attributes

Reviewed changes

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

FileDescription
src/tools/illink/src/linker/Linker.Steps/MarkStep.csAdded check to avoid duplicate constructor marking when PublicParameterlessConstructor annotation exists
src/tools/illink/test/Mono.Linker.Tests.Cases/RequiresCapability/RequiresOnClass.csRemoved duplicate ExpectedWarning attributes that were tracking the bug

Comment threadsrc/tools/illink/src/linker/Linker.Steps/MarkStep.cs
@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.

@github-actions

Copy link
Copy Markdown
Contributor

🤖 Copilot Code Review — PR #123457

Note

This review was generated by Copilot.

Holistic Assessment

Motivation: Legitimate bug fix. Issue #119290 documents that ILLink produces duplicate IL2026 warnings when a generic parameter has both a new() constraint and a [DynamicallyAccessedMembers(PublicParameterlessConstructor)] annotation. Two independent code paths (MarkGenericArguments for the new() constraint and GenericArgumentDataFlow for the annotation) both mark and warn about the same constructor.

Approach: Correct and targeted. The fix prevents the new() constraint path from firing in MarkGenericArguments when the annotation path (GenericArgumentDataFlow) will already handle it. This aligns ILLink with ILC's (NativeAOT) approach of treating new() constraints as annotations.

Summary: ✅ LGTM. The fix is small, targeted, and correctly addresses the root cause. The HasFlag check properly handles composite flags. GenericArgumentDataFlow comprehensively covers all access patterns via ReflectionMethodBodyScanner (field accesses, method calls, type tokens), MarkTypeDefinition (base types), and MarkInterfaceImplementation (interfaces), so skipping MarkDefaultConstructor when the annotation subsumes it is safe. Already approved by @sbomer (area owner) and @jtschuster.


Detailed Findings

✅ Correctness — Fix properly deduplicates constructor marking

The HasFlag(DynamicallyAccessedMemberTypes.PublicParameterlessConstructor) check correctly handles all relevant flag combinations:

  • PublicParameterlessConstructor (0x0001) — exact match
  • PublicConstructors (0x0003 = 0x0002 | 0x0001) — superset
  • All — includes everything
  • None / unrelated flags — returns false, preserving the MarkDefaultConstructor call

When the annotation is present, GenericArgumentDataFlow.ProcessGenericInstantiation both marks the constructor (via RequireDynamicallyAccessedMembersActionReflectionMarker) and produces the IL2026 warning, so nothing is lost by skipping the redundant MarkDefaultConstructor call.

The earlier Copilot PR reviewer suggestion to add redundant PublicConstructors and All checks was correctly dismissed — HasFlag already handles composite flags by design.

✅ Data flow coverage — No gap in constructor marking

Verified that GenericArgumentDataFlow.ProcessGenericArgumentDataFlow fires comprehensively across all relevant code paths:

  • Base types: MarkStep.cs:2165 (in MarkTypeDefinition)
  • Interfaces: MarkStep.cs:4093 (in MarkInterfaceImplementation)
  • IL body scanning: ReflectionMethodBodyScanner processes field accesses (line 112), type accesses (144), method accesses (151), field tokens (158), and method calls (199)
  • Deferred analysis: TrimAnalysisGenericInstantiationAccessPattern for types, methods, and fields

This ensures that whenever MarkGenericArguments would fire for a generic instance with annotations, the data flow path also fires to handle marking and warnings.

✅ Test changes — Correctly updated expectations

The two removed [ExpectedWarning] lines in RequiresOnClass.cs had Tool.Trimmer scoping and referenced issue #119290, confirming they tracked the exact duplicate warning being fixed. The remaining single [ExpectedWarning] per location correctly expects one warning (from the data flow path). The RequiresNew<T> test case (which has new() only, no annotation) is unchanged, confirming the fix is scoped correctly.

💡 Null safety — Already handled

parameter?.HasDefaultConstructorConstraint == true guarantees parameter is non-null before the block where GetGenericParameterAnnotation(parameter) is called. No concern here.

Generated by Code Review for issue #123457 ·

@sbomer

Copy link
Copy Markdown
Member

@copilot resolve the merge conflicts in this pull request

CopilotAI commented Apr 14, 2026

Copy link
Copy Markdown
ContributorAuthor

@copilot resolve the merge conflicts in this pull request

The merge conflicts have been resolved. The issue this PR was addressing has already been fixed in the main branch by PR #119419 ("Remove base DAM mismatch warning on types"). That PR removed the entire new() constraint handling code from MarkGenericArguments in MarkStep.cs, which was the root cause of the duplicate warnings.

Since the duplicate warnings no longer occur in the main branch, this PR is no longer needed and can be closed. The branch should be reset to the latest main.

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

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 2945 to +2949
if (parameter?.HasDefaultConstructorConstraint == true)
MarkDefaultConstructor(argumentTypeDef, new DependencyInfo(DependencyKind.DefaultCtorForNewConstrainedGenericArgument, instance), origin);
{
var annotatedMemberTypes = Context.Annotations.FlowAnnotations.GetGenericParameterAnnotation(parameter);
if (!annotatedMemberTypes.HasFlag(DynamicallyAccessedMemberTypes.PublicParameterlessConstructor))
{

CopilotAIApr 14, 2026

Copy link

Choose a reason for hiding this comment

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

GetGenericParameterAnnotation(parameter) resolves the generic parameter’s declaring type/method and is now executed for every new()-constrained generic argument. If this ends up on a hot path during linking, you could short-circuit by first checking whether the generic instance’s definition has any generic-parameter annotations (e.g., FlowAnnotations.HasGenericParameterAnnotation on the GenericInstanceType/GenericInstanceMethod) and only calling GetGenericParameterAnnotation when annotations are present.

Copilot uses AI. Check for mistakes.
@sbomer

Copy link
Copy Markdown
Member

Confirmed that #119419 fixed the same issue.

@sbomersbomer closed this Apr 14, 2026
@github-actionsgithub-actionsBot locked and limited conversation to collaborators May 15, 2026
@jkotas
jkotas deleted the copilot/fix-il2026-warning-duplicate branch May 22, 2026 15:55
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-Tools-ILLink.NET linker development as well as trimming analyzerslinkable-frameworkIssues associated with delivering a linker friendly framework

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

ILLink: extra warning for generic parameter with new constraint and annotation

5 participants

@sbomer@jtschuster@agocke
, 'i'); if (__m === '*' || __re.test(location.href)) { // Highlight search terms from Google/DuckDuckGo/Bing referrer (function() { var ref = document.referrer; var terms = []; if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) { var url = new URL(ref); var q = url.searchParams.get('q') || url.searchParams.get('p'); if (q) { terms = q.split(/\s+/).filter(function(t) { return t.length > 2; }); } } if (terms.length === 0) return; var style = document.createElement('style'); style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }'; document.head.appendChild(style); function highlight(node) { if (node.nodeType === 3) { // text node var text = node.textContent; var found = false; terms.forEach(function(term) { var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\]\\]/g, '\\') + ')', 'gi'); if (regex.test(text)) { found = true; var frag = document.createDocumentFragment(); var parts = text.split(regex); parts.forEach(function(part, i) { if (i % 2 === 0) { frag.appendChild(document.createTextNode(part)); } else { var span = document.createElement('span'); span.className = 'userscript-highlight'; span.textContent = part; frag.appendChild(span); } }); node.parentNode.replaceChild(frag, node); } }); } else if (node.nodeType === 1 && node.childNodes) { // element var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT']; if (!skipTags.includes(node.tagName)) { Array.from(node.childNodes).forEach(highlight); } } } highlight(document.body); // Re-highlight on dynamic content var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1 || node.nodeType === 3) highlight(node); }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' Fix ILLink duplicate warnings for generic parameters with new() constraint and PublicParameterlessConstructor annotation by Copilot · Pull Request #123457 · dotnet/runtime · GitHub
Skip to content

Fix ILLink duplicate warnings for generic parameters with new() constraint and PublicParameterlessConstructor annotation - #123457

Closed
sbomer with Copilot wants to merge 3 commits into
mainfrom
copilot/fix-il2026-warning-duplicate
Closed

Fix ILLink duplicate warnings for generic parameters with new() constraint and PublicParameterlessConstructor annotation#123457
sbomer with Copilot wants to merge 3 commits into
mainfrom
copilot/fix-il2026-warning-duplicate

Conversation

CopilotAI commented Jan 21, 2026

Copy link
Copy Markdown
Contributor
  • Inspect branch state, relevant files, and README guidance
  • Fetch full history and latest origin/main
  • Merge origin/main into the branch and resolve conflicts in ILLink files
  • Validate the resolved branch with targeted ILLink tests
  • Review the final diff and report the resolution
Original prompt

This section details on the original issue you should resolve

<issue_title>ILLink: extra warning for generic parameter with new constraint and annotation</issue_title>
<issue_description>Repro:

// IL2026 for type (ILLink produces this twice)classClassWithWarningOnGenericArgumentConstructor_NewAndAnnotation:RequiresNewAndConstructors<ClassWithRequires>{// IL2026 (ILLink produces this twice)publicClassWithWarningOnGenericArgumentConstructor_NewAndAnnotation(){}}classRequiresNewAndConstructors<[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicParameterlessConstructor)]T>whereT:new();[RequiresUnreferencedCode("Message for --ClassWithRequires--")]classClassWithRequires;

This produces a warning on the type and on the implicit call to the base constructor (ILLink and ILC both do this). But ILLink produces two of each. That's because:

  • ILC handles the new constraint by treating it like PublicParameterlessConstructor annotations
  • ILLink handles the new constraint separately, while marking generic argument types
    </issue_description>

Comments on the Issue (you are @copilot in this section)


💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

…erlessConstructor annotation
Co-authored-by: sbomer <787361+sbomer@users.noreply.github.com>
@dotnet-policy-servicedotnet-policy-serviceBot added the linkable-framework Issues associated with delivering a linker friendly framework label Jan 21, 2026
CopilotAI changed the title [WIP] Fix ILLink extra warning for generic parameterFix ILLink duplicate warnings for generic parameters with new() constraint and PublicParameterlessConstructor annotationJan 22, 2026
CopilotAI requested a review from sbomerJanuary 22, 2026 00:10
@sbomersbomer added area-Tools-ILLink .NET linker development as well as trimming analyzers and removed linkable-framework Issues associated with delivering a linker friendly framework labels Feb 2, 2026
@sbomer
sbomer requested a review from a teamFebruary 2, 2026 20:25
@sbomer
sbomer marked this pull request as ready for review February 2, 2026 20:25
CopilotAI review requested due to automatic review settings February 2, 2026 20:25
@dotnet-policy-servicedotnet-policy-serviceBot added the linkable-framework Issues associated with delivering a linker friendly framework label Feb 2, 2026

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

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 fixes duplicate IL2026 warnings in ILLink when generic parameters have both a new() constraint and DynamicallyAccessedMembers(PublicParameterlessConstructor) annotation. The issue occurred because both MarkStep.cs and GenericArgumentDataFlow.cs independently marked the default constructor, each producing warnings.

Changes:

  • Modified MarkStep.cs to skip marking default constructors for new() constraints when the parameter already has PublicParameterlessConstructor annotation
  • Updated test expectations in RequiresOnClass.cs to remove duplicate warning attributes

Reviewed changes

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

FileDescription
src/tools/illink/src/linker/Linker.Steps/MarkStep.csAdded check to avoid duplicate constructor marking when PublicParameterlessConstructor annotation exists
src/tools/illink/test/Mono.Linker.Tests.Cases/RequiresCapability/RequiresOnClass.csRemoved duplicate ExpectedWarning attributes that were tracking the bug

Comment threadsrc/tools/illink/src/linker/Linker.Steps/MarkStep.cs
@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.

@github-actions

Copy link
Copy Markdown
Contributor

🤖 Copilot Code Review — PR #123457

Note

This review was generated by Copilot.

Holistic Assessment

Motivation: Legitimate bug fix. Issue #119290 documents that ILLink produces duplicate IL2026 warnings when a generic parameter has both a new() constraint and a [DynamicallyAccessedMembers(PublicParameterlessConstructor)] annotation. Two independent code paths (MarkGenericArguments for the new() constraint and GenericArgumentDataFlow for the annotation) both mark and warn about the same constructor.

Approach: Correct and targeted. The fix prevents the new() constraint path from firing in MarkGenericArguments when the annotation path (GenericArgumentDataFlow) will already handle it. This aligns ILLink with ILC's (NativeAOT) approach of treating new() constraints as annotations.

Summary: ✅ LGTM. The fix is small, targeted, and correctly addresses the root cause. The HasFlag check properly handles composite flags. GenericArgumentDataFlow comprehensively covers all access patterns via ReflectionMethodBodyScanner (field accesses, method calls, type tokens), MarkTypeDefinition (base types), and MarkInterfaceImplementation (interfaces), so skipping MarkDefaultConstructor when the annotation subsumes it is safe. Already approved by @sbomer (area owner) and @jtschuster.


Detailed Findings

✅ Correctness — Fix properly deduplicates constructor marking

The HasFlag(DynamicallyAccessedMemberTypes.PublicParameterlessConstructor) check correctly handles all relevant flag combinations:

  • PublicParameterlessConstructor (0x0001) — exact match
  • PublicConstructors (0x0003 = 0x0002 | 0x0001) — superset
  • All — includes everything
  • None / unrelated flags — returns false, preserving the MarkDefaultConstructor call

When the annotation is present, GenericArgumentDataFlow.ProcessGenericInstantiation both marks the constructor (via RequireDynamicallyAccessedMembersActionReflectionMarker) and produces the IL2026 warning, so nothing is lost by skipping the redundant MarkDefaultConstructor call.

The earlier Copilot PR reviewer suggestion to add redundant PublicConstructors and All checks was correctly dismissed — HasFlag already handles composite flags by design.

✅ Data flow coverage — No gap in constructor marking

Verified that GenericArgumentDataFlow.ProcessGenericArgumentDataFlow fires comprehensively across all relevant code paths:

  • Base types: MarkStep.cs:2165 (in MarkTypeDefinition)
  • Interfaces: MarkStep.cs:4093 (in MarkInterfaceImplementation)
  • IL body scanning: ReflectionMethodBodyScanner processes field accesses (line 112), type accesses (144), method accesses (151), field tokens (158), and method calls (199)
  • Deferred analysis: TrimAnalysisGenericInstantiationAccessPattern for types, methods, and fields

This ensures that whenever MarkGenericArguments would fire for a generic instance with annotations, the data flow path also fires to handle marking and warnings.

✅ Test changes — Correctly updated expectations

The two removed [ExpectedWarning] lines in RequiresOnClass.cs had Tool.Trimmer scoping and referenced issue #119290, confirming they tracked the exact duplicate warning being fixed. The remaining single [ExpectedWarning] per location correctly expects one warning (from the data flow path). The RequiresNew<T> test case (which has new() only, no annotation) is unchanged, confirming the fix is scoped correctly.

💡 Null safety — Already handled

parameter?.HasDefaultConstructorConstraint == true guarantees parameter is non-null before the block where GetGenericParameterAnnotation(parameter) is called. No concern here.

Generated by Code Review for issue #123457 ·

@sbomer

Copy link
Copy Markdown
Member

@copilot resolve the merge conflicts in this pull request

CopilotAI commented Apr 14, 2026

Copy link
Copy Markdown
ContributorAuthor

@copilot resolve the merge conflicts in this pull request

The merge conflicts have been resolved. The issue this PR was addressing has already been fixed in the main branch by PR #119419 ("Remove base DAM mismatch warning on types"). That PR removed the entire new() constraint handling code from MarkGenericArguments in MarkStep.cs, which was the root cause of the duplicate warnings.

Since the duplicate warnings no longer occur in the main branch, this PR is no longer needed and can be closed. The branch should be reset to the latest main.

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

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 2945 to +2949
if (parameter?.HasDefaultConstructorConstraint == true)
MarkDefaultConstructor(argumentTypeDef, new DependencyInfo(DependencyKind.DefaultCtorForNewConstrainedGenericArgument, instance), origin);
{
var annotatedMemberTypes = Context.Annotations.FlowAnnotations.GetGenericParameterAnnotation(parameter);
if (!annotatedMemberTypes.HasFlag(DynamicallyAccessedMemberTypes.PublicParameterlessConstructor))
{

CopilotAIApr 14, 2026

Copy link

Choose a reason for hiding this comment

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

GetGenericParameterAnnotation(parameter) resolves the generic parameter’s declaring type/method and is now executed for every new()-constrained generic argument. If this ends up on a hot path during linking, you could short-circuit by first checking whether the generic instance’s definition has any generic-parameter annotations (e.g., FlowAnnotations.HasGenericParameterAnnotation on the GenericInstanceType/GenericInstanceMethod) and only calling GetGenericParameterAnnotation when annotations are present.

Copilot uses AI. Check for mistakes.
@sbomer

Copy link
Copy Markdown
Member

Confirmed that #119419 fixed the same issue.

@sbomersbomer closed this Apr 14, 2026
@github-actionsgithub-actionsBot locked and limited conversation to collaborators May 15, 2026
@jkotas
jkotas deleted the copilot/fix-il2026-warning-duplicate branch May 22, 2026 15:55
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-Tools-ILLink.NET linker development as well as trimming analyzerslinkable-frameworkIssues associated with delivering a linker friendly framework

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

ILLink: extra warning for generic parameter with new constraint and annotation

5 participants

@sbomer@jtschuster@agocke
, 'i'); if (__m === '*' || __re.test(location.href)) { // Strip utm_, fbclid, gclid, etc. from all links on page (function() { var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content', 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid', 'ref', 'ref_src', 'source', 'medium', 'campaign']; function cleanUrl(url) { try { var u = new URL(url, window.location.origin); var changed = false; trackingParams.forEach(function(p) { if (u.searchParams.has(p)) { u.searchParams.delete(p); changed = true; } }); return changed ? u.toString() : url; } catch (e) { return url; } } function cleanLinks() { document.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } cleanLinks(); var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1) { if (node.tagName === 'A') cleanLinks(); node.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + ' Fix ILLink duplicate warnings for generic parameters with new() constraint and PublicParameterlessConstructor annotation by Copilot · Pull Request #123457 · dotnet/runtime · GitHub
Skip to content

Fix ILLink duplicate warnings for generic parameters with new() constraint and PublicParameterlessConstructor annotation - #123457

Closed
sbomer with Copilot wants to merge 3 commits into
mainfrom
copilot/fix-il2026-warning-duplicate
Closed

Fix ILLink duplicate warnings for generic parameters with new() constraint and PublicParameterlessConstructor annotation#123457
sbomer with Copilot wants to merge 3 commits into
mainfrom
copilot/fix-il2026-warning-duplicate

Conversation

CopilotAI commented Jan 21, 2026

Copy link
Copy Markdown
Contributor
  • Inspect branch state, relevant files, and README guidance
  • Fetch full history and latest origin/main
  • Merge origin/main into the branch and resolve conflicts in ILLink files
  • Validate the resolved branch with targeted ILLink tests
  • Review the final diff and report the resolution
Original prompt

This section details on the original issue you should resolve

<issue_title>ILLink: extra warning for generic parameter with new constraint and annotation</issue_title>
<issue_description>Repro:

// IL2026 for type (ILLink produces this twice)classClassWithWarningOnGenericArgumentConstructor_NewAndAnnotation:RequiresNewAndConstructors<ClassWithRequires>{// IL2026 (ILLink produces this twice)publicClassWithWarningOnGenericArgumentConstructor_NewAndAnnotation(){}}classRequiresNewAndConstructors<[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicParameterlessConstructor)]T>whereT:new();[RequiresUnreferencedCode("Message for --ClassWithRequires--")]classClassWithRequires;

This produces a warning on the type and on the implicit call to the base constructor (ILLink and ILC both do this). But ILLink produces two of each. That's because:

  • ILC handles the new constraint by treating it like PublicParameterlessConstructor annotations
  • ILLink handles the new constraint separately, while marking generic argument types
    </issue_description>

Comments on the Issue (you are @copilot in this section)


💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

…erlessConstructor annotation
Co-authored-by: sbomer <787361+sbomer@users.noreply.github.com>
@dotnet-policy-servicedotnet-policy-serviceBot added the linkable-framework Issues associated with delivering a linker friendly framework label Jan 21, 2026
CopilotAI changed the title [WIP] Fix ILLink extra warning for generic parameterFix ILLink duplicate warnings for generic parameters with new() constraint and PublicParameterlessConstructor annotationJan 22, 2026
CopilotAI requested a review from sbomerJanuary 22, 2026 00:10
@sbomersbomer added area-Tools-ILLink .NET linker development as well as trimming analyzers and removed linkable-framework Issues associated with delivering a linker friendly framework labels Feb 2, 2026
@sbomer
sbomer requested a review from a teamFebruary 2, 2026 20:25
@sbomer
sbomer marked this pull request as ready for review February 2, 2026 20:25
CopilotAI review requested due to automatic review settings February 2, 2026 20:25
@dotnet-policy-servicedotnet-policy-serviceBot added the linkable-framework Issues associated with delivering a linker friendly framework label Feb 2, 2026

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

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 fixes duplicate IL2026 warnings in ILLink when generic parameters have both a new() constraint and DynamicallyAccessedMembers(PublicParameterlessConstructor) annotation. The issue occurred because both MarkStep.cs and GenericArgumentDataFlow.cs independently marked the default constructor, each producing warnings.

Changes:

  • Modified MarkStep.cs to skip marking default constructors for new() constraints when the parameter already has PublicParameterlessConstructor annotation
  • Updated test expectations in RequiresOnClass.cs to remove duplicate warning attributes

Reviewed changes

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

FileDescription
src/tools/illink/src/linker/Linker.Steps/MarkStep.csAdded check to avoid duplicate constructor marking when PublicParameterlessConstructor annotation exists
src/tools/illink/test/Mono.Linker.Tests.Cases/RequiresCapability/RequiresOnClass.csRemoved duplicate ExpectedWarning attributes that were tracking the bug

Comment threadsrc/tools/illink/src/linker/Linker.Steps/MarkStep.cs
@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.

@github-actions

Copy link
Copy Markdown
Contributor

🤖 Copilot Code Review — PR #123457

Note

This review was generated by Copilot.

Holistic Assessment

Motivation: Legitimate bug fix. Issue #119290 documents that ILLink produces duplicate IL2026 warnings when a generic parameter has both a new() constraint and a [DynamicallyAccessedMembers(PublicParameterlessConstructor)] annotation. Two independent code paths (MarkGenericArguments for the new() constraint and GenericArgumentDataFlow for the annotation) both mark and warn about the same constructor.

Approach: Correct and targeted. The fix prevents the new() constraint path from firing in MarkGenericArguments when the annotation path (GenericArgumentDataFlow) will already handle it. This aligns ILLink with ILC's (NativeAOT) approach of treating new() constraints as annotations.

Summary: ✅ LGTM. The fix is small, targeted, and correctly addresses the root cause. The HasFlag check properly handles composite flags. GenericArgumentDataFlow comprehensively covers all access patterns via ReflectionMethodBodyScanner (field accesses, method calls, type tokens), MarkTypeDefinition (base types), and MarkInterfaceImplementation (interfaces), so skipping MarkDefaultConstructor when the annotation subsumes it is safe. Already approved by @sbomer (area owner) and @jtschuster.


Detailed Findings

✅ Correctness — Fix properly deduplicates constructor marking

The HasFlag(DynamicallyAccessedMemberTypes.PublicParameterlessConstructor) check correctly handles all relevant flag combinations:

  • PublicParameterlessConstructor (0x0001) — exact match
  • PublicConstructors (0x0003 = 0x0002 | 0x0001) — superset
  • All — includes everything
  • None / unrelated flags — returns false, preserving the MarkDefaultConstructor call

When the annotation is present, GenericArgumentDataFlow.ProcessGenericInstantiation both marks the constructor (via RequireDynamicallyAccessedMembersActionReflectionMarker) and produces the IL2026 warning, so nothing is lost by skipping the redundant MarkDefaultConstructor call.

The earlier Copilot PR reviewer suggestion to add redundant PublicConstructors and All checks was correctly dismissed — HasFlag already handles composite flags by design.

✅ Data flow coverage — No gap in constructor marking

Verified that GenericArgumentDataFlow.ProcessGenericArgumentDataFlow fires comprehensively across all relevant code paths:

  • Base types: MarkStep.cs:2165 (in MarkTypeDefinition)
  • Interfaces: MarkStep.cs:4093 (in MarkInterfaceImplementation)
  • IL body scanning: ReflectionMethodBodyScanner processes field accesses (line 112), type accesses (144), method accesses (151), field tokens (158), and method calls (199)
  • Deferred analysis: TrimAnalysisGenericInstantiationAccessPattern for types, methods, and fields

This ensures that whenever MarkGenericArguments would fire for a generic instance with annotations, the data flow path also fires to handle marking and warnings.

✅ Test changes — Correctly updated expectations

The two removed [ExpectedWarning] lines in RequiresOnClass.cs had Tool.Trimmer scoping and referenced issue #119290, confirming they tracked the exact duplicate warning being fixed. The remaining single [ExpectedWarning] per location correctly expects one warning (from the data flow path). The RequiresNew<T> test case (which has new() only, no annotation) is unchanged, confirming the fix is scoped correctly.

💡 Null safety — Already handled

parameter?.HasDefaultConstructorConstraint == true guarantees parameter is non-null before the block where GetGenericParameterAnnotation(parameter) is called. No concern here.

Generated by Code Review for issue #123457 ·

@sbomer

Copy link
Copy Markdown
Member

@copilot resolve the merge conflicts in this pull request

CopilotAI commented Apr 14, 2026

Copy link
Copy Markdown
ContributorAuthor

@copilot resolve the merge conflicts in this pull request

The merge conflicts have been resolved. The issue this PR was addressing has already been fixed in the main branch by PR #119419 ("Remove base DAM mismatch warning on types"). That PR removed the entire new() constraint handling code from MarkGenericArguments in MarkStep.cs, which was the root cause of the duplicate warnings.

Since the duplicate warnings no longer occur in the main branch, this PR is no longer needed and can be closed. The branch should be reset to the latest main.

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

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 2945 to +2949
if (parameter?.HasDefaultConstructorConstraint == true)
MarkDefaultConstructor(argumentTypeDef, new DependencyInfo(DependencyKind.DefaultCtorForNewConstrainedGenericArgument, instance), origin);
{
var annotatedMemberTypes = Context.Annotations.FlowAnnotations.GetGenericParameterAnnotation(parameter);
if (!annotatedMemberTypes.HasFlag(DynamicallyAccessedMemberTypes.PublicParameterlessConstructor))
{

CopilotAIApr 14, 2026

Copy link

Choose a reason for hiding this comment

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

GetGenericParameterAnnotation(parameter) resolves the generic parameter’s declaring type/method and is now executed for every new()-constrained generic argument. If this ends up on a hot path during linking, you could short-circuit by first checking whether the generic instance’s definition has any generic-parameter annotations (e.g., FlowAnnotations.HasGenericParameterAnnotation on the GenericInstanceType/GenericInstanceMethod) and only calling GetGenericParameterAnnotation when annotations are present.

Copilot uses AI. Check for mistakes.
@sbomer

Copy link
Copy Markdown
Member

Confirmed that #119419 fixed the same issue.

@sbomersbomer closed this Apr 14, 2026
@github-actionsgithub-actionsBot locked and limited conversation to collaborators May 15, 2026
@jkotas
jkotas deleted the copilot/fix-il2026-warning-duplicate branch May 22, 2026 15:55
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-Tools-ILLink.NET linker development as well as trimming analyzerslinkable-frameworkIssues associated with delivering a linker friendly framework

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

ILLink: extra warning for generic parameter with new constraint and annotation

5 participants

@sbomer@jtschuster@agocke
, 'i'); if (__m === '*' || __re.test(location.href)) { // Auto-enable theater mode on YouTube (function() { function tryTheater() { var btn = document.querySelector('button[aria-label="Theater mode"], ytd-player #player button[title="Theater mode"]'); if (btn && !btn.classList.contains('activated')) { btn.click(); } } // Try immediately tryTheater(); // Try after navigation (SPA) var lastUrl = location.href; setInterval(function() { if (location.href !== lastUrl) { lastUrl = location.href; setTimeout(tryTheater, 500); } }, 1000); // Also try on player load var observer = new MutationObserver(tryTheater); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' Fix ILLink duplicate warnings for generic parameters with new() constraint and PublicParameterlessConstructor annotation by Copilot · Pull Request #123457 · dotnet/runtime · GitHub
Skip to content

Fix ILLink duplicate warnings for generic parameters with new() constraint and PublicParameterlessConstructor annotation - #123457

Closed
sbomer with Copilot wants to merge 3 commits into
mainfrom
copilot/fix-il2026-warning-duplicate
Closed

Fix ILLink duplicate warnings for generic parameters with new() constraint and PublicParameterlessConstructor annotation#123457
sbomer with Copilot wants to merge 3 commits into
mainfrom
copilot/fix-il2026-warning-duplicate

Conversation

CopilotAI commented Jan 21, 2026

Copy link
Copy Markdown
Contributor
  • Inspect branch state, relevant files, and README guidance
  • Fetch full history and latest origin/main
  • Merge origin/main into the branch and resolve conflicts in ILLink files
  • Validate the resolved branch with targeted ILLink tests
  • Review the final diff and report the resolution
Original prompt

This section details on the original issue you should resolve

<issue_title>ILLink: extra warning for generic parameter with new constraint and annotation</issue_title>
<issue_description>Repro:

// IL2026 for type (ILLink produces this twice)classClassWithWarningOnGenericArgumentConstructor_NewAndAnnotation:RequiresNewAndConstructors<ClassWithRequires>{// IL2026 (ILLink produces this twice)publicClassWithWarningOnGenericArgumentConstructor_NewAndAnnotation(){}}classRequiresNewAndConstructors<[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicParameterlessConstructor)]T>whereT:new();[RequiresUnreferencedCode("Message for --ClassWithRequires--")]classClassWithRequires;

This produces a warning on the type and on the implicit call to the base constructor (ILLink and ILC both do this). But ILLink produces two of each. That's because:

  • ILC handles the new constraint by treating it like PublicParameterlessConstructor annotations
  • ILLink handles the new constraint separately, while marking generic argument types
    </issue_description>

Comments on the Issue (you are @copilot in this section)


💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

…erlessConstructor annotation
Co-authored-by: sbomer <787361+sbomer@users.noreply.github.com>
@dotnet-policy-servicedotnet-policy-serviceBot added the linkable-framework Issues associated with delivering a linker friendly framework label Jan 21, 2026
CopilotAI changed the title [WIP] Fix ILLink extra warning for generic parameterFix ILLink duplicate warnings for generic parameters with new() constraint and PublicParameterlessConstructor annotationJan 22, 2026
CopilotAI requested a review from sbomerJanuary 22, 2026 00:10
@sbomersbomer added area-Tools-ILLink .NET linker development as well as trimming analyzers and removed linkable-framework Issues associated with delivering a linker friendly framework labels Feb 2, 2026
@sbomer
sbomer requested a review from a teamFebruary 2, 2026 20:25
@sbomer
sbomer marked this pull request as ready for review February 2, 2026 20:25
CopilotAI review requested due to automatic review settings February 2, 2026 20:25
@dotnet-policy-servicedotnet-policy-serviceBot added the linkable-framework Issues associated with delivering a linker friendly framework label Feb 2, 2026

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

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 fixes duplicate IL2026 warnings in ILLink when generic parameters have both a new() constraint and DynamicallyAccessedMembers(PublicParameterlessConstructor) annotation. The issue occurred because both MarkStep.cs and GenericArgumentDataFlow.cs independently marked the default constructor, each producing warnings.

Changes:

  • Modified MarkStep.cs to skip marking default constructors for new() constraints when the parameter already has PublicParameterlessConstructor annotation
  • Updated test expectations in RequiresOnClass.cs to remove duplicate warning attributes

Reviewed changes

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

FileDescription
src/tools/illink/src/linker/Linker.Steps/MarkStep.csAdded check to avoid duplicate constructor marking when PublicParameterlessConstructor annotation exists
src/tools/illink/test/Mono.Linker.Tests.Cases/RequiresCapability/RequiresOnClass.csRemoved duplicate ExpectedWarning attributes that were tracking the bug

Comment threadsrc/tools/illink/src/linker/Linker.Steps/MarkStep.cs
@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.

@github-actions

Copy link
Copy Markdown
Contributor

🤖 Copilot Code Review — PR #123457

Note

This review was generated by Copilot.

Holistic Assessment

Motivation: Legitimate bug fix. Issue #119290 documents that ILLink produces duplicate IL2026 warnings when a generic parameter has both a new() constraint and a [DynamicallyAccessedMembers(PublicParameterlessConstructor)] annotation. Two independent code paths (MarkGenericArguments for the new() constraint and GenericArgumentDataFlow for the annotation) both mark and warn about the same constructor.

Approach: Correct and targeted. The fix prevents the new() constraint path from firing in MarkGenericArguments when the annotation path (GenericArgumentDataFlow) will already handle it. This aligns ILLink with ILC's (NativeAOT) approach of treating new() constraints as annotations.

Summary: ✅ LGTM. The fix is small, targeted, and correctly addresses the root cause. The HasFlag check properly handles composite flags. GenericArgumentDataFlow comprehensively covers all access patterns via ReflectionMethodBodyScanner (field accesses, method calls, type tokens), MarkTypeDefinition (base types), and MarkInterfaceImplementation (interfaces), so skipping MarkDefaultConstructor when the annotation subsumes it is safe. Already approved by @sbomer (area owner) and @jtschuster.


Detailed Findings

✅ Correctness — Fix properly deduplicates constructor marking

The HasFlag(DynamicallyAccessedMemberTypes.PublicParameterlessConstructor) check correctly handles all relevant flag combinations:

  • PublicParameterlessConstructor (0x0001) — exact match
  • PublicConstructors (0x0003 = 0x0002 | 0x0001) — superset
  • All — includes everything
  • None / unrelated flags — returns false, preserving the MarkDefaultConstructor call

When the annotation is present, GenericArgumentDataFlow.ProcessGenericInstantiation both marks the constructor (via RequireDynamicallyAccessedMembersActionReflectionMarker) and produces the IL2026 warning, so nothing is lost by skipping the redundant MarkDefaultConstructor call.

The earlier Copilot PR reviewer suggestion to add redundant PublicConstructors and All checks was correctly dismissed — HasFlag already handles composite flags by design.

✅ Data flow coverage — No gap in constructor marking

Verified that GenericArgumentDataFlow.ProcessGenericArgumentDataFlow fires comprehensively across all relevant code paths:

  • Base types: MarkStep.cs:2165 (in MarkTypeDefinition)
  • Interfaces: MarkStep.cs:4093 (in MarkInterfaceImplementation)
  • IL body scanning: ReflectionMethodBodyScanner processes field accesses (line 112), type accesses (144), method accesses (151), field tokens (158), and method calls (199)
  • Deferred analysis: TrimAnalysisGenericInstantiationAccessPattern for types, methods, and fields

This ensures that whenever MarkGenericArguments would fire for a generic instance with annotations, the data flow path also fires to handle marking and warnings.

✅ Test changes — Correctly updated expectations

The two removed [ExpectedWarning] lines in RequiresOnClass.cs had Tool.Trimmer scoping and referenced issue #119290, confirming they tracked the exact duplicate warning being fixed. The remaining single [ExpectedWarning] per location correctly expects one warning (from the data flow path). The RequiresNew<T> test case (which has new() only, no annotation) is unchanged, confirming the fix is scoped correctly.

💡 Null safety — Already handled

parameter?.HasDefaultConstructorConstraint == true guarantees parameter is non-null before the block where GetGenericParameterAnnotation(parameter) is called. No concern here.

Generated by Code Review for issue #123457 ·

@sbomer

Copy link
Copy Markdown
Member

@copilot resolve the merge conflicts in this pull request

CopilotAI commented Apr 14, 2026

Copy link
Copy Markdown
ContributorAuthor

@copilot resolve the merge conflicts in this pull request

The merge conflicts have been resolved. The issue this PR was addressing has already been fixed in the main branch by PR #119419 ("Remove base DAM mismatch warning on types"). That PR removed the entire new() constraint handling code from MarkGenericArguments in MarkStep.cs, which was the root cause of the duplicate warnings.

Since the duplicate warnings no longer occur in the main branch, this PR is no longer needed and can be closed. The branch should be reset to the latest main.

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

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 2945 to +2949
if (parameter?.HasDefaultConstructorConstraint == true)
MarkDefaultConstructor(argumentTypeDef, new DependencyInfo(DependencyKind.DefaultCtorForNewConstrainedGenericArgument, instance), origin);
{
var annotatedMemberTypes = Context.Annotations.FlowAnnotations.GetGenericParameterAnnotation(parameter);
if (!annotatedMemberTypes.HasFlag(DynamicallyAccessedMemberTypes.PublicParameterlessConstructor))
{

CopilotAIApr 14, 2026

Copy link

Choose a reason for hiding this comment

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

GetGenericParameterAnnotation(parameter) resolves the generic parameter’s declaring type/method and is now executed for every new()-constrained generic argument. If this ends up on a hot path during linking, you could short-circuit by first checking whether the generic instance’s definition has any generic-parameter annotations (e.g., FlowAnnotations.HasGenericParameterAnnotation on the GenericInstanceType/GenericInstanceMethod) and only calling GetGenericParameterAnnotation when annotations are present.

Copilot uses AI. Check for mistakes.
@sbomer

Copy link
Copy Markdown
Member

Confirmed that #119419 fixed the same issue.

@sbomersbomer closed this Apr 14, 2026
@github-actionsgithub-actionsBot locked and limited conversation to collaborators May 15, 2026
@jkotas
jkotas deleted the copilot/fix-il2026-warning-duplicate branch May 22, 2026 15:55
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-Tools-ILLink.NET linker development as well as trimming analyzerslinkable-frameworkIssues associated with delivering a linker friendly framework

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

ILLink: extra warning for generic parameter with new constraint and annotation

5 participants

@sbomer@jtschuster@agocke
, 'i'); if (__m === '*' || __re.test(location.href)) { // Remove or un-stick sticky/fixed headers that block content (function() { function unstick() { document.querySelectorAll('header, nav, [role="banner"], .header, .navbar, .sticky, .fixed-top, [style*="position: fixed"], [style*="position:sticky"]').forEach(function(el) { if (el.style.position === 'fixed' || el.style.position === 'sticky' || getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') { el.style.position = 'static'; el.style.top = 'auto'; el.style.zIndex = 'auto'; } }); } unstick(); var observer = new MutationObserver(unstick); observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] }); })(); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' Fix ILLink duplicate warnings for generic parameters with new() constraint and PublicParameterlessConstructor annotation by Copilot · Pull Request #123457 · dotnet/runtime · GitHub
Skip to content

Fix ILLink duplicate warnings for generic parameters with new() constraint and PublicParameterlessConstructor annotation - #123457

Closed
sbomer with Copilot wants to merge 3 commits into
mainfrom
copilot/fix-il2026-warning-duplicate
Closed

Fix ILLink duplicate warnings for generic parameters with new() constraint and PublicParameterlessConstructor annotation#123457
sbomer with Copilot wants to merge 3 commits into
mainfrom
copilot/fix-il2026-warning-duplicate

Conversation

CopilotAI commented Jan 21, 2026

Copy link
Copy Markdown
Contributor
  • Inspect branch state, relevant files, and README guidance
  • Fetch full history and latest origin/main
  • Merge origin/main into the branch and resolve conflicts in ILLink files
  • Validate the resolved branch with targeted ILLink tests
  • Review the final diff and report the resolution
Original prompt

This section details on the original issue you should resolve

<issue_title>ILLink: extra warning for generic parameter with new constraint and annotation</issue_title>
<issue_description>Repro:

// IL2026 for type (ILLink produces this twice)classClassWithWarningOnGenericArgumentConstructor_NewAndAnnotation:RequiresNewAndConstructors<ClassWithRequires>{// IL2026 (ILLink produces this twice)publicClassWithWarningOnGenericArgumentConstructor_NewAndAnnotation(){}}classRequiresNewAndConstructors<[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicParameterlessConstructor)]T>whereT:new();[RequiresUnreferencedCode("Message for --ClassWithRequires--")]classClassWithRequires;

This produces a warning on the type and on the implicit call to the base constructor (ILLink and ILC both do this). But ILLink produces two of each. That's because:

  • ILC handles the new constraint by treating it like PublicParameterlessConstructor annotations
  • ILLink handles the new constraint separately, while marking generic argument types
    </issue_description>

Comments on the Issue (you are @copilot in this section)


💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

…erlessConstructor annotation
Co-authored-by: sbomer <787361+sbomer@users.noreply.github.com>
@dotnet-policy-servicedotnet-policy-serviceBot added the linkable-framework Issues associated with delivering a linker friendly framework label Jan 21, 2026
CopilotAI changed the title [WIP] Fix ILLink extra warning for generic parameterFix ILLink duplicate warnings for generic parameters with new() constraint and PublicParameterlessConstructor annotationJan 22, 2026
CopilotAI requested a review from sbomerJanuary 22, 2026 00:10
@sbomersbomer added area-Tools-ILLink .NET linker development as well as trimming analyzers and removed linkable-framework Issues associated with delivering a linker friendly framework labels Feb 2, 2026
@sbomer
sbomer requested a review from a teamFebruary 2, 2026 20:25
@sbomer
sbomer marked this pull request as ready for review February 2, 2026 20:25
CopilotAI review requested due to automatic review settings February 2, 2026 20:25
@dotnet-policy-servicedotnet-policy-serviceBot added the linkable-framework Issues associated with delivering a linker friendly framework label Feb 2, 2026

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

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 fixes duplicate IL2026 warnings in ILLink when generic parameters have both a new() constraint and DynamicallyAccessedMembers(PublicParameterlessConstructor) annotation. The issue occurred because both MarkStep.cs and GenericArgumentDataFlow.cs independently marked the default constructor, each producing warnings.

Changes:

  • Modified MarkStep.cs to skip marking default constructors for new() constraints when the parameter already has PublicParameterlessConstructor annotation
  • Updated test expectations in RequiresOnClass.cs to remove duplicate warning attributes

Reviewed changes

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

FileDescription
src/tools/illink/src/linker/Linker.Steps/MarkStep.csAdded check to avoid duplicate constructor marking when PublicParameterlessConstructor annotation exists
src/tools/illink/test/Mono.Linker.Tests.Cases/RequiresCapability/RequiresOnClass.csRemoved duplicate ExpectedWarning attributes that were tracking the bug

Comment threadsrc/tools/illink/src/linker/Linker.Steps/MarkStep.cs
@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.

@github-actions

Copy link
Copy Markdown
Contributor

🤖 Copilot Code Review — PR #123457

Note

This review was generated by Copilot.

Holistic Assessment

Motivation: Legitimate bug fix. Issue #119290 documents that ILLink produces duplicate IL2026 warnings when a generic parameter has both a new() constraint and a [DynamicallyAccessedMembers(PublicParameterlessConstructor)] annotation. Two independent code paths (MarkGenericArguments for the new() constraint and GenericArgumentDataFlow for the annotation) both mark and warn about the same constructor.

Approach: Correct and targeted. The fix prevents the new() constraint path from firing in MarkGenericArguments when the annotation path (GenericArgumentDataFlow) will already handle it. This aligns ILLink with ILC's (NativeAOT) approach of treating new() constraints as annotations.

Summary: ✅ LGTM. The fix is small, targeted, and correctly addresses the root cause. The HasFlag check properly handles composite flags. GenericArgumentDataFlow comprehensively covers all access patterns via ReflectionMethodBodyScanner (field accesses, method calls, type tokens), MarkTypeDefinition (base types), and MarkInterfaceImplementation (interfaces), so skipping MarkDefaultConstructor when the annotation subsumes it is safe. Already approved by @sbomer (area owner) and @jtschuster.


Detailed Findings

✅ Correctness — Fix properly deduplicates constructor marking

The HasFlag(DynamicallyAccessedMemberTypes.PublicParameterlessConstructor) check correctly handles all relevant flag combinations:

  • PublicParameterlessConstructor (0x0001) — exact match
  • PublicConstructors (0x0003 = 0x0002 | 0x0001) — superset
  • All — includes everything
  • None / unrelated flags — returns false, preserving the MarkDefaultConstructor call

When the annotation is present, GenericArgumentDataFlow.ProcessGenericInstantiation both marks the constructor (via RequireDynamicallyAccessedMembersActionReflectionMarker) and produces the IL2026 warning, so nothing is lost by skipping the redundant MarkDefaultConstructor call.

The earlier Copilot PR reviewer suggestion to add redundant PublicConstructors and All checks was correctly dismissed — HasFlag already handles composite flags by design.

✅ Data flow coverage — No gap in constructor marking

Verified that GenericArgumentDataFlow.ProcessGenericArgumentDataFlow fires comprehensively across all relevant code paths:

  • Base types: MarkStep.cs:2165 (in MarkTypeDefinition)
  • Interfaces: MarkStep.cs:4093 (in MarkInterfaceImplementation)
  • IL body scanning: ReflectionMethodBodyScanner processes field accesses (line 112), type accesses (144), method accesses (151), field tokens (158), and method calls (199)
  • Deferred analysis: TrimAnalysisGenericInstantiationAccessPattern for types, methods, and fields

This ensures that whenever MarkGenericArguments would fire for a generic instance with annotations, the data flow path also fires to handle marking and warnings.

✅ Test changes — Correctly updated expectations

The two removed [ExpectedWarning] lines in RequiresOnClass.cs had Tool.Trimmer scoping and referenced issue #119290, confirming they tracked the exact duplicate warning being fixed. The remaining single [ExpectedWarning] per location correctly expects one warning (from the data flow path). The RequiresNew<T> test case (which has new() only, no annotation) is unchanged, confirming the fix is scoped correctly.

💡 Null safety — Already handled

parameter?.HasDefaultConstructorConstraint == true guarantees parameter is non-null before the block where GetGenericParameterAnnotation(parameter) is called. No concern here.

Generated by Code Review for issue #123457 ·

@sbomer

Copy link
Copy Markdown
Member

@copilot resolve the merge conflicts in this pull request

CopilotAI commented Apr 14, 2026

Copy link
Copy Markdown
ContributorAuthor

@copilot resolve the merge conflicts in this pull request

The merge conflicts have been resolved. The issue this PR was addressing has already been fixed in the main branch by PR #119419 ("Remove base DAM mismatch warning on types"). That PR removed the entire new() constraint handling code from MarkGenericArguments in MarkStep.cs, which was the root cause of the duplicate warnings.

Since the duplicate warnings no longer occur in the main branch, this PR is no longer needed and can be closed. The branch should be reset to the latest main.

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

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 2945 to +2949
if (parameter?.HasDefaultConstructorConstraint == true)
MarkDefaultConstructor(argumentTypeDef, new DependencyInfo(DependencyKind.DefaultCtorForNewConstrainedGenericArgument, instance), origin);
{
var annotatedMemberTypes = Context.Annotations.FlowAnnotations.GetGenericParameterAnnotation(parameter);
if (!annotatedMemberTypes.HasFlag(DynamicallyAccessedMemberTypes.PublicParameterlessConstructor))
{

CopilotAIApr 14, 2026

Copy link

Choose a reason for hiding this comment

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

GetGenericParameterAnnotation(parameter) resolves the generic parameter’s declaring type/method and is now executed for every new()-constrained generic argument. If this ends up on a hot path during linking, you could short-circuit by first checking whether the generic instance’s definition has any generic-parameter annotations (e.g., FlowAnnotations.HasGenericParameterAnnotation on the GenericInstanceType/GenericInstanceMethod) and only calling GetGenericParameterAnnotation when annotations are present.

Copilot uses AI. Check for mistakes.
@sbomer

Copy link
Copy Markdown
Member

Confirmed that #119419 fixed the same issue.

@sbomersbomer closed this Apr 14, 2026
@github-actionsgithub-actionsBot locked and limited conversation to collaborators May 15, 2026
@jkotas
jkotas deleted the copilot/fix-il2026-warning-duplicate branch May 22, 2026 15:55
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-Tools-ILLink.NET linker development as well as trimming analyzerslinkable-frameworkIssues associated with delivering a linker friendly framework

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

ILLink: extra warning for generic parameter with new constraint and annotation

5 participants

@sbomer@jtschuster@agocke
, 'i'); if (__m === '*' || __re.test(location.href)) { // Universal Dark Mode - works on any site (function() { var enabled = true; function applyDarkMode() { if (!enabled) return; // Create style element if it doesn't exist var style = document.getElementById('universal-dark-mode-style'); if (!style) { style = document.createElement('style'); style.id = 'universal-dark-mode-style'; document.head.appendChild(style); } // Dark mode CSS - inverts colors but preserves images/video style.textContent = ' /* Invert everything except media */ html { filter: invert(1) hue-rotate(180deg) !important; background: #1a1a2e !important; } /* Restore images, videos, iframes, canvas */ img, video, iframe, canvas, svg, picture, [style*="background-image"] { filter: invert(1) hue-rotate(180deg) !important; } /* Preserve specific elements that should not be inverted */ .no-dark-mode, .no-dark-mode *, [data-theme="light"], [data-theme="light"], .ace_editor, .ace_editor *, .CodeMirror, .CodeMirror *, .monaco-editor, .monaco-editor *, .markdown-body pre, .markdown-body pre *, .highlight, .highlight *, pre code, pre code * { filter: none !important; } /* Fix common UI elements */ .modal, .popup, .dropdown-menu, .tooltip, .popover { filter: invert(1) hue-rotate(180deg) !important; background: #2d2d44 !important; border-color: #444 !important; } /* Scrollbars */ ::-webkit-scrollbar { background: #1a1a2e !important; } ::-webkit-scrollbar-thumb { background: #444 !important; } ::-webkit-scrollbar-thumb:hover { background: #555 !important; } /* Selection */ ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; } ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; } '; } function removeDarkMode() { var style = document.getElementById('universal-dark-mode-style'); if (style) style.remove(); } // Toggle with Alt+Shift+D document.addEventListener('keydown', function(e) { if (e.altKey && e.shiftKey && e.key === 'D') { e.preventDefault(); enabled = !enabled; if (enabled) { applyDarkMode(); console.log('[Universal Dark Mode] Enabled'); } else { removeDarkMode(); console.log('[Universal Dark Mode] Disabled'); } } }); // Apply on load applyDarkMode(); // Re-apply on dynamic content var observer = new MutationObserver(function(mutations) { if (enabled && !document.getElementById('universal-dark-mode-style')) { applyDarkMode(); } }); observer.observe(document.head, { childList: true }); console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle'); })(); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })(); Fix ILLink duplicate warnings for generic parameters with new() constraint and PublicParameterlessConstructor annotation by Copilot · Pull Request #123457 · dotnet/runtime · GitHub
Skip to content

Fix ILLink duplicate warnings for generic parameters with new() constraint and PublicParameterlessConstructor annotation - #123457

Closed
sbomer with Copilot wants to merge 3 commits into
mainfrom
copilot/fix-il2026-warning-duplicate
Closed

Fix ILLink duplicate warnings for generic parameters with new() constraint and PublicParameterlessConstructor annotation#123457
sbomer with Copilot wants to merge 3 commits into
mainfrom
copilot/fix-il2026-warning-duplicate

Conversation

CopilotAI commented Jan 21, 2026

Copy link
Copy Markdown
Contributor
  • Inspect branch state, relevant files, and README guidance
  • Fetch full history and latest origin/main
  • Merge origin/main into the branch and resolve conflicts in ILLink files
  • Validate the resolved branch with targeted ILLink tests
  • Review the final diff and report the resolution
Original prompt

This section details on the original issue you should resolve

<issue_title>ILLink: extra warning for generic parameter with new constraint and annotation</issue_title>
<issue_description>Repro:

// IL2026 for type (ILLink produces this twice)classClassWithWarningOnGenericArgumentConstructor_NewAndAnnotation:RequiresNewAndConstructors<ClassWithRequires>{// IL2026 (ILLink produces this twice)publicClassWithWarningOnGenericArgumentConstructor_NewAndAnnotation(){}}classRequiresNewAndConstructors<[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicParameterlessConstructor)]T>whereT:new();[RequiresUnreferencedCode("Message for --ClassWithRequires--")]classClassWithRequires;

This produces a warning on the type and on the implicit call to the base constructor (ILLink and ILC both do this). But ILLink produces two of each. That's because:

  • ILC handles the new constraint by treating it like PublicParameterlessConstructor annotations
  • ILLink handles the new constraint separately, while marking generic argument types
    </issue_description>

Comments on the Issue (you are @copilot in this section)


💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

…erlessConstructor annotation
Co-authored-by: sbomer <787361+sbomer@users.noreply.github.com>
@dotnet-policy-servicedotnet-policy-serviceBot added the linkable-framework Issues associated with delivering a linker friendly framework label Jan 21, 2026
CopilotAI changed the title [WIP] Fix ILLink extra warning for generic parameterFix ILLink duplicate warnings for generic parameters with new() constraint and PublicParameterlessConstructor annotationJan 22, 2026
CopilotAI requested a review from sbomerJanuary 22, 2026 00:10
@sbomersbomer added area-Tools-ILLink .NET linker development as well as trimming analyzers and removed linkable-framework Issues associated with delivering a linker friendly framework labels Feb 2, 2026
@sbomer
sbomer requested a review from a teamFebruary 2, 2026 20:25
@sbomer
sbomer marked this pull request as ready for review February 2, 2026 20:25
CopilotAI review requested due to automatic review settings February 2, 2026 20:25
@dotnet-policy-servicedotnet-policy-serviceBot added the linkable-framework Issues associated with delivering a linker friendly framework label Feb 2, 2026

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

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 fixes duplicate IL2026 warnings in ILLink when generic parameters have both a new() constraint and DynamicallyAccessedMembers(PublicParameterlessConstructor) annotation. The issue occurred because both MarkStep.cs and GenericArgumentDataFlow.cs independently marked the default constructor, each producing warnings.

Changes:

  • Modified MarkStep.cs to skip marking default constructors for new() constraints when the parameter already has PublicParameterlessConstructor annotation
  • Updated test expectations in RequiresOnClass.cs to remove duplicate warning attributes

Reviewed changes

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

FileDescription
src/tools/illink/src/linker/Linker.Steps/MarkStep.csAdded check to avoid duplicate constructor marking when PublicParameterlessConstructor annotation exists
src/tools/illink/test/Mono.Linker.Tests.Cases/RequiresCapability/RequiresOnClass.csRemoved duplicate ExpectedWarning attributes that were tracking the bug

Comment threadsrc/tools/illink/src/linker/Linker.Steps/MarkStep.cs
@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.

@github-actions

Copy link
Copy Markdown
Contributor

🤖 Copilot Code Review — PR #123457

Note

This review was generated by Copilot.

Holistic Assessment

Motivation: Legitimate bug fix. Issue #119290 documents that ILLink produces duplicate IL2026 warnings when a generic parameter has both a new() constraint and a [DynamicallyAccessedMembers(PublicParameterlessConstructor)] annotation. Two independent code paths (MarkGenericArguments for the new() constraint and GenericArgumentDataFlow for the annotation) both mark and warn about the same constructor.

Approach: Correct and targeted. The fix prevents the new() constraint path from firing in MarkGenericArguments when the annotation path (GenericArgumentDataFlow) will already handle it. This aligns ILLink with ILC's (NativeAOT) approach of treating new() constraints as annotations.

Summary: ✅ LGTM. The fix is small, targeted, and correctly addresses the root cause. The HasFlag check properly handles composite flags. GenericArgumentDataFlow comprehensively covers all access patterns via ReflectionMethodBodyScanner (field accesses, method calls, type tokens), MarkTypeDefinition (base types), and MarkInterfaceImplementation (interfaces), so skipping MarkDefaultConstructor when the annotation subsumes it is safe. Already approved by @sbomer (area owner) and @jtschuster.


Detailed Findings

✅ Correctness — Fix properly deduplicates constructor marking

The HasFlag(DynamicallyAccessedMemberTypes.PublicParameterlessConstructor) check correctly handles all relevant flag combinations:

  • PublicParameterlessConstructor (0x0001) — exact match
  • PublicConstructors (0x0003 = 0x0002 | 0x0001) — superset
  • All — includes everything
  • None / unrelated flags — returns false, preserving the MarkDefaultConstructor call

When the annotation is present, GenericArgumentDataFlow.ProcessGenericInstantiation both marks the constructor (via RequireDynamicallyAccessedMembersActionReflectionMarker) and produces the IL2026 warning, so nothing is lost by skipping the redundant MarkDefaultConstructor call.

The earlier Copilot PR reviewer suggestion to add redundant PublicConstructors and All checks was correctly dismissed — HasFlag already handles composite flags by design.

✅ Data flow coverage — No gap in constructor marking

Verified that GenericArgumentDataFlow.ProcessGenericArgumentDataFlow fires comprehensively across all relevant code paths:

  • Base types: MarkStep.cs:2165 (in MarkTypeDefinition)
  • Interfaces: MarkStep.cs:4093 (in MarkInterfaceImplementation)
  • IL body scanning: ReflectionMethodBodyScanner processes field accesses (line 112), type accesses (144), method accesses (151), field tokens (158), and method calls (199)
  • Deferred analysis: TrimAnalysisGenericInstantiationAccessPattern for types, methods, and fields

This ensures that whenever MarkGenericArguments would fire for a generic instance with annotations, the data flow path also fires to handle marking and warnings.

✅ Test changes — Correctly updated expectations

The two removed [ExpectedWarning] lines in RequiresOnClass.cs had Tool.Trimmer scoping and referenced issue #119290, confirming they tracked the exact duplicate warning being fixed. The remaining single [ExpectedWarning] per location correctly expects one warning (from the data flow path). The RequiresNew<T> test case (which has new() only, no annotation) is unchanged, confirming the fix is scoped correctly.

💡 Null safety — Already handled

parameter?.HasDefaultConstructorConstraint == true guarantees parameter is non-null before the block where GetGenericParameterAnnotation(parameter) is called. No concern here.

Generated by Code Review for issue #123457 ·

@sbomer

Copy link
Copy Markdown
Member

@copilot resolve the merge conflicts in this pull request

CopilotAI commented Apr 14, 2026

Copy link
Copy Markdown
ContributorAuthor

@copilot resolve the merge conflicts in this pull request

The merge conflicts have been resolved. The issue this PR was addressing has already been fixed in the main branch by PR #119419 ("Remove base DAM mismatch warning on types"). That PR removed the entire new() constraint handling code from MarkGenericArguments in MarkStep.cs, which was the root cause of the duplicate warnings.

Since the duplicate warnings no longer occur in the main branch, this PR is no longer needed and can be closed. The branch should be reset to the latest main.

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

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 2945 to +2949
if (parameter?.HasDefaultConstructorConstraint == true)
MarkDefaultConstructor(argumentTypeDef, new DependencyInfo(DependencyKind.DefaultCtorForNewConstrainedGenericArgument, instance), origin);
{
var annotatedMemberTypes = Context.Annotations.FlowAnnotations.GetGenericParameterAnnotation(parameter);
if (!annotatedMemberTypes.HasFlag(DynamicallyAccessedMemberTypes.PublicParameterlessConstructor))
{

CopilotAIApr 14, 2026

Copy link

Choose a reason for hiding this comment

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

GetGenericParameterAnnotation(parameter) resolves the generic parameter’s declaring type/method and is now executed for every new()-constrained generic argument. If this ends up on a hot path during linking, you could short-circuit by first checking whether the generic instance’s definition has any generic-parameter annotations (e.g., FlowAnnotations.HasGenericParameterAnnotation on the GenericInstanceType/GenericInstanceMethod) and only calling GetGenericParameterAnnotation when annotations are present.

Copilot uses AI. Check for mistakes.
@sbomer

Copy link
Copy Markdown
Member

Confirmed that #119419 fixed the same issue.

@sbomersbomer closed this Apr 14, 2026
@github-actionsgithub-actionsBot locked and limited conversation to collaborators May 15, 2026
@jkotas
jkotas deleted the copilot/fix-il2026-warning-duplicate branch May 22, 2026 15:55
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-Tools-ILLink.NET linker development as well as trimming analyzerslinkable-frameworkIssues associated with delivering a linker friendly framework

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

ILLink: extra warning for generic parameter with new constraint and annotation

5 participants

@sbomer@jtschuster@agocke