Remove inline IL parsing method call

[TIHan]

Found an inline IL expression in FSharp.Core where all it did was make a call to string's GetHashCode. It can be replaced with a normal expression call.

Since this was the only place where this kind of inline IL was ever used, I removed the ability to do inline IL method calls. Inline IL is only meant for FSharp.Core.

[TIHan]

This example:

let test () = hash ""

Will emit a tail call right before the call virt GetHashCode with this change; emitting the tail call kills JIT optimizations. Regardless of the change, the tail call would occur even if you called GetHashCode directly:

let test () = "".GetHashCode()

The tail call is an issue in general, see #6329; but at least hash and GetHashCode are now consistent and we should still take the change. F# will not always emit a tail call which makes me less worried.

[TIHan]

Having trouble getting this to pass CI. Some tests are not using the built FSharp.Core and are instead referencing a package.

[smoothdeveloper]

just a note for whoever it may help in the future, removing the ability to compile those IL method calls will make trying to compile pre-existing PRs fail with this message:

src\fsharp\FSharp.Core\prim-types.fs(1670,28): error FS0371: Error while parsing embedded IL

[smoothdeveloper]

Inline IL is only meant for FSharp.Core.

Despite it may be true, and Microsoft stance on supporting undocumented or non public things is well known, it may be safer to not remove the ability to do the IL method calls just on basis of removing it's sole usage in FSharp.Core

Inline IL is used in open source libraries such as FSharpPlus, FSharp.UMX and probably others, and it is likely that it is relied in closed codebases, removing the ability to do IL method calls may break things (not FSharpPlus or FSharp.UMX AFAIK), are we really saving anything by removing its support?

[cartermp]

@smoothdeveloper this PR doesn't remove the ability to do inline IL calls. It just the sole case in the compiler codebase.

[cartermp]

Just a quick scan of those libraries doesn't show them using the call instruction, nor using the compiling-fslib flag.

[cartermp]

A full GitHub search also yields no usage of this from what I can tell: https://github.com/search?p=4&q=compiling-fslib&type=Code

That doesn't account for internal codebases of course, but I think we're well within the threshold of acceptability if there is no apparent usage of this internal feature in F# OSS libraries

[smoothdeveloper]

@cartermp, what I was saying

IL method calls

of course it doesn't remove inline IL, it does remove IL method calls (call).

Yes nothing on github shows usage of call, yet I'm not sure if github usage of F# is representative of the whole set of "dark matter" existing F# codebase, which is directly impacting businesses that would rely on that,

I was just trying to:

• have you all pay another look on the decision to remove it, as it stands, I'd not rely on solely on github and I don't think it is figurable beside letting the change out, not arguing the decision if it's been pondered enough
• help people/contributors with pending PRs based on revision prior to that change by reporting the issue / error message, to make it searchable and come to the source of truth

edit: the other impact is we get a call turned into callvirt in the IL, I don't think it really matters, but let's get exhaustive since I came all this way 🙂

[TIHan]

No one other than FSharp.Core should be using inline IL. In fact, I wish we could get rid of inline IL altogether and have more intrinsics, but that is another day.

Using the compiling-fslib flag is dangerous; I think it is a mistake to allow it to be enabled. It can slightly change the semantics of the language as there are places in the compiler where it will handle behavior differently. This is absolutely unsupported behavior and we reserve the right to change it.

@smoothdeveloper I understand your concern. If the compiler allows you to do a 'thing', then people can and will do the 'thing'. That's true, but for features like this it is communicated in advance they are only for specific cases and are not supported. If we are concerned about the use of these kind of features, then having a --langversion: preview for preview features is a lost cause because we reserve the right to change and/or remove those features. Businesses could technically rely on preview features and break later on when we change it - the same thing with C#.

Regarding this specific inline IL feature, I am not worried in the slightest.

[TIHan]

Changing call to callvirt does have some differences depending the case. The JIT can optimize a callvirt to be a direct call if it knows what the type is. However, the JIT will not do that if there is a tailcall instruction before the callvirt. Doing a direct call can also trigger other optimizations such as aggressive inling. That's what I know so far.

[smoothdeveloper]

👌 got it, thanks for all the feedback @cartermp / @TIHan.