-
Notifications
You must be signed in to change notification settings - Fork 5.2k
Fix type parameter mapping for static closures #118607
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
Conversation
Roslyn can generate code (specifically for nested async lambda state machines) where a static closure environment is referenced from a nested type of the display class. Handle this case by detecting nested classes, and never tracking type parameter instantiations from nested classes to outer classes. This fixes the reported stack overflow in ILLink and ILC.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR fixes a stack overflow issue in ILLink and ILC caused by improper type parameter mapping for static closures. The fix specifically addresses cases where Roslyn generates nested async lambda state machines with static closure environments referenced from nested types of the display class.
Key changes:
- Added logic to detect nested classes and avoid tracking type parameter instantiations from nested to outer classes
- Updated both ILLink and ILC implementations with consistent handling
- Added comprehensive test cases for nested async lambdas and local functions
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| CompilerGeneratedTypes.cs | Added test cases for nested async lambda and local function scenarios to verify the fix |
| Linker.Dataflow/CompilerGeneratedState.cs | Updated type checking logic to use IsSameOrNestedType helper method instead of simple equality |
| ILCompiler.Compiler/Compiler/Dataflow/CompilerGeneratedState.cs | Added equivalent IsSameOrNestedType implementation for ILC with proper type hierarchy traversal |
src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/CompilerGeneratedState.cs
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is a good assumption. I can't think of a situation where we would lower into a nested class. Even relatively complicated cases seem to end up in peer classes, e.g. https://sharplab.io/#v2:EYLgxg9gTgpgtADwGwBYA+ABATARgLABQGAzAATakDCpA3oaQ+WRiqQLIAUAlgHYAupLgEpa9RuIBuAQyikEpALykOIhQD5SY8QzoFt+wQGpDAbi0GGvAQCtFg0odI4zei4xbssK8xd1uDXA5K1i7++tKyAJ52KooaXMahYeKRKkn+AL4+BmxeQunaGQXiCGnmWQQZQA
|
I think this also fixes #115905. Didn't know we had a similar bug for ILLink! |
The original repro involves an async state machine that gets lowered into a nested class. The fix supports type parameters flowing into nested classes, just not from nested classes out into containing classes. |
Roslyn can generate code (specifically for nested async lambda state machines) where a static closure environment is referenced from a nested type of the display class. Handle this case by detecting nested classes, and never tracking type parameter instantiations from nested classes to outer classes. This fixes the reported stack overflow in ILLink and ILC.
Fixes #117153