Skip to content

C#: Use separate newtype branch for AssignableDefinitionNode - #15694

Merged
hvitved merged 2 commits into
github:mainfrom
hvitved:csharp/assignable-definition-node
Feb 23, 2024
Merged

C#: Use separate newtype branch for AssignableDefinitionNode#15694
hvitved merged 2 commits into
github:mainfrom
hvitved:csharp/assignable-definition-node

Conversation

@hvitved

@hvitvedhvitved commented Feb 22, 2024

Copy link
Copy Markdown
Contributor

In preparation for #15665, this PR adjusts AssignableDefinitionNode from being backed up by an SSA definition node to being backed up by a new newtype injector.

The motivation is that SSA definitions only exist for writes that are live, and on #15665 we much simplify which captured variables to construct SSA forms for, so flow through primary constructors cannot rely on the existence of SSA definitions for the captured parameters.

Like #12566 for Ruby, this PR also adds all assignments (i.e., AssignableDefinitionNodes) in data flow paths.

@hvitved
hvitvedforce-pushed the csharp/assignable-definition-node branch 2 times, most recently from e3318d3 to 2c8220eCompareFebruary 22, 2024 16:10
@hvitved
hvitvedforce-pushed the csharp/assignable-definition-node branch from 2c8220e to 303a2bbCompareFebruary 22, 2024 20:05
@hvitvedhvitved added the no-change-note-required This PR does not need a change note label Feb 23, 2024
@hvitved
hvitved marked this pull request as ready for review February 23, 2024 07:44
@hvitved
hvitved requested a review from a team as a code ownerFebruary 23, 2024 07:44

@michaelnebelmichaelnebel 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.

Looks good to me!

@hvitved
hvitved merged commit d8645cc into github:mainFeb 23, 2024
@hvitved
hvitved deleted the csharp/assignable-definition-node branch February 23, 2024 09:45
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

C#no-change-note-requiredThis PR does not need a change note

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@hvitved@michaelnebel