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
Add handling of calli in MethodBodyScanner #1325
Conversation
We now need this to link CoreLib.
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.
Linker has ability to do IL based tests, maybe we should add one for this.
It'd be great to have this covered by test (it's new scenario and can break with other things). We could bump C# compiler dependency to make it easier |
AFAIK this is still only available in the Roslyn myget feed (it wasn't part of the preview releases yet) - would we be okay with grabbing a compiler from there? I really wanted to avoid writing IL tests for these because they're a pain to maintain (we have a bunch of default interface methods tests in CoreCLR repo that are in IL and all the reasons why they were in IL are now gone, but I'll have to deal with them for the IL Linker work now and it's just annoying...). |
Are you sure? I think Roslyn comes from Arcade in our case so if this code which exposed this bug is about to land into dotnet/runtime you should be able to write the same code here as we sync on arcade dependency. |
The Roslyn we're using to build the tests is some old pre-C# 8 version. I'll see if I can bump it. |
I forgot we use CodeDom compiler, that might make it a bit tricky |
Hm, it's a bit more involved to get the right version of the package. I'll merge as-is so that people who're waiting in CoreLib are not blocked by this. If we get to adding tests for covariant returns, we can add testing for this too. I don't want to write either of them in IL because it's a maintenance pain. |
We now need this to link CoreLib. Commit migrated from dotnet/linker@dae2b42
We now need this to link CoreLib.
https://github.com/dotnet/runtime/pull/32520/files#r451338721