CODEWITHSUNDEEP
f#overridingstack-overflowtostring
Vagif Abilov
Vagif Abilov

Reputation: 9991

Avoiding stack overflow when overriding ToString in F#

F# has a formatting directive "%A" that is very powerful because triggers a formatter to expand types and list individual members. In some places in our application data are logged using ToString method (there are some technical reasons for that), and then for types like discriminated unions it's only a type name that is logged. Too bad so we started overriding ToString methods for some types.

To give you an example:

open System

type DiscrUnion =
    | Text of string

let t1 = DiscrUnion.Text "text"
sprintf "%A" t1
sprintf "%s" <| t1.ToString()


type DiscrUnionWithToString =
    | Text of string
    override this.ToString() = sprintf "%A" this

let t2 = DiscrUnionWithToString.Text "text"
sprintf "%A" t2
sprintf "%s" <| t2.ToString()

DiscrUnion.ToString() is printed like "FSI_0003+DiscrUnion", but for DiscrUnionWithToString.ToString() I get the actual properties: Text "text".

So far so good. However, for CLR types such override causes a catastrophic result: stack overflow! Here is an example:

type PocoType() =
    member val Text : string = null with get, set

let t3 = PocoType()
t3.Text <- "text"
sprintf "%A" t3
sprintf "%s" <| t3.ToString()

type PocoTypeWithToString() =
    member val Text : string = null with get, set
    override this.ToString() = sprintf "%A" this

let t4 = PocoTypeWithToString()
t4.Text <- "text"
sprintf "%A" t4
sprintf "%s" <| t4.ToString()

Don't even try to instantiate PocoTypeWithToString. StackOverflowException.

I understand that for POCO type an attempt to use "%A" formatting directive causes ToString call, so when ToString itself contains such directive it will fail. But what is the right way for ToString overrides? And should I beware only C# kind of types (discriminated unions and records seem to work fine), or there are other things to be aware of?

Upvotes: 4

Views: 853

Answers (3)

Ivan Zaruba
Ivan Zaruba

Reputation: 4504

DiscrUnionWithToString.ToString() I get the actual properties: Text "text"

When I faced that problem I came up with this

type DiscrUnionWithToString =
| Text of string
override text.ToString() =
    match text with
    | Text text -> text

Upvotes: 0

Ringil
Ringil

Reputation: 6537

The reason why the StackOverflowException happen is because the printer uses GetValueInfoOfObject to format. As you can see, if the object is an F# object, it has special cases for how to deal with them (tuples, functions, unions, exceptions, records).

However, if it isn't one of those cases, it will make it an ObjectValue(obj). Later on, in reprL we have some special cases to deal with ObjectValues such as string, array, map/set, ienumerable, and then at the end if that fails, it will just make it a basic layout (let basicL = LayoutOps.objL obj) of type Leaf.

Much later, that Leaf is formatted using leafformatter. leafformatter can deal with primitives, but when it deals with a complex object such as your POCO, it does let text = obj.ToString(), which results in an infinite loop and the StackOverflow exception.

The solution is to not use %A on POCOs.

The good news is that the next version of F# may have a default ToString implementation for records/unions that is effectively override this.ToString() = sprintf "%A" this. The implementation for it is partially complete here: https://github.com/Microsoft/visualfsharp/pull/1589. It may solve the problem you had to begin with.

Upvotes: 7

scrwtp
scrwtp

Reputation: 13577

Simple answer - don't use such a blanket implementation for ToString everywhere.

Format string %A kicks off a rather hairy reflection-based printer that may fall back to ToString for cases it doesn't handle in a special way. See the code for anyToStringForPrintf here.

A cleaner solution would be to have a single sprintf %A at the point you log the objects rather than have all the DU's implement a boilerplate ToString, but you're saying that's not an option.

For a regular .NET class (as opposed to an F#-specific record or union), don't use this - instead use some meaningful identifier or output all the members, or do whatever else you like. Just don't kick off an endless loop of ToStrings.

Upvotes: 4

Related Questions