Uh oh!
There was an error while loading. Please reload this page.
Pull in tests from iltrim prototype - #126275
Closed
MichalStrehovsky wants to merge 1 commit into
Closed
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a set of new linker/analyzer test cases (ported from the iltrim prototype repo) into the illink test tree, and wires them into the Roslyn analyzer test-case generator output so they run as part of the existing suites.
Changes:
- Added new
Mono.Linker.Tests.Cases.Basictest cases covering signatures, control flow, resources, generics, virtual/interface scenarios, etc. - Added new
Mono.Linker.Tests.Cases.MultiAssemblytest cases (and dependencies) covering multi-assembly linking and type forwarding. - Updated/generated
ILLink.RoslynAnalyzer.Testssuite mappings to include the new test cases.
Reviewed changes
Copilot reviewed 29 out of 31 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/tools/illink/test/Mono.Linker.Tests.Cases/MultiAssembly/TypeRefToAssembly.cs | Adds a multi-assembly typeref scope/keep assertion test. |
| src/tools/illink/test/Mono.Linker.Tests.Cases/MultiAssembly/MultiAssembly.cs | Adds a multi-assembly member retention test for a dependency assembly. |
| src/tools/illink/test/Mono.Linker.Tests.Cases/MultiAssembly/ForwarderReference.cs | Adds a type-forwarding scenario validating reference removal/retention across assemblies. |
| src/tools/illink/test/Mono.Linker.Tests.Cases/MultiAssembly/Dependencies/TypeRefToAssembly_Library.cs | Dependency type for TypeRefToAssembly test. |
| src/tools/illink/test/Mono.Linker.Tests.Cases/MultiAssembly/Dependencies/Forwarder.cs | Dependency assembly-level type forwarder attribute for forwarding scenario. |
| src/tools/illink/test/Mono.Linker.Tests.Cases/MultiAssembly/Dependencies/ForwardedType.cs | Defines forwarded type and members used in forwarder test. |
| src/tools/illink/test/Mono.Linker.Tests.Cases/MultiAssembly/Dependencies/Dep.cs | Defines dependency members used by MultiAssembly test. |
| src/tools/illink/test/Mono.Linker.Tests.Cases/Basic/VirtualMethods.cs | Adds virtual dispatch retention test. |
| src/tools/illink/test/Mono.Linker.Tests.Cases/Basic/TypeSpecSignature.cs | Adds typespec/unsafe signature coverage. |
| src/tools/illink/test/Mono.Linker.Tests.Cases/Basic/TypeOf.cs | Adds basic typeof retention scenario. |
| src/tools/illink/test/Mono.Linker.Tests.Cases/Basic/Switch.cs | Adds switch statement/expression coverage. |
| src/tools/illink/test/Mono.Linker.Tests.Cases/Basic/Resources.cs | Adds embedded resource preservation coverage. |
| src/tools/illink/test/Mono.Linker.Tests.Cases/Basic/MultiDimArraySignature.cs | Adds multidimensional array signature coverage. |
| src/tools/illink/test/Mono.Linker.Tests.Cases/Basic/MethodSpecSignature.cs | Adds generic method instantiation signature coverage. |
| src/tools/illink/test/Mono.Linker.Tests.Cases/Basic/LibraryModeTest.cs | Adds test targeting library-mode linker behavior. |
| src/tools/illink/test/Mono.Linker.Tests.Cases/Basic/InterfaceOrder.cs | Adds interface ordering metadata/retention coverage. |
| src/tools/illink/test/Mono.Linker.Tests.Cases/Basic/InterfaceCalls.cs | Adds interface call retention coverage. |
| src/tools/illink/test/Mono.Linker.Tests.Cases/Basic/InstanceFields.cs | Adds instance field/layout scenario. |
| src/tools/illink/test/Mono.Linker.Tests.Cases/Basic/GenericType.cs | Adds generic base type chain retention coverage. |
| src/tools/illink/test/Mono.Linker.Tests.Cases/Basic/GenericParameters.cs | Adds open generic parameter/type name scenario. |
| src/tools/illink/test/Mono.Linker.Tests.Cases/Basic/FunctionPointer.cs | Adds function-pointer signature coverage. |
| src/tools/illink/test/Mono.Linker.Tests.Cases/Basic/First.cs | Adds “first method”/entry-point style scenario. |
| src/tools/illink/test/Mono.Linker.Tests.Cases/Basic/Finalizer.cs | Adds finalizer retention/removal behavior test. |
| src/tools/illink/test/Mono.Linker.Tests.Cases/Basic/FieldsOfEnum.cs | Adds enum field retention/formatting coverage. |
| src/tools/illink/test/Mono.Linker.Tests.Cases/Basic/FieldSignature.cs | Adds field signature/type reference coverage. |
| src/tools/illink/test/Mono.Linker.Tests.Cases/Basic/FieldRVA.cs | Adds RVA/initializer data retention coverage. |
| src/tools/illink/test/Mono.Linker.Tests.Cases/Basic/ExceptionRegions.cs | Adds try/catch/filter/finally region coverage. |
| src/tools/illink/test/Mono.Linker.Tests.Cases/Basic/Calli.cs | Adds calli emission scenario (with ILVerify skipped). |
| src/tools/illink/test/Mono.Linker.Tests.Cases/Basic/Dependencies/Resources_EmbeddedResource.txt | Adds embedded resource content used by Resources test. |
| src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/generated/ILLink.RoslynAnalyzer.Tests.Generator/ILLink.RoslynAnalyzer.Tests.TestCaseGenerator/MultiAssemblyTests.g.cs | Registers new MultiAssembly suite test methods for analyzer harness. |
| src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/generated/ILLink.RoslynAnalyzer.Tests.Generator/ILLink.RoslynAnalyzer.Tests.TestCaseGenerator/BasicTests.g.cs | Registers new Basic suite test methods for analyzer harness. |
Comment on lines
+6
to
+7
| class GenericParameters | ||
| { |
There was a problem hiding this comment.
Indentation uses tab characters on the class declaration; repo .editorconfig sets indent_style=space. Please replace tabs with spaces so formatting is consistent and tooling doesn't churn the file.
Suggested change
| class GenericParameters | |
| { | |
| class GenericParameters | |
| { |
Comment on lines
+7
to
+12
| class FieldRVA | ||
| { | ||
| [Kept] | ||
| [KeptInitializerData] | ||
| static int Main() => new byte[] { 1, 2, 3, 4, 5 }.Length; | ||
| } |
There was a problem hiding this comment.
Indentation uses tab characters for the type declaration/closing brace; repo .editorconfig sets indent_style=space. Please convert these tabs to spaces to match formatting conventions.
Suggested change
| class FieldRVA | |
| { | |
| [Kept] | |
| [KeptInitializerData] | |
| staticintMain()=>newbyte[]{1,2,3,4,5}.Length; | |
| } | |
| class FieldRVA | |
| { | |
| [Kept] | |
| [KeptInitializerData] | |
| staticintMain()=>newbyte[]{1,2,3,4,5}.Length; | |
| } |
Comment on lines
+1
to
+7
| using Mono.Linker.Tests.Cases.Expectations.Assertions; | ||
| using Mono.Linker.Tests.Cases.Expectations.Metadata; | ||
| namespace Mono.Linker.Tests.Cases.MultiAssembly | ||
| { | ||
| [SetupLinkerAction ("link", "Dep")] |
There was a problem hiding this comment.
This file starts with an empty line and uses spacing like [SetupLinkerAction ("link", ...)] (space before the (). Most test cases in this tree avoid the leading blank line and use [SetupLinkerAction("link", ...)]; aligning to that style reduces diff churn.
Suggested change
| usingMono.Linker.Tests.Cases.Expectations.Assertions; | |
| usingMono.Linker.Tests.Cases.Expectations.Metadata; | |
| namespaceMono.Linker.Tests.Cases.MultiAssembly | |
| { | |
| [SetupLinkerAction("link","Dep")] | |
| usingMono.Linker.Tests.Cases.Expectations.Assertions; | |
| usingMono.Linker.Tests.Cases.Expectations.Metadata; | |
| namespaceMono.Linker.Tests.Cases.MultiAssembly | |
| { | |
| [SetupLinkerAction("link","Dep")] |
Comment on lines
+1
to
+10
| using Mono.Linker.Tests.Cases.Expectations.Assertions; | ||
| using Mono.Linker.Tests.Cases.Expectations.Metadata; | ||
| namespace Mono.Linker.Tests.Cases.MultiAssembly | ||
| { | ||
| [SetupLinkerAction ("link", "ForwardedType")] | ||
| [SetupCompileBefore("Forwarder.dll", new[] { "Dependencies/ForwardedType.cs" })] | ||
| [SetupCompileAfter ("ForwardedType.dll", new[] { "Dependencies/ForwardedType.cs" })] |
There was a problem hiding this comment.
This file starts with an empty line and has inconsistent attribute invocation spacing (e.g., SetupLinkerAction (, SetupCompileAfter (). Most test cases here use no space before ( and no leading blank line; please normalize formatting to match the surrounding convention.
Suggested change
| usingMono.Linker.Tests.Cases.Expectations.Assertions; | |
| usingMono.Linker.Tests.Cases.Expectations.Metadata; | |
| namespaceMono.Linker.Tests.Cases.MultiAssembly | |
| { | |
| [SetupLinkerAction("link","ForwardedType")] | |
| [SetupCompileBefore("Forwarder.dll",new[]{"Dependencies/ForwardedType.cs"})] | |
| [SetupCompileAfter("ForwardedType.dll",new[]{"Dependencies/ForwardedType.cs"})] | |
| usingMono.Linker.Tests.Cases.Expectations.Assertions; | |
| usingMono.Linker.Tests.Cases.Expectations.Metadata; | |
| namespaceMono.Linker.Tests.Cases.MultiAssembly | |
| { | |
| [SetupLinkerAction("link","ForwardedType")] | |
| [SetupCompileBefore("Forwarder.dll",new[]{"Dependencies/ForwardedType.cs"})] | |
| [SetupCompileAfter("ForwardedType.dll",new[]{"Dependencies/ForwardedType.cs"})] |
This was referenced Mar 29, 2026
sbomer
approved these changes
Apr 9, 2026
| b.Method1(); | ||
| // TODO: uncomment once we're okay with ToString bringing the whole world into closure | ||
| //((object)default(MyValueType)).ToString(); |
Member
There was a problem hiding this comment.
Should this be removed or uncommented?
agocke added a commit
that referenced
this pull request
Apr 14, 2026
5 years ago we did a hackathon exploring illink written on top of the ILCompiler.DependencyAnalysisFramework/managed type system and System.Reflection.Metadata. I let Claude: * rebase the commits on top of main (required interventions by Claude since things didn't merge cleanly) * make things buildable again * replace ILTrim.TypeSystem with ILCompiler.TypeSystem * Get rid of most of ILTrim.Tests in favor of just taking Mono.Linker.Tests as-is * Extracting the tests into a separate pull request #126275 * Add some extra code to ILTrim.Tests to let us baseline failing tests in a sidecar test file (feels better than adding Tools.ILTrim). * 32% of test pass * We had ~14000 lines unique to iltrim hackathon in the branch, now down to ~7000. I experimentally started prompting Claude to fix failing tests and have a couple good result, but this is not part of that. Submitting as a draft, I haven't really reviewed this myself either. Cc @dotnet/illink
MichalStrehovsky
commented
Apr 27, 2026
MemberAuthor
Running illink tests became much slower after the xunit migration, so let's not add unnecessary tests. |
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 freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
When we did the iltrim hackathon, we wrote some new tests. These are the tests from https://github.com/MichalStrehovsky/iltrim, moved to the illink tree.
Sending this out for considerations. We can say we don't want them and that would be fine too.
Cc @dotnet/illink