search

dotnet / runtime

.NET is a cross-platform runtime for cloud, mobile, desktop, and IoT apps.
https://docs.microsoft.com/dotnet/core/
MIT License
14.93k stars 4.64k forks source link

Update metadata when entities are deleted by an EnC edit #75154

Open tmat opened 2 years ago

tmat commented 2 years ago

Roslyn now allows to delete type members. Deleted method bodies are updated to throw MissingMethodException or to call to the new method (WIP). The deletions are however not reflected in metadata returned by Reflection which may affect app frameworks that depend on Reflection information.

Consider adding a naming convention for names of deleted members that Roslyn can use to mark deleted members and that Reflection skips over when enumerating type members.

ghost commented 2 years ago

Tagging subscribers to this area: @dotnet/area-system-reflection See info in area-owners.md if you want to be subscribed.

Issue Details
Roslyn now allows to delete type members. Deleted method bodies are updated to throw MissingMethodException or to call to the new method (WIP). The deletions are however not reflected in metadata returned by Reflection which may affect Frameworks that depend on Reflection information. Consider adding a naming convention for names of deleted members that Roslyn can use to mark deleted members and that Reflection skips over when enumerating type members.
Author: tmat
Assignees: -
Labels: `area-System.Reflection`
Milestone: -
tmat commented 2 years ago

@lambdageek @tommcdon (FYI: I added area-EnC-CLR label) @davidwengier

tommcdon commented 2 years ago

cc @mikelle-rogers

lambdageek commented 2 years ago

Mono has had this code since time immemorial (but only for fields, not methods, properties, events, etc):

/* a field is ignored if it's named "_Deleted" and it has the specialname and rtspecialname flags set */
#define mono_field_is_deleted(field) (((field)->type->attrs & (FIELD_ATTRIBUTE_SPECIAL_NAME | FIELD_ATTRIBUTE_RT_SPECIAL_NAME)) \
                      && (strcmp (mono_field_get_name (field), "_Deleted") == 0))

Perhaps we could formalize something like this? (on the other hand, this affects more than reflection in Mono, it's also used in the instance layout calculation, for example)

In any case, the ECMA "special name" attribute flags on members might be useful

teo-tsirpanis commented 2 years ago

Can you rename the label to area-EnC-coreclr to match with the others' naming style (and I will keep only this because issues have only one area)? docs/area-owners.md needs to be updated as well.

ghost commented 2 years ago

Tagging subscribers to this area: @tommcdon See info in area-owners.md if you want to be subscribed.

Issue Details
Roslyn now allows to delete type members. Deleted method bodies are updated to throw MissingMethodException or to call to the new method (WIP). The deletions are however not reflected in metadata returned by Reflection which may affect app frameworks that depend on Reflection information. Consider adding a naming convention for names of deleted members that Roslyn can use to mark deleted members and that Reflection skips over when enumerating type members.
Author: tmat
Assignees: mikelle-rogers
Labels: `area-Diagnostics-coreclr`, `feature-request`, `area-EnC-mono`
Milestone: 8.0.0
jeffschwMSFT commented 2 years ago

I deleted area-enc-clr and added the area label area-diagnostics-coreclr

GSPP commented 1 year ago

Throwing MissingMethodException seems dubious because this exception usually means a mismatch between binaries from different compilations/versions. This is going to be misleading.