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

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

Open
tmat opened this issue Sep 6, 2022 · 8 comments
Open

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

tmat opened this issue Sep 6, 2022 · 8 comments

Comments

@tmat
Copy link
Member

tmat commented Sep 6, 2022

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 ghost added the untriaged New issue has not been triaged by the area owner label Sep 6, 2022
@ghost
Copy link

ghost commented Sep 6, 2022

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 tmat added area-EnC-CLR area-EnC-mono Hot Reload for WebAssembly, iOS/Android, etc labels Sep 6, 2022
@tmat
Copy link
Member Author

tmat commented Sep 6, 2022

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

@tmat tmat changed the title EnC: Update metadata when entities are deleted in an EnC edit Update metadata when entities are deleted in an EnC edit Sep 6, 2022
@tmat tmat changed the title Update metadata when entities are deleted in an EnC edit Update metadata when entities are deleted by an EnC edit Sep 6, 2022
@tommcdon tommcdon added this to the 8.0.0 milestone Sep 6, 2022
@ghost ghost removed the untriaged New issue has not been triaged by the area owner label Sep 6, 2022
@tommcdon
Copy link
Member

tommcdon commented Sep 6, 2022

cc @mikelle-rogers

@mikelle-rogers mikelle-rogers self-assigned this Sep 6, 2022
@lambdageek
Copy link
Member

lambdageek commented Sep 6, 2022

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
Copy link
Contributor

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.

@teo-tsirpanis teo-tsirpanis removed area-System.Reflection area-EnC-mono Hot Reload for WebAssembly, iOS/Android, etc labels Sep 7, 2022
@lambdageek lambdageek added the area-EnC-mono Hot Reload for WebAssembly, iOS/Android, etc label Sep 7, 2022
@ghost
Copy link

ghost commented Sep 7, 2022

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
Copy link
Member

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

@GSPP
Copy link

GSPP commented Oct 7, 2022

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants