Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Add regression test for EnC nullable attribute assertion during unmodified type completion #80657

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Draft
wants to merge 3 commits into
base: main
Choose a base branch
Loading
from
Draft

Add regression test for EnC nullable attribute assertion during unmodified type completion #80657

wants to merge 3 commits into from

Conversation

Copilot
Copy link
Contributor

@Copilot Copilot AI commented Oct 11, 2025 •
edited
Loading

Fixes #79320

Problem

During Edit and Continue (EnC) compilation, when a modified method calls a method in an unmodified type, the compiler needs to complete the unmodified type (e.g., to check if the called method is virtual). During type completion, if the type has members with nullable annotations, the compiler attempts to ensure NullableContextAttribute exists by calling EnsureEmbeddableAttributeExists with modifyCompilation: true.

This could trigger an assertion failure:

Debug.Assert(!modifyCompilation || !_needsGeneratedAttributes_IsFrozen);

The assertion fires because the _needsGeneratedAttributes_IsFrozen flag may already be set when completing unmodified types during EnC delta compilation.

Solution

The code already has the correct fix through special handling in PEModuleBuilder.GetNeedsGeneratedAttributesInternal():

return (EmbeddableAttributes)_needsGeneratedAttributes | 
       Compilation.GetNeedsGeneratedAttributes(freezeState: this is not PEDeltaAssemblyBuilder);

When compiling an EnC delta (PEDeltaAssemblyBuilder), we pass freezeState: false, which prevents the Compilation's _needsGeneratedAttributes_IsFrozen flag from being set. This allows late completion of unmodified types without triggering the assertion.

Changes

This PR adds a regression test NullableContextAttribute_UnmodifiedTypeCompletion that:

  1. Creates a baseline compilation without nullable reference types
  2. Emits an EnC delta that:
    • Updates a method to call an unmodified method (triggering completion of the unmodified type)
    • Adds a new method with nullable annotations (requiring NullableContextAttribute generation)
  3. Verifies that nullable attributes are correctly synthesized without assertion failures
  4. Includes comprehensive documentation explaining the scenario and the existing fix mechanism

Testing

  • New test passes on both .NET 9.0 and .NET Framework 4.7.2
  • All 214 existing EditAndContinueTests continue to pass with no regressions

Follow-up

As mentioned in the issue, a benchmark test should be added to the dotnet/performance repository to measure EnC performance and ensure the performance gains from #71254 are maintained. This will be tracked separately as it requires work in a different repository.

Original prompt

This section details on the original issue you should resolve

<issue_title>Assertion fails in CSharpCompilation.EnsureEmbeddableAttributeExists when compiling EnC delta</issue_title>
<issue_description>Repro:
see #79320

Assertion:
Debug.Assert(!modifyCompilation || !_needsGeneratedAttributes_IsFrozen);

Values:

Image

Call stack:

