[ILLink] Mark recursive interface implementations in MarkStep#99922
Merged
jtschuster merged 8 commits intodotnet:mainfrom Mar 27, 2024
Merged
[ILLink] Mark recursive interface implementations in MarkStep#99922jtschuster merged 8 commits intodotnet:mainfrom
jtschuster merged 8 commits intodotnet:mainfrom
Conversation
- ILVerify will fail due to members not being kept, so to make the errors more informative, run kept member validation first, then ILVerify
ghost
added
the
needs-area-label
dotnet-policy-service
bot
added
the
linkable-framework
Member
Author
|
The only test diff compared with #99566 comes in MultipleDimsProvidedByRecursiveInterface.cs: @@ -16,14 +16,16 @@ namespace Mono.Linker.Tests.Cases.Inheritance.Interfaces.DefaultInterfaceMethods
[KeptMemberInAssembly ("library.dll", typeof(Program.I0), "Method()")]
[KeptTypeInAssembly ("library.dll", typeof(Program.I00))]
[KeptMemberInAssembly ("library.dll", typeof(Program.I00), "Program.I0.Method()")]
- [KeptMemberInAssembly ("library.dll", typeof(Program.I01), "Program.I0.Method()")]
+ // Bug: DIM resolution doesn't look at recursive interfaces
+ //[KeptMemberInAssembly ("library.dll", typeof(Program.I01), "Program.I0.Method()")]
[KeptInterfaceOnTypeInAssembly ("library.dll", typeof (Program.I00), "library.dll", typeof (Program.I0))]
[KeptInterfaceOnTypeInAssembly ("library.dll", typeof (Program.MyFoo), "library.dll", typeof (Program.I000))]
[KeptTypeInAssembly ("library.dll", typeof(Program.I000))]
[KeptInterfaceOnTypeInAssembly ("library.dll", typeof (Program.I000), "library.dll", typeof (Program.I00))]
- [KeptInterfaceOnTypeInAssembly ("library.dll", typeof (Program.MyFoo), "library.dll", typeof (Program.I010))]
- [KeptInterfaceOnTypeInAssembly ("library.dll", typeof (Program.I010), "library.dll", typeof (Program.I01))]
- [KeptInterfaceOnTypeInAssembly ("library.dll", typeof (Program.I01), "library.dll", typeof (Program.I0))]
+ // Bug: DIM resolution doesn't look at recursive interfaces
+ //[KeptInterfaceOnTypeInAssembly ("library.dll", typeof (Program.MyFoo), "library.dll", typeof (Program.I010))]
+ //[KeptInterfaceOnTypeInAssembly ("library.dll", typeof (Program.I010), "library.dll", typeof (Program.I01))]
+ //[KeptInterfaceOnTypeInAssembly ("library.dll", typeof (Program.I01), "library.dll", typeof (Program.I0))]
#endif
class MultipleDimsProvidedByRecursiveInterface
{ |
jtschuster
added
area-Tools-ILLink
dotnet-policy-service
bot
added
the
linkable-framework
jtschuster
changed the title
jtschuster
added
the
NO-MERGE
sbomer
reviewed
Mar 21, 2024
...s/illink/test/Mono.Linker.Tests.Cases/UnreachableBlock/CompilerGeneratedCodeSubstitutions.cs
Show resolved
Hide resolved
Member
|
Do we need to update any other calls to |
Member
Author
Yeah, there are a few places, like interface method resolution, but they might be fairly significant changes, so should we separate them out into their own PRs? |
sbomer
reviewed
Mar 25, 2024
Member
sbomer
left a comment
sbomer
left a comment
There was a problem hiding this comment.
Yeah, there are a few places, like interface method resolution, but they might be fairly significant changes, so should we separate them out into their own PRs?
That seems fine to me. The worst I would expect is that we miss marking some interfaces which we are already missing today.
Co-authored-by: Sven Boemer <[email protected]>
jtschuster
removed
the
NO-MERGE
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
From #99566 (comment), splitting off a simpler change that doesn't cache the recursive interfaces and doesn't use the recursive interfaces in DIM finding. This does nearly double the time it takes to trim a hello world, whereas #99566 speeds it up slightly, so I'd be concerned about timeouts if this gets merged.