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

Revert ILLink's usage of dependency analysis framework #104267

Merged
merged 4 commits into from
Jul 2, 2024

Conversation

sbomer
Copy link
Member

@sbomer sbomer commented Jul 1, 2024

See #103987 (comment) for context.

Custom steps rely on getting a chance to see a type before we build the type info (and in particular the interface method mapping), but we make no such guarantee. #104266 tracks this problem.

Our use of the dependency analysis framework exacerbated this because MarkType was split into two pieces, with the part that calls into the custom step being delayed through a dependency analysis framework node, making it more likely to be run too late to influence the type info.

This reverts ILLink's usage of the dependency analysis framework to bring us back to a state where the ordering still isn't guaranteed, but works for the testcase that got regressed. We should bring this back as soon as possible with a proper fix that actually guarantees the ordering required by custom steps. This doesn't look completely straightforward, but should be possible to do with the dependency analysis framework.

Fixes #103987 (but we need a better long-term fix for #104266).

@sbomer sbomer requested a review from marek-safar as a code owner July 1, 2024 22:42
@dotnet-issue-labeler dotnet-issue-labeler bot added the area-Tools-ILLink .NET linker development as well as trimming analyzers label Jul 1, 2024
Copy link
Contributor

Tagging subscribers to this area: @agocke, @sbomer, @vitek-karas
See info in area-owners.md if you want to be subscribed.

@sbomer
Copy link
Member Author

sbomer commented Jul 2, 2024

/ba-g "Failure is #104263"

@sbomer sbomer merged commit c5f2d4f into dotnet:main Jul 2, 2024
159 of 162 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Aug 2, 2024
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[android] .NET 9 p6, trimmer step no longer saving changes to disk
2 participants