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] assorted hot reload fixes for cumulative property updates #85351

Merged
merged 4 commits into from
Apr 25, 2023

Conversation

lambdageek
Copy link
Member

@lambdageek lambdageek commented Apr 25, 2023

  • is_addition is already defined as token_index-1 >= delta_info->count[token_table].prev_gen_rows so:

    1. the assertion as written is wrong for cumulative updates that add new properties or events
    2. the assertion would be trivially true if it was updated to use delta_info->count[token_table].prev_gen_rows.

    Fixes the ability to cumulatively add new properties

  • Assert that we don't see generic instances in hot_reload_get_property and hot_reload_get_event.
    They only get called when we're looking at a definition (class def or gtd) and want to find a property using a token.

  • Allow overwriting getter/setter (and event add/remove/raise) methods - if a setter is updated in a cumulative update, the previously added prop (or event) will have its semantic method overwritten with a new one.

  • Actually set the generation on a class update info when adding members. This will allow the generic instance code to trigger a recomputation and see the second (or later) generation updated info on instances.

`is_addition` is already defined as `token_index-1 >=
delta_info->count[token_table].prev_gen_rows` so:

1. the assertion as written is wrong for cumulative updates that add
new properties or events
2. the assertion would be trivially true if it was updated to use
`delta_info->count[token_table].prev_gen_rows`.

Fixes the ability to cumulatively add new properties
@dotnet-issue-labeler dotnet-issue-labeler bot added the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label Apr 25, 2023
@ghost ghost assigned lambdageek Apr 25, 2023
@lambdageek lambdageek added area-EnC-mono Hot Reload for WebAssembly, iOS/Android, etc and removed needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners labels Apr 25, 2023
Give recompute_ginst_update_info something to work with.
@lambdageek
Copy link
Member Author

There are additional changes. And also an updated regression test

@lambdageek lambdageek changed the title [hot_reload] remove unneeded assertion [hot_reload] assorted hot reload fixes for cumulative property updates Apr 25, 2023
@lewing
Copy link
Member

lewing commented Apr 25, 2023

@carlossanlop we'd like this in the snap

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.

4 participants