dotnet / corert

This repo contains CoreRT, an experimental .NET Core runtime optimized for AOT (ahead of time compilation) scenarios, with the accompanying compiler toolchain.
http://dot.net
MIT License
2.91k stars 508 forks source link

[.NET Native] ILT0005 build error with ref T returning delegate #8273

Closed Sergio0694 closed 4 years ago

Sergio0694 commented 4 years ago

Opening this issue in here for public tracking. The original comment was here in the PR where I stumbled upon this bug. I've also already sent an email to the .NET Native team with a repro and a sample project that reproduces the issue.

Overview

Using a custom ref T returning delegate seems to trigger a build error on UWP. Consider this snippet:

public delegate ref T FieldAccessor<T>();

private int n;

private void Button_OnClick(object sender, RoutedEventArgs e)
{
    FieldAccessor<int> accessor = () => ref n;

    accessor() = default;
}

Creating a blank app and just adding that code results in the following error:

ILT0005: 'C:\Program Files (x86)\Microsoft SDKs\UWPNuGetPackages\runtime.win10-x64.microsoft.net.native.compiler\2.2.8-rel-28605-00\tools\x64\ilc\Tools\nutc_driver.exe @"C:\Users\Sergio\source\repos\FieldAccessorRepro\FieldAccessorRepro\obj\x64\Release\ilc\intermediate\MDIL\FieldAccessorRepro.rsp"' returned exit code 2

You can find a full repro project to download here.

This bug is currently blocking one of the follow up PRs for the Microsoft.Toolkit.Mvvm package (here), as it causes the build to just fail on UWP. I'm hoping this could be solved with some trick/workaround on the UWP side, as I'd really, really like not to be forced to alter the API surface on the .NET Standard side for a specific issue with the .NET Native compiler.

Thanks in advance! 😊

MichalStrehovsky commented 4 years ago

Thank you for the report! We don't track .NET native issues here because people in charge of servicing .NET native don't look here (see https://github.com/dotnet/corert#net-native-for-uwp-support). Email and developer community are the only channels that work.

Sergio0694 commented 4 years ago

Hey @MichalStrehovsky - sorry about that! I had posted some .NET Native issues in this repo in the past and I hadn't noticed the note about what channels to use now! I've posted this issue in developercommunity.visualstudio.com here.

Side note, in case you had any ideas about potential workarounds for this on the UWP side, please feel free to chime in! Knowing the slow release cycle for .NET Native updates (and the fact it's basically in maintenance mode now) I'm afraid it could be months before a fix for this could actually be implemented into the compiler and shipped to the NuGet package πŸ˜₯

MichalStrehovsky commented 4 years ago

Yes, we wanted a central location where to file issues because I'm the only one who deals with .NET native AND looks here, and I don't scale.

I vaguely remember a bug report like this from the past months. The first stage of the compiler generates bad IL for any delegate with a ref return that the next stage chokes one. I'm afraid the only workaround is to not use such delegates in code the compiler compiles.

The CoreRT AOT compiler might have the same bug because the IL generator for delegates is taken from .net native.

Sergio0694 commented 4 years ago

Yes, we wanted a central location where to file issues because I'm the only one who deals with .NET native AND looks here, and I don't scale.

Ah, yeah that makes perfect sense πŸ˜„

I vaguely remember a bug report like this from the past months. The first stage of the compiler generates bad IL for any delegate with a ref return that the next stage chokes one. I'm afraid the only workaround is to not use such delegates in code the compiler compiles.

That... Doesn't sound good, ouch πŸ˜₯ I had the idea of using a ref T delegate here, as it resulted in a much more efficient implementation that was also way easier to use for devs. The only other approaches I'm aware of to capture a field to do side effect on in a closure would otherwise be:

The former is much less efficient (and I really would like to avoid reflection), the latter only works if you assume every single dev would read the docs and write perfect code, otherwise it would be incredibly dangerous to use and could break everything 🀣

Anyway Michal I really appreciate you taking the time to reply here and elaborate on what's going on with this issue!

cc. @michael-hawker - this is a follow up from that issue I mentioned the other day. I really, really don't like the idea of having an API being influenced (for the worst too) like this for a bug in a platform-specific tool, especially with the whole premise of the Microsoft.Toolkit.Mvvm library being completely designed for just .NET Standard and being platform agnostic. Unfortunately though just having this delegate in the codebase completely crushes the build for everyone using UWP πŸ˜”

I'm wondering - with the 7.0 release planned for somewhere around November, maybe there could be time for the .NET Native to push a fix for this in time for that? Assuming it wouldn't be super tricky to fix, of course. But I mean maybe this specific thing being used in a Microsoft/.NET Foundation project could be enough to prioritize a fix for this a bit? πŸ€”