Skip to content

PEDeltaAssemblyBuilder: Don't freeze _needsGeneratedAttributes compilation state.#80600

Merged
AlekseyTs merged 1 commit intodotnet:mainfrom
AlekseyTs:Issue80405
Oct 10, 2025
Merged

PEDeltaAssemblyBuilder: Don't freeze _needsGeneratedAttributes compilation state.#80600
AlekseyTs merged 1 commit intodotnet:mainfrom
AlekseyTs:Issue80405

Conversation

@AlekseyTs
Copy link
Contributor

Related to #80405.

@AlekseyTs AlekseyTs requested review from 333fred and tmat October 9, 2025 01:58
@AlekseyTs AlekseyTs requested a review from a team as a code owner October 9, 2025 01:58
private EmbeddableAttributes GetNeedsGeneratedAttributesInternal()
{
return (EmbeddableAttributes)_needsGeneratedAttributes | Compilation.GetNeedsGeneratedAttributes();
return (EmbeddableAttributes)_needsGeneratedAttributes | Compilation.GetNeedsGeneratedAttributes(freezeState: this is not PEDeltaAssemblyBuilder);
Copy link
Member

Choose a reason for hiding this comment

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

Why do we have the freezing in the first place? Aren't we opening other bugs if we skip it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Why do we have the freezing in the first place? Aren't we opening other bugs if we skip it?

It is true that we are potentially opening ourselves to other negative consequences. But, apparently, Edit-And-Continue "likes" to live dangerously and is willing to accept a potential instability in exchange for immediate performance gains.

@AlekseyTs
Copy link
Contributor Author

@tmat, @333fred Please review

@tmat
Copy link
Member

tmat commented Oct 9, 2025

Let's add a test that hits this. I'll be able to look into it next week.

@AlekseyTs
Copy link
Contributor Author

Let's add a test that hits this. I'll be able to look into it next week.

Per offline discussion, I am going to merge this PR without the test. The original issue will remain open and will track the follow up work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants