fsharp icon indicating copy to clipboard operation
fsharp copied to clipboard

NullReferenceException when calling a virtual Object method on a value type from inline function

Open ForNeVeR opened this issue 5 years ago • 7 comments

Repro steps

Write the following F# program, and run it:

let inline toString (x: ^a) =
    (^a : (member ToString : unit -> string) x)

[<EntryPoint>]
let main argv =
    let s = toString 123
    printfn "%s" s
    0

Expected behavior

This program should print a string 123.

Actual behavior

This program throws a NullReferenceException:

Unhandled exception. System.NullReferenceException: Object reference not set to an instance of an object.
   at Program.main(String[] argv) in T:\Temp\ConsoleApp12\ConsoleApp12\Program.fs:line 6

It works on reference types though.

Known workarounds

Do not use an inline function there.

Related information

$ dotnet --info
.NET Core SDK (reflecting any global.json):
 Version:   3.1.100
 Commit:    cd82f021f4

Runtime Environment:
 OS Name:     Windows
 OS Version:  10.0.18362
 OS Platform: Windows
 RID:         win10-x64
 Base Path:   C:\Program Files\dotnet\sdk\3.1.100\

Host (useful for support):
  Version: 3.1.0
  Commit:  65f04fb6db

ForNeVeR avatar Jan 05 '20 14:01 ForNeVeR

It's a bit of a guess, but I think this is by design. Your code doesn't do boxing, and the compiler doesn't automatically do that for statically resolved parameters. If you'd add box, I think it'll work as expected.

abelbraaksma avatar Jan 05 '20 17:01 abelbraaksma

By design or not, I believe that's a very bad idea to compile code in such a way that's guaranteed to fail in runtime.

It should either compile well or generate a compile error, and should never do the strange thing. I'm okay with filing a language suggestion if it's preferred for this particular change.

ForNeVeR avatar Jan 06 '20 09:01 ForNeVeR

I believe that's a very bad idea to compile code in such a way that's guaranteed to fail in runtime.

I agree, but there are a lot of cases where it's impossible for a compiler to find out at compile time that it will always fail, let alone to decide whether your code was intentional, or not.

I mean, you can write let f x = x / 0, but the compiler won't complain, even though it'll always fail when called.

Though it's pretty hard to know as a programmer that you need boxing when you use inline generics to mix both reference and value types. There are special extensions in the compiler code itself to deal with this, but these are not available to normal code.

I believe there are several proposals underway, and SRTP itself gets a lot of attention for F# 5.0 at the moment, but I'm not sure any of those address this particular issue.

Interestingly, F#'s own string function uses boxing as well to prevent this error from occurring.

abelbraaksma avatar Feb 06 '20 18:02 abelbraaksma

Is it really that hard to know which cases are "bad"? It seems they are simple (or at least let's say "straightforward"), if I understand correctly.

We could issue a compilation error / warning each time a member constraint (e.g. a (member ToString : unit -> string)) has to resolve to a virtual member if it is known that the statically resolved type in question (^a) is a value type.

Does it sound right? Are there any nonproblematic uses of this pattern (I mean, will such an error break any good, working code)? Any other downsides I don't see?

To me, it seems it's possible to detect all such uses of virtual methods on structs in compile time (because it's the compiler that emits the broken code), but I may be very wrong in this regard.

ForNeVeR avatar Feb 09 '20 12:02 ForNeVeR

Actually, I now think this is solvable, because virtual members on structs can only ever be the ones defined on object (and interfaces, but that's not possible without new language features). I believe they can be successfully run with a constrained callvirt, in which case no boxing takes place. But the compiler must issue the correct call.

abelbraaksma avatar Jul 15 '20 08:07 abelbraaksma

Tagging https://github.com/dotnet/fsharp/issues/4924 and using this as a canonical sub-problem of that issue

https://github.com/dotnet/fsharp/issues/4924#issuecomment-390406522 for more information

cartermp avatar Jul 23 '20 15:07 cartermp

Like everything related to SRTP resolution we should assess w.r.t. to the overall work in #6805

dsyme avatar Aug 31 '20 16:08 dsyme