Skip to content
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.

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

EnC: Defer lambda rude edit reporting to runtime #70418

Merged
merged 5 commits into from
Dec 7, 2023
Merged

Conversation

tmat
Copy link
Member

@tmat tmat commented Oct 17, 2023

Defer the decision of whether or not to allow certain kinds of EnC changes to take effect to the runtime. Apply this approach to lambdas. The IDE currently blocks any changes to lambdas that would alter the shape of the closure (e.g. static lambda changed to a lambda capturing a variable). If, however, the old lambda is not invoked by the application (i.e. the delegate pointing to it is not used after the EnC update) there is no need to block the change. The new lambda will be constructed with the updated closure and can be executed without issues. The IDE does not know for any specific lambda whether or not the application will use the delegate after the edit. However, if we update the old version of the lambda body to throw an exception with rude edit message we can allow the change to go through. If the old lambda is executed it will throw and the debugger will display the rude edit message. As a follow up we will also consider emitting a specific exception type that gets a special treatment from the debugger -- see https://devdiv.visualstudio.com/DevDiv/_workitems/edit/1903644.

Fixes #67841, #68708
Implements #70450, #54672

@tmat
Copy link
Member Author

tmat commented Nov 30, 2023

@davidwengier @cston @jjonescz PTAL

Copy link
Contributor

@davidwengier davidwengier left a comment

Choose a reason for hiding this comment

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

EnC specific stuff looks good to me.

@cston
Copy link
Member

cston commented Dec 6, 2023

                    // A new display class and method is generated for lambda that captures x.

Are we capturing x in the second generation?


Refers to: src/Compilers/CSharp/Test/Emit/Emit/EditAndContinue/EditAndContinueClosureTests.cs:6602 in 187755b. [](commit_id = 187755b, deletion_comment = False)

@cston
Copy link
Member

cston commented Dec 6, 2023

    public void CapureOrdering()

Typo


Refers to: src/Compilers/CSharp/Test/Emit/Emit/EditAndContinue/EditAndContinueClosureTests.cs:8872 in 187755b. [](commit_id = 187755b, deletion_comment = False)

Copy link
Member

@cston cston left a comment

Choose a reason for hiding this comment

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

Reviewed Compiler and EE changes; minor comments only.

@tmat
Copy link
Member Author

tmat commented Dec 6, 2023

Had to force push :( Changes are in the last commit.

@tmat tmat merged commit 63f6ee3 into dotnet:main Dec 7, 2023
27 checks passed
@tmat tmat deleted the Lambdas branch December 7, 2023 20:16
@ghost ghost added this to the Next milestone Dec 7, 2023
@tmat tmat modified the milestones: Next, 17.9 P3 Dec 7, 2023
@tmat tmat mentioned this pull request Sep 14, 2024
12 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Compilers Interactive-EnC untriaged Issues and PRs which have not yet been triaged by a lead
Projects
None yet
Development

Successfully merging this pull request may close these issues.

EnC Razor scenario: allow adding new lambda closures
4 participants