Skip to content

Conversation

@tmat
Copy link
Member

@tmat tmat commented Aug 4, 2025

Prefixes the name of deleted methods, properties and events with deleted_ and sets their SpecialName flag to true.
This will allow Reflection to filter out deleted members.

Contributes to dotnet/runtime#75154
Implements #69834

UPDATE:
Just realized that renaming the method is not desirable for public methods. If we rename method M to deleted_M any MemberRef token that refers to M from outside of the assembly will fail to resolve when JIT compiles the caller. That will throw MissingMethodException, which is not what we want. We want the deleted method to be called and its body throws HotReloadException.

Instead we emit System.Runtime.CompilerServices.MetadataUpdateDeletedAttribute on the deleted member, if the attribute is defined.

[jcouv:] API proposal for MetadataUpdateDeletedAttribute

@tmat
Copy link
Member Author

tmat commented Aug 5, 2025

@DustinCampbell @phil-allen-msft @dotnet/roslyn-compiler ptal

@tmat tmat changed the title Set name and flags of deleted definitions EnC: Set name and flags of deleted definitions Aug 5, 2025
@tmat tmat marked this pull request as draft August 13, 2025 20:18
@tmat
Copy link
Member Author

tmat commented Aug 13, 2025

Actually, just realized this might cause undesired behavior in some cases. Following up...

@tmat tmat marked this pull request as ready for review August 25, 2025 22:12
@tmat tmat requested a review from a team as a code owner August 25, 2025 22:12
@tmat
Copy link
Member Author

tmat commented Aug 25, 2025

@333fred ptal again. I have reverted the rename and replaced it with emitting an attribute. See description.

@tmat tmat changed the title EnC: Set name and flags of deleted definitions EnC: Mark deleted members with an attribute Aug 25, 2025
@jaredpar
Copy link
Member

@dotnet/roslyn-compiler PTAL

@jcouv jcouv self-assigned this Aug 26, 2025

System_Runtime_CompilerServices_HotReloadException__ctorStringInt32,
System_Runtime_CompilerServices_MetadataUpdateOriginalTypeAttribute__ctor,
System_Runtime_CompilerServices_MetadataUpdateDeletedAttribute__ctor,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are we aiming to add the BCL type in .NET 10 or did we miss the window? At least, we should get the API design proposal approved before .NET 10 ships

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My question was about timing. Talked to Jeff Handley just now and it looks like there's still a window to add APIs in .NET 10 (RC2 code complete late September). Sounds like he's the area owner, so he'll follow-up with Tomas to get this through API review.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Checking with @steveisok

Copy link
Member

@jcouv jcouv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done with review pass (commit 4)

Copy link
Member

@jcouv jcouv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM Thanks (commit 6)

@tmat tmat enabled auto-merge (squash) August 26, 2025 22:40
@tmat tmat disabled auto-merge August 28, 2025 19:47
@tmat tmat merged commit b2da93c into dotnet:main Aug 28, 2025
28 checks passed
@tmat tmat deleted the MemberDelete branch August 28, 2025 19:47
@dotnet-policy-service dotnet-policy-service bot added this to the Next milestone Aug 28, 2025
@akhera99 akhera99 modified the milestones: Next, 18.0 P1, 18.0 P2 Sep 22, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants