Repository navigation
[R2R] Handle missing virtual method signatures - #132337
Merged
agocke merged 2 commits intoAug 14, 2026
Merged
Conversation
Keep interface virtual dependency discovery best-effort when a method signature cannot be resolved. Preserve dependencies discovered before the failure so affected code can fall back to runtime JIT. Co-authored-by: Copilot <[email protected]> Copilot-Session: 6a24ce0e-bddf-4530-8331-432914f41441
|
Azure Pipelines: Successfully started running 3 pipeline(s). 13 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
Co-authored-by: Copilot <[email protected]> Copilot-Session: 6a24ce0e-bddf-4530-8331-432914f41441
jkotas
approved these changes
Aug 14, 2026
Contributor
There was a problem hiding this comment.
Pull request overview
This PR improves crossgen2/R2R dependency analysis robustness when interface virtual method signature resolution fails (e.g., due to missing referenced assemblies), and adds regression coverage for that scenario.
Changes:
- Make
InheritedVirtualMethodsNoderesilient toTypeSystemExceptionduring inherited interface virtual dependency discovery by catching and continuing with already-collected dependencies. - Fix the R2R test harness to propagate the output directory to dependent
CompiledAssemblyinstances. - Add a regression test case (
MissingVirtualSignature) exercising an interface method signature that depends on an assembly that is not provided to crossgen2.
Show a summary per file
| File | Description |
|---|---|
| src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/InheritedVirtualMethodsNode.cs | Catch type-resolution failures during interface virtual-slot scanning so compilation can fall back to runtime JIT. |
| src/coreclr/tools/aot/ILCompiler.ReadyToRun.Tests/TestCasesRunner/R2RTestRunner.cs | Ensure all Roslyn-compiled assemblies (including dependencies) receive an output directory before compilation. |
| src/coreclr/tools/aot/ILCompiler.ReadyToRun.Tests/TestCases/R2RTestSuites.cs | Add a regression test covering missing interface signature dependencies. |
| src/coreclr/tools/aot/ILCompiler.ReadyToRun.Tests/TestCases/MissingVirtualSignature/Input.cs | New test input assembly defining the problematic interface signature and an entrypoint method that should still compile. |
| src/coreclr/tools/aot/ILCompiler.ReadyToRun.Tests/TestCases/MissingVirtualSignature/Dependency.cs | New dependency assembly defining the missing return type referenced by the interface signature. |
Review details
- Files reviewed: 5/5 changed files
- Comments generated: 2
- Review effort level: Lite
Contributor
There was a problem hiding this comment.
Review details
Suppressed comments (2)
src/coreclr/tools/aot/ILCompiler.ReadyToRun.Tests/TestCases/MissingVirtualSignature/Input.cs:9
- This test-case source is compiled with nullable enabled (
NullableContextOptions.Enablein R2RTestCaseCompiler). Returningnullfor a non-nullable reference type will produce CS8603 warnings during the in-test Roslyn compilation. Usenull!(or make the return type nullable) to keep the test compilation warning-free while preserving the missing-type reference in the signature.
MissingSignatureType GetMissingType() => null;
src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/InheritedVirtualMethodsNode.cs:141
- The
TypeSystemExceptionhandler currently wraps the entire interface-scan block, so a single resolution failure will short-circuit discovery for all remaining implemented interfaces on this type (even if they’re fully resolvable). To keep recovery best-effort and minimize unnecessary JIT fallback, consider moving the try/catch inside the per-interface (or per-method) loop so failures only skip the problematic interface/method and the rest of the interfaces are still analyzed.
catch (TypeSystemException)
{
// TODO: https://github.com/dotnet/runtime/issues/132338 - Gate this recovery on --resilient and report a warning.
}
- Files reviewed: 5/5 changed files
- Comments generated: 0 new
- Review effort level: Lite
agocke
enabled auto-merge (squash)
August 14, 2026 20:28
This was referenced Aug 14, 2026
Member
Author
|
/ba-g all failures |
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.
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.
Summary
TypeSystemExceptionduring inherited interface virtual dependency discoveryThis intentionally keeps the change local to
InheritedVirtualMethodsNode; broader resilient-mode policy and diagnostics will be handled separately.Testing
InheritedVirtualMethodsNode.GetConditionalStaticDependenciesILCompiler.ReadyToRun.Testsbuild: 0 warnings, 0 errorsMissingVirtualSignature: 1 passed, 0 failedNote
This pull request description was generated with GitHub Copilot.