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

Mark .overrides from marked methods in a copy assembly #82197

Merged
merged 1 commit into from
Feb 16, 2023

Conversation

jtschuster
Copy link
Member

Fixes #81746

Static abstract interface methods that aren't used are expected to be removed, and any references to them in the metadata of the overriders are expected to be removed in SweepStep. However, overrides may come from a 'copy' assembly, so their metadata won't get swept at all. We would still remove the static abstract method, but would leave the reference to it in the overrider's metadata, creating invalid metadata.

This fixes the issue by making sure all methods referenced in the .overrides metadata of method X are marked if method X is in a copy assembly, where previously we would postpone marking the .overrides if the base was static abstract.

@ghost ghost added the linkable-framework Issues associated with delivering a linker friendly framework label Feb 15, 2023
@dotnet-issue-labeler dotnet-issue-labeler bot added the area-Tools-ILLink .NET linker development as well as trimming analyzers label Feb 15, 2023
@ghost ghost assigned jtschuster Feb 15, 2023
@ghost
Copy link

ghost commented Feb 15, 2023

Tagging subscribers to 'linkable-framework': @eerhardt, @vitek-karas, @LakshanF, @sbomer, @joperezr
See info in area-owners.md if you want to be subscribed.

Issue Details

Fixes #81746

Static abstract interface methods that aren't used are expected to be removed, and any references to them in the metadata of the overriders are expected to be removed in SweepStep. However, overrides may come from a 'copy' assembly, so their metadata won't get swept at all. We would still remove the static abstract method, but would leave the reference to it in the overrider's metadata, creating invalid metadata.

This fixes the issue by making sure all methods referenced in the .overrides metadata of method X are marked if method X is in a copy assembly, where previously we would postpone marking the .overrides if the base was static abstract.

Author: jtschuster
Assignees: -
Labels:

linkable-framework

Milestone: -

Copy link
Member

@sbomer sbomer 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 a lot!

@jtschuster jtschuster merged commit b171cda into dotnet:main Feb 16, 2023
@jtschuster jtschuster deleted the OverrideInCopyAssembly branch February 16, 2023 19:01
@jtschuster jtschuster restored the OverrideInCopyAssembly branch February 17, 2023 20:42
jtschuster added a commit to jtschuster/linker that referenced this pull request Feb 17, 2023
jtschuster added a commit to jtschuster/linker that referenced this pull request Feb 17, 2023
jtschuster added a commit to jtschuster/linker that referenced this pull request Feb 17, 2023
jtschuster added a commit to jtschuster/linker that referenced this pull request Feb 17, 2023
@marek-safar marek-safar added this to the 7.0.x milestone Feb 20, 2023
jtschuster added a commit to jtschuster/linker that referenced this pull request Feb 21, 2023
jtschuster added a commit to jtschuster/linker that referenced this pull request Feb 21, 2023
jtschuster added a commit to dotnet/linker that referenced this pull request Feb 21, 2023
Static abstract interface methods that aren't used are expected to be removed, and any references to them in the metadata of the overriders are expected to be removed in SweepStep. However, overrides may come from a 'copy' assembly, so their metadata won't get swept at all. We would still remove the static abstract method, but would leave the reference to it in the overrider's metadata, creating invalid metadata.

This fixes the issue by making sure all methods referenced in the .overrides metadata of method X are marked if method X is in a copy assembly, where previously we would postpone marking the .overrides if the base was static abstract.
@ghost ghost locked as resolved and limited conversation to collaborators Mar 22, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-Tools-ILLink .NET linker development as well as trimming analyzers linkable-framework Issues associated with delivering a linker friendly framework
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Crash when deserializing certain types in Blazor Web Assembly projects when project is published
3 participants