>	Microsoft.CodeAnalysis.CSharp.dll!Microsoft.CodeAnalysis.CSharp.CSharpCompilation.EnsureEmbeddableAttributeExists(Microsoft.CodeAnalysis.CSharp.EmbeddableAttributes attribute, Microsoft.CodeAnalysis.CSharp.BindingDiagnosticBag diagnostics, Microsoft.CodeAnalysis.Location location, bool modifyCompilation) Line 516	C#
 	Microsoft.CodeAnalysis.CSharp.dll!Microsoft.CodeAnalysis.CSharp.CSharpCompilation.EnsureNullableContextAttributeExists(Microsoft.CodeAnalysis.CSharp.BindingDiagnosticBag diagnostics, Microsoft.CodeAnalysis.Location location, bool modifyCompilation) Line 562	C#
 	Microsoft.CodeAnalysis.CSharp.dll!Microsoft.CodeAnalysis.CSharp.Symbols.SourceMemberMethodSymbol.AfterAddingTypeMembersChecks(Microsoft.CodeAnalysis.CSharp.ConversionsBase conversions, Microsoft.CodeAnalysis.CSharp.BindingDiagnosticBag diagnostics) Line 992	C#
 	Microsoft.CodeAnalysis.CSharp.dll!Microsoft.CodeAnalysis.CSharp.Symbols.SourceOrdinaryMethodOrUserDefinedOperatorSymbol.AfterAddingTypeMembersChecks(Microsoft.CodeAnalysis.CSharp.ConversionsBase conversions, Microsoft.CodeAnalysis.CSharp.BindingDiagnosticBag diagnostics) Line 222	C#
 	Microsoft.CodeAnalysis.CSharp.dll!Microsoft.CodeAnalysis.CSharp.Symbols.SourceOrdinaryMethodSymbol.AfterAddingTypeMembersChecks(Microsoft.CodeAnalysis.CSharp.ConversionsBase conversions, Microsoft.CodeAnalysis.CSharp.BindingDiagnosticBag diagnostics) Line 713	C#
 	Microsoft.CodeAnalysis.CSharp.dll!Microsoft.CodeAnalysis.CSharp.Symbols.SourceMemberContainerTypeSymbol.CheckSpecialMemberErrors(Microsoft.CodeAnalysis.CSharp.BindingDiagnosticBag diagnostics) Line 2534	C#
 	Microsoft.CodeAnalysis.CSharp.dll!Microsoft.CodeAnalysis.CSharp.Symbols.SourceMemberContainerTypeSymbol.AfterMembersChecks(Microsoft.CodeAnalysis.CSharp.BindingDiagnosticBag diagnostics) Line 1861	C#
 	Microsoft.CodeAnalysis.CSharp.dll!Microsoft.CodeAnalysis.CSharp.Symbols.SourceMemberContainerTypeSymbol.ForceComplete(Microsoft.CodeAnalysis.SourceLocation locationOpt, System.Predicate<Microsoft.CodeAnalysis.CSharp.Symbol> filter, System.Threading.CancellationToken cancellationToken) Line 634	C#
 	Microsoft.CodeAnalysis.CSharp.dll!Microsoft.CodeAnalysis.CSharp.Symbols.SourceMemberMethodSymbol.IsMetadataVirtual(Microsoft.CodeAnalysis.CSharp.Symbols.MethodSymbol.IsMetadataVirtualOption option) Line 594	C#
 	Microsoft.CodeAnalysis.CSharp.dll!Microsoft.CodeAnalysis.CSharp.Symbols.MethodToClassRewriter.VisitCall(Microsoft.CodeAnalysis.CSharp.BoundCall node) Line 105	C#
 	Microsoft.CodeAnalysis.CSharp.dll!Microsoft.CodeAnalysis.CSharp.ClosureConversion.VisitCall(Microsoft.CodeAnalysis.CSharp.BoundCall node) Line 1108	C#
 	Microsoft.CodeAnalysis.CSharp.dll!Microsoft.CodeAnalysis.CSharp.BoundCall.Accept(Microsoft.CodeAnalysis.CSharp.BoundTreeVisitor visitor) Line 6186	C#
 	Microsoft.CodeAnalysis.CSharp.dll!Microsoft.CodeAnalysis.CSharp.BoundTreeRewriterWithStackGuard.VisitExpressionOrPatternWithoutStackGuard(Microsoft.CodeAnalysis.CSharp.BoundNode node) Line 233	C#
 	Microsoft.CodeAnalysis.CSharp.dll!Microsoft.CodeAnalysis.CSharp.BoundTreeVisitor.VisitExpressionOrPatternWithStackGuard(Microsoft.CodeAnalysis.CSharp.BoundNode node) Line 243	C#
 	Microsoft.CodeAnalysis.CSharp.dll!Microsoft.CodeAnalysis.CSharp.BoundTreeVisitor.VisitExpressionOrPatternWithStackGuard(ref int recursionDepth, Microsoft.CodeAnalysis.CSharp.BoundNode node) Line 217	C#
 	Microsoft.CodeAnalysis.CSharp.dll!Microsoft.CodeAnalysis.CSharp.BoundTreeRewriterWithStackGuard.Visit(Microsoft.CodeAnalysis.CSharp.BoundNode node) Line 220	C#
 	Microsoft.CodeAnalysis.CSharp.dll!Microsoft.CodeAnalysis.CSharp.BoundTreeRewriter.VisitExpressionStatement(Microsoft.CodeAnalysis.CSharp.BoundExpressionStatement node) Line 11433	C#
 	Microsoft.CodeAnalysis.CSharp.dll!Microsoft.CodeAnalysis.CSharp.BoundExpressionStatement.Accept(Microsoft.CodeAnalysis.CSharp.BoundTreeVisitor visitor) Line 3680	C#
 	Microsoft.CodeAnalysis.CSharp.dll!Microsoft.CodeAnalysis.CSharp.BoundTreeRewriterWithStackGuard.Visit(Microsoft.CodeAnalysis.CSharp.BoundNode node) Line 223	C#
 	Microsoft.CodeAnalysis.CSharp.dll!Microsoft.CodeAnalysis.CSharp.BoundTreeRewriter.VisitSequencePoint(Microsoft.CodeAnalysis.CSharp.BoundSequencePoint node) Line 11357	C#
 	Microsoft.CodeAnalysis.CSharp.dll!Microsoft.CodeAnalysis.CSharp.BoundSequencePoint.Accept(Microsoft.CodeAnalysis.CSharp.BoundTreeVisitor visitor) Line 3156	C#
 	Microsoft.CodeAnalysis.CSharp.dll!Microsoft.CodeAnalysis.CSharp.BoundTreeRewriterWithStackGuard.Visit(M...

</details>

Fixes dotnet/roslyn#80405

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

💬 Share your feedback on Copilot coding agent for the chance to win a $200 gift card! Click [here](https://survey3.medallia.com/?EAHeSx-AP01bZqG0Ld9QLQ) to start the survey.

@Copilot Copilot AI assigned Copilot and tmat Oct 11, 2025
Copilot AI and others added 2 commits October 11, 2025 17:27
@Copilot @tmat
Add regression test for EnC nullable attribute assertion
c801761 
Co-authored-by: tmat <41759+tmat@users.noreply.github.com>
@Copilot @tmat
Improve documentation for EnC nullable attribute test
c092598 
Co-authored-by: tmat <41759+tmat@users.noreply.github.com>
@Copilot Copilot AI changed the title [WIP] Fix assertion failure in CSharpCompilation.EnsureEmbeddableAttributeExists Add regression test for EnC nullable attribute assertion during unmodified type completion Oct 11, 2025
@Copilot Copilot AI requested a review from tmat October 11, 2025 17:33
Copilot finished work on behalf of tmat October 11, 2025 17:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tmat tmat Awaiting requested review from tmat

At least 1 approving review is required to merge this pull request.

Assignees

@tmat tmat

Copilot code review Copilot

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

HR fails when removing a property with an initializer from a partial class

2 participants

@Copilot @tmat
Morty Proxy This is a proxified and sanitized view of the page, visit original site.