-
Notifications
You must be signed in to change notification settings - Fork 5.2k
Update ILVerify to honor the "async" flag #121503
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
|
Tagging subscribers to this area: @JulieLeeMSFT |
b82dbda to
8454c59
Compare
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 adds support for async method verification in ILVerification. The changes enable the verifier to properly handle async methods that return Task or ValueTask types by unwrapping their return types and validating the stack state accordingly.
Key changes:
- Modified return statement verification logic to detect async methods and unwrap Task/ValueTask return types
- Added helper method to identify and unwrap Task, ValueTask, Task, and ValueTask types
- Added comprehensive test coverage for async method verification scenarios
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/tests/ilverify/ILTests/RuntimeAsyncTests.ilproj | Project file for new async method verification tests |
| src/tests/ilverify/ILTests/RuntimeAsyncTests.il | IL test cases covering valid and invalid async method patterns |
| src/coreclr/tools/ILVerification/ILImporter.Verify.cs | Implementation of async method return type validation logic |
|
/ba-g Timeouts |
|
I am not sure if ILVerify tests are running in outerloop. I don't see an "ilverify" work item, and when I follow the instructions in the readme I do not get any ILVerificationTests.cmd file. @jkoritzinsky Fall out from test merging? Any idea how to run these tests now? |
|
I think Jeremy is OOF. I was able to validate the tests locally so I think this is good to merge. I opened #121594 about the testing. |
Runtime specification: https://github.com/dotnet/runtime/blob/main/docs/design/specs/runtime-async.md
Note: ilasm/ildasm were already updated to recognize the "async" keyword
Relates to test plan dotnet/roslyn#75960