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

@dotnet-issue-labeler dotnet-issue-labeler bot added Area-Compilers untriaged Issues and PRs which have not yet been triaged by a lead labels Oct 17, 2023
@tmat tmat changed the title Lambdas EnC: Defer lambda rude edit reporting to runtime Oct 24, 2023
@tmat tmat force-pushed the Lambdas branch 5 times, most recently from cd867cc to 6464328 Compare November 21, 2023 01:40
@tmat tmat force-pushed the Lambdas branch 2 times, most recently from 7f928bb to 66317d7 Compare November 30, 2023 03:48
@tmat tmat marked this pull request as ready for review November 30, 2023 03:49
@tmat tmat requested review from a team as code owners November 30, 2023 03:49
@tmat
Copy link
Member Author

tmat commented Nov 30, 2023

@davidwengier @cston @jjonescz PTAL

Copy link
Member

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