ToString() throws an exception for ValueOption<_>when value is ValueNone

[rca22]

The following code throws

let x : ValueOption<int> = ValueNone
let y = x.ToString() 

System.InvalidOperationException: ValueOption.Value
   at Microsoft.FSharp.Core.FSharpValueOption`1.get_Value()
   at Microsoft.FSharp.Core.FSharpValueOption`1.ToString()
   at Microsoft.FSharp.Core.PrintfImpl.FinalFast1@264.Invoke(FSharpFunc`2 env, A a)
   at <StartupCode$FSI_0013>.$FSI_0013.main@()

The behaviour should be comparable to the option type where

let x : Option<int> = None
let y = x.ToString()

Does not throw and sets y = "<null>"

This is a real pain when logging value in text files, as you expect sprintf "%O" to work for all F# types. Perhaps you could simply override .ToString() for ValueOption?

This is FSharp.Core 4.6.2, .Net Framework 4.8

[abelbraaksma]

Out of curiosity, does string x give the same error? And what about `sprintf "%o" x"? Imo this is probably a bug (or a regression if it worked before).

[rca22]

Yes, string x and sprintf “%O” x give the same error, as a result of them calling the underlying .ToString() method I assume.

…

Sent from my iPhone On 4 Oct 2019, at 20:08, Abel Braaksma <notifications@github.com<mailto:notifications@github.com>> wrote: Out of curiosity, does string x give the same error? And what about `sprintf "%o" x"? Imo this is probably a bug (or a regression if it worked before). — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub<#7693?email_source=notifications&email_token=AKGTZ376W6U7YW7WHJWBU4TQM6IDDA5CNFSM4I5HBAT2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEAMTW7Y#issuecomment-538524543>, or mute the thread<https://github.com/notifications/unsubscribe-auth/AKGTZ34D7AHXB4VAF657GC3QM6IDDANCNFSM4I5HBATQ>.

[abelbraaksma]

If I understand the related linked issues correctly, x.ToString() should throw if it is supposed to be behave the same as option, but string x should not.

The question remains what we should print for ValueNone. And perhaps it is possible not to throw on x.ToString(), after all ValueNone is not null.

[rca22]

@abelbraaksma: you've misunderstood. x.ToString() should never throw, because for a discriminated union like ValueOption, all values are valid. string x should also never throw as a result - this will end up calling the ToString() member. I would suggest printing "<null>" for ValueNone, so that the behaviour is identical to Option<_>

[abelbraaksma]

@rca22, I should have been clearer that I tried to summarize the discussions in the other thread.

I don't agree with "<null>", as a value can never be null.

I think full parity is not possible, because currently, option with None throws. And you're suggesting it shouldn't. Or we should change the behavior for options as well.

[rca22]

@abelbraaksma No: option with None does not throw - as I put in my original email. The ValueOption behaviour should match that, IMO.

[cartermp]

The current PR gives "ValueNone" as per here: https://github.com/dotnet/fsharp/pull/7712/files#diff-1ff43febb9aa3f01fb97440321fb1507R226-R228

This is because Option compiles the None case down into a null, but ValueOption does not because it is a struct.

[rca22]

In that case, I agree it is a very good reason to display ValueNone. Richard

…

Sent from my iPhone On 9 Oct 2019, at 20:01, Phillip Carter <notifications@github.com<mailto:notifications@github.com>> wrote: The current PR gives "ValueNone" as per here: https://github.com/dotnet/fsharp/pull/7712/files#diff-1ff43febb9aa3f01fb97440321fb1507R226-R228 This is because Option compiles the None case down into a null, but ValueOption does not because it is a struct. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub<#7693?email_source=notifications&email_token=AKGTZ36SX3BWXXL2QW5MGE3QNYMA7A5CNFSM4I5HBAT2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEAYY2UI#issuecomment-540118353>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AKGTZ3YVCNYXK43EKYRWPJDQNYMA7ANCNFSM4I5HBATQ>.

[abelbraaksma]

No: option with None does not throw - as I put in my original email. The ValueOption behaviour should match that, IMO.

@rca22, not on my current version of VS 2019, 16.3.1, which I think is very recent. I don't know how you tested it, but I cannot repro the non-error scenario:

> None.ToString();;
System.NullReferenceException: Object reference not set to an instance of an object.
   at <StartupCode$FSI_0017>.$FSI_0017.main@()
Stopped due to error

> let x = None in x.ToString();;
System.NullReferenceException: Object reference not set to an instance of an object.
   at <StartupCode$FSI_0019>.$FSI_0019.main@()
Stopped due to error

> sprintf "%O" None;;
val it : string = "<null>"

> sprintf "%A" None;;
val it : string = "<null>"

> ValueNone.ToString();;
System.InvalidOperationException: ValueOption.Value
   at Microsoft.FSharp.Core.FSharpValueOption`1.get_Value() in E:\A\_work\159\s\src\fsharp\FSharp.Core\prim-types.fs:line 2934
   at Microsoft.FSharp.Core.FSharpValueOption`1.ToString() in E:\A\_work\159\s\src\fsharp\FSharp.Core\prim-types.fs:line 2951
   at <StartupCode$FSI_0020>.$FSI_0020.main@()
Stopped due to error

> let x = ValueNone in x.ToString();;
System.InvalidOperationException: ValueOption.Value
   at Microsoft.FSharp.Core.FSharpValueOption`1.get_Value() in E:\A\_work\159\s\src\fsharp\FSharp.Core\prim-types.fs:line 2934
   at Microsoft.FSharp.Core.FSharpValueOption`1.ToString() in E:\A\_work\159\s\src\fsharp\FSharp.Core\prim-types.fs:line 2951
   at <StartupCode$FSI_0021>.$FSI_0021.main@()
Stopped due to error

> sprintf "%O" ValueNone;;
System.InvalidOperationException: ValueOption.Value
   at Microsoft.FSharp.Core.FSharpValueOption`1.get_Value() in E:\A\_work\159\s\src\fsharp\FSharp.Core\prim-types.fs:line 2934
   at Microsoft.FSharp.Core.FSharpValueOption`1.ToString() in E:\A\_work\159\s\src\fsharp\FSharp.Core\prim-types.fs:line 2951
   at Microsoft.FSharp.Core.PrintfImpl.FinalFast1@259.Invoke(FSharpFunc`2 env, A a) in E:\A\_work\159\s\src\fsharp\FSharp.Core\printf.fs:line 262
   at <StartupCode$FSI_0022>.$FSI_0022.main@()
