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

[hot_reload] Allow benign Param table updates #56800

Merged
merged 5 commits into from
Aug 4, 2021

Conversation

lambdageek
Copy link
Member

When updating the body of a lambda, Roslyn emits a Param table update for the parameters of the lambda even if nothing about the parameter changed. This is by design.

Update mono method body replacement to allow these benign updates (and ignore them). As long as the sequence number and parameter name are the same as previously, we allow it.

This enables Mono to apply updates to lambda bodies and to their enclosing methods. This does not allow new lambdas to be added to a method body. This does not allow for method signatures to change, or for parameter renaming.

Note: this PR also bumps hotreload-utils, which picks up changes from Roslyn necessary to generate nice deltas.

Fixes #56574, part of #44806

@lambdageek lambdageek added the area-EnC-mono Hot Reload for WebAssembly, iOS/Android, etc label Aug 3, 2021
@lambdageek lambdageek requested a review from marek-safar as a code owner August 3, 2021 20:15
@lambdageek lambdageek requested review from thaystg and vargaz August 3, 2021 20:16
@lambdageek lambdageek force-pushed the benign-param-updates branch from 386dc5f to 81bf189 Compare August 4, 2021 02:04
@@ -39,6 +39,33 @@ void StaticMethodBodyUpdate()
});
}

[ConditionalFact(typeof(ApplyUpdateUtil), nameof (ApplyUpdateUtil.IsSupported))]
[ActiveIssue("https://github.com/dotnet/runtime/issues/54617", typeof(PlatformDetection), nameof(PlatformDetection.IsBrowser), nameof(PlatformDetection.IsMonoAOT))]
Copy link
Member

Choose a reason for hiding this comment

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

Is this wasm aot specific or should it be disabled on all mono aot?

Copy link
Member Author

Choose a reason for hiding this comment

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

It's not going to work on any Mono AOT configuration - but ApplyUpdateUtil.IsSupported is already ruling out all the others.

@lambdageek lambdageek merged commit 9514716 into dotnet:main Aug 4, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Sep 3, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-EnC-mono Hot Reload for WebAssembly, iOS/Android, etc
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[hot_reload] Mono should ignore Param table row updates
3 participants