[mono] Allow CustomAttribute table modifications if Parent stays unchanged #55097
Comments
dotnet-issue-labeler
bot
added
the
untriaged
This was referenced Jul 6, 2021
ghost
added
the
in-pr
lambdageek
added a commit
that referenced
this issue
Jul 12, 2021
…es (#55445) This fixes #55097 - which allows us to support C# nullability analysis once again in hot reload deltas. Specifically we allow EnC deltas to include modifications of existing rows in the CustomAttribute table as long as the Parent and Type columns stay the same (that is: a custom attribute modification that still applies to the same element - and uses the same custom attribute constructor, but may have a different value). To support this, we add a new BaselineInfo:any_modified_rows array that keeps track for each table whether any rows have been modified (as opposed to added) by any EnC delta. When the runtime calls mono_metadata_decode_row, if there have been any deltas that modified a row in the requested table, we call hot_reload_effective_table_slow which find the latest delta that modified that row. If there have only been additions, we stop at the first delta that added the row we're looking for, if there are modifications, we look through all the deltas and return the latest one. * [hot_reload] Add test for updating custom attribute ctor values Just changing the arguments of a custom attribute constructor should generate an update to the CustomAttributes table with the same parent and .ctor. That kind of change should be allowed by Mono and CoreCLR * [hot_reload] Allow custom attribute row modifications if Parent and Type unchanged. Allows updates to the constructor arguments (or property values) * [hot_reload] Implement table lookup of modified rows Add a bool array on the base image to keep track of whether each table had any row modifications (as opposed to row additions) in any generation. If there was a modification, take the slow path in mono_image_effective_table even if the index we're looking at is in the base image. Update hot_reload_effective_table_slow so that if there was a modified row, we look at all the deltas to see if there's an even later update to that row. (When there are only additions, keep same behavior as before - only look as far as the generation that added the row we wanted to find). Also refine the assertion in hot_reload_relative_delta_index to properly account for EnCMap entries that correspond to modifications - in that case we might stop searching a metadata delta before we hit the end of the table if the EnCmap entries start pointing to rows that are past the one we wanted to look up. * Update the CustomAttributeUpdates test to check attribute value Check that we get the updated custom attribute string property value. * Re-enable nullability for hot reload tests Mono can now deal with the custom attributes modifications that Roslyn emits
ghost
removed
the
in-pr
ghost
locked as resolved and limited conversation to collaborators
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Allow nullability analysis of modified method bodies.
Roslyn emits nullability attributes by emitting an update to a CustomAttribute table row when a method body is updated.
In most(all?) cases the Constructor stays the same too. The value might or might not change, but we don't care.
This is distinct from supporting full custom attribute changes. In this case Roslyn doesn't consider a CA row update as a rude edit even with just Baseline capabilities.
(Previously Roslyn used to emit new CA rows when a method body was updated. Mono ignored it because we tacitly allow through many row additions to most tables. Modifications we scrutinize.)
After this is fixed the ApplyUpdateTest and the hot reload functional tests can be updated to re-enable Nullability analysis for the modifiable assembly projects.
Contributes to #44806
The text was updated successfully, but these errors were encountered: