-
Notifications
You must be signed in to change notification settings - Fork 5.2k
Add MetadataUpdateDeletedAttribute #119584
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
Conversation
This attribute is intended to be emitted only by Roslyn. Its intent is to be used by reflection as a filter to "remove" types and members that have been deleted during a hot reload session. Implements dotnet#118903
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR introduces a new MetadataUpdateDeletedAttribute
that will be emitted by Roslyn to mark types and members as deleted during hot reload sessions. This attribute is designed to be used by reflection as a filter to "remove" types and members that have been deleted.
Key changes:
- Adds the new
MetadataUpdateDeletedAttribute
class to the runtime - Updates reference assemblies to include the new attribute
- Includes various compatibility suppressions that appear unrelated to the main change
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.
Show a summary per file
File | Description |
---|---|
src/libraries/System.Runtime.Loader/ref/System.Runtime.Loader.cs | Adds the reference definition of MetadataUpdateDeletedAttribute |
src/libraries/System.Private.CoreLib/src/System/Runtime/CompilerServices/MetadataUpdateDeletedAttribute.cs | Implements the actual MetadataUpdateDeletedAttribute class with documentation |
src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems | Adds the new source file to the build |
src/libraries/System.Collections.Specialized/src/CompatibilitySuppressions.xml | Removes a comment line (unrelated to main change) |
src/coreclr/nativeaot/System.Private.CoreLib/src/CompatibilitySuppressions.xml | Adds various compatibility suppressions (unrelated to main change) |
src/coreclr/System.Private.CoreLib/CompatibilitySuppressions.xml | Adds compatibility suppressions for CoreCLR (unrelated to main change) |
Even though the API was approved, this change should not be merged until dotnet/roslyn#80125 goes into Roslyn. |
src/coreclr/System.Private.CoreLib/CompatibilitySuppressions.xml
Outdated
Show resolved
Hide resolved
I do not think this PR needs to wait for that. The support for this attribute is in Roslyn already. Is the actual implementation of the filtering and tests coming to this PR? |
I can - would prefer that actually. Wasn't sure if the preference was separate so started this way. |
Yes, we prefer to have the API exposed, implemented and tested all in one PR. |
src/coreclr/System.Private.CoreLib/src/System/RuntimeType.CoreCLR.cs
Outdated
Show resolved
Hide resolved
...System.Private.CoreLib/src/System/Runtime/CompilerServices/MetadataUpdateDeletedAttribute.cs
Outdated
Show resolved
Hide resolved
...tem.Reflection.Metadata.ApplyUpdate.Test.ReflectionDeleteMethod/ReflectionDeleteMethod_v1.cs
Outdated
Show resolved
Hide resolved
src/coreclr/System.Private.CoreLib/src/System/RuntimeType.CoreCLR.cs
Outdated
Show resolved
Hide resolved
@jkotas can you give this another pass? I think we're down to mono reflection quirks. |
Looks good to me ... once all tests are green. |
/azp run |
You have several pipelines (over 10) configured to build pull requests in this repository. Specify which pipelines you would like to run by using /azp run [pipelines] command. You can specify multiple pipelines using a comma separated list. |
/azp run runtime (Build Libraries Test Run release coreclr osx x64 Debug) |
No pipelines are associated with this pull request. |
@jkotas - the tests are passing. Can you give this another review? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
/backport to release/10.0 |
Started backporting to release/10.0: https://github.com/dotnet/runtime/actions/runs/18382550241 |
This is pretty high risk for .NET 10 at this point |
@jeffhandley backporting to "release/10.0" failed, the patch most likely resulted in conflicts: $ git am --3way --empty=keep --ignore-whitespace --keep-non-patch changes.patch
Applying: Add MetadataUpdateDeletedAttribute
Applying: Add ability to filter members in reflection and test
Applying: Feedback and tests
Applying: More feedback
Applying: Fix typo
Applying: Cleanup
Applying: Cleanup
Applying: Fix ref assembly formatting
error: sha1 information is lacking or useless (src/libraries/System.Runtime.Loader/ref/System.Runtime.Loader.cs).
error: could not build fake ancestor
hint: Use 'git am --show-current-patch=diff' to see the failed patch
hint: When you have resolved this problem, run "git am --continue".
hint: If you prefer to skip this patch, run "git am --skip" instead.
hint: To restore the original branch and stop patching, run "git am --abort".
hint: Disable this message with "git config set advice.mergeConflict false"
Patch failed at 0008 Fix ref assembly formatting
Error: The process '/usr/bin/git' failed with exit code 128 Please backport manually! |
This attribute is intended to be emitted only by Roslyn. Its intent is to be used by reflection as a filter to "remove" types and members that have been deleted during a hot reload session. Implements dotnet#118903 * Add ability to filter members in reflection and test --------- Co-authored-by: Tomas Matousek <tomat@microsoft.com>
…ering (#120572) * Add MetadataUpdateDeletedAttribute (#119584) This attribute is intended to be emitted only by Roslyn. Its intent is to be used by reflection as a filter to "remove" types and members that have been deleted during a hot reload session. Implements #118903 * Add ability to filter members in reflection and test --------- Co-authored-by: Tomas Matousek <tomat@microsoft.com> * Fix assert message with errant text pasted in (#120574) --------- Co-authored-by: Steve Pfister <steveisok@users.noreply.github.com> Co-authored-by: Tomas Matousek <tomat@microsoft.com>
This attribute is intended to be emitted only by Roslyn. The plan is for it to be used by reflection as a filter to "remove" types and members that have been deleted during a hot reload session.
Resolves #118903