Skip to content

[hot_reload] assorted hot reload fixes for cumulative property updates#85351

Merged
carlossanlop merged 4 commits intodotnet:mainfrom
lambdageek:fix-create-new-props-cumulative
Apr 25, 2023
Merged

[hot_reload] assorted hot reload fixes for cumulative property updates#85351
carlossanlop merged 4 commits intodotnet:mainfrom
lambdageek:fix-create-new-props-cumulative

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
@lambdageek lambdageek requested a review from marek-safar as a code owner April 25, 2023 19:36
@ghost ghost 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 requested a review from fanyang-mono April 25, 2023 19:36
@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