search

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 510 forks source link

Display which type providing compiler helper is missing #8355

Closed kant2002 closed 3 years ago

kant2002 commented 3 years ago

I add these changes when I attempt to troubleshoot custom runtime and it failed that type was missing without indication why it want that type in first place. Have to know method which compiler want to use helps me troubleshoot what I should do.

jkotas commented 3 years ago

Have to know method which compiler want to use helps me troubleshoot what I should do.

I have looked at the error messages before and after this change. I am not sure whether the more verbose error message actually helps with troubleshooting. The error messages before this change are:

Expected type 'Internal.Runtime.CompilerHelpers.ThrowHelpers' not found in module 'System.Private.Corelib'

or

Expected method 'ThrowMissingFieldException' not found on type 'Internal.Runtime.CompilerHelpers.ThrowHelpers'

After this change:

Cannot find compiler helper method ThrowMissingFieldException on type ThrowHelpers.
Expected type 'Internal.Runtime.CompilerHelpers.ThrowHelpers' not found in module 'System.Private.Corelib'

or

Cannot find compiler helper method ThrowMissingFieldException on type ThrowHelpers.
Expected method 'ThrowMissingFieldException' not found on type 'Internal.Runtime.CompilerHelpers.ThrowHelpers'

I like the shorter messages better. The longer error messages are harder to parse. The shorter message clearly states what the immediate problem is. Once you fix the immediate problem (e.g. add a dummy ThrowHelpers type in my example), you will get the next error about more stuff missing. It is pretty standard process for experiments with stripped down runtimes.

MichalStrehovsky commented 3 years ago

I have mixed feeling about this pull request too. The thing I have the biggest issue with is the catch (InvalidOperationException). We generally only catch very specific exception types that we own (and we can't get a spurious one from the framework that we could unintentionally catch). The InvalidOperationException here is not intended to be catchable - it's just supposed to be "one of the exception types we don't control and should tear down the compilation".

Making a new exception type for this so that we keep the existing model feels like too much code for what this brings.

kant2002 commented 3 years ago

Let me recreate scenario where I have a need for such diagnostics.

On Mon, Oct 12, 2020 at 13:34 Michal Strehovský notifications@github.com wrote:

I have mixed feeling about this pull request too. The thing I have the biggest issue with is the catch (InvalidOperationException). We generally only catch very specific exception types that we own (and we can't get a spurious one from the framework that we could unintentionally catch). The InvalidOperationException here is not intended to be catchable - it's just supposed to be "one of the exception types we don't control and should tear down the compilation".

Making a new exception type for this so that we keep the existing model feels like too much code for what this brings.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/dotnet/corert/pull/8355#issuecomment-706935558, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABAPKN4E3JW5BHVERQILQDDSKKWPTANCNFSM4SDVGQNA .

jkotas commented 3 years ago

If you can demonstrate that this is a meaningful improvement, could you please open a new PR against https://github.com/dotnet/runtimelab/tree/feature/NativeAOT? Thank you!