Stopped due to error

> sprintf "%A" ValueNone;;
val it : string = "ValueNone"

In other words, currently, only sprintf appears to not throw for options, but throws for value options (unless you specify %A). I'm not saying we should keep the exceptions, but both currently throw for None or ValueNone.

[rca22]

@abelbraaksma apologies for the belated response. I tried the exact same code for testing x.ToString() when x is Option<int>.None on my current version of VS which is 16.2.4 and agree that it fails, as per your last email.

> let x : int option = None;;
val x : int option = None
> let y = x.ToString();;
System.NullReferenceException: Object reference not set to an instance of an object.
   at <StartupCode$FSI_0003>.$FSI_0003.main@()
Stopped due to error
> let y = string x;;
val y : string = ""

> let y = sprintf "%O" x;;
val y : string = "<null>"

Whereas for ValueOption all of these fail. Apologies for the incorrect statement in my initial email - I must have copied the results for Option down incorrectly.

IMO the NullReferenceException thrown by Option is a bug in the compiler optimisation to use null, and shouldn't happen either! After all, if I defined something logically identical, type A<'x> = | B | C of 'x I can't imagine that would end up throwing exceptions for B...

[abelbraaksma]

While I agree it is sensible to expect no error, ToString is an overridden member, and requires an instance, hence it doesn't work. Though in F# we shouldn't have nulls and I think it is fair to expect this to work without exception.

[rca22]

Good point...

[abelbraaksma]

@KevinRansom, can we summarize what the resolution was, considering this discussion went in several directions? I.e., ideally it never throws now on either printfn family of functions and .ToString(), but are these what this PR solved or only the former?

[abelbraaksma]

Just happened to hit this issue again, so let me answer my own question, in case others are wondering what was implemented:

• string ValueNone gets printed as "ValueNone"
• ValueNone.ToString() does not throw anymore
• string(ValueSome 42) gets printed as "ValueSome 42"
• string None prints "" (empty string)
• string(Some 42) prints "Some 42"
• None.ToString() still raises an error (though the code path here suggests this should be "null")
• Inside FSI, if a value is of type option and has value None, this will be shown as "val it : int option = None"