Compiled ToStrings under -reflectionfree for DUs and Records

[charlesroddie]

This PR implements ToString on user-defined DUs and records, under the --reflectionfree switch, by delegating into the equivalent of LanguagePrimitives.anyToStringShowingNull, as is currently used by Option. Previously only the type name was shown. The new ToString is AOT/trim friendly, provided that the inner types have AOT/trim-friendly ToString methods.

It also implements ToString on Result and Choice types, and consolidates behaviour to match Option.

Fixes fsharp/fslang-suggestions#919

Behavioural changes (ungated)

• WIP: Explicit ToStrings on FSharp.Core types are being discussed in a thread below.

Behavioural changes (subject to --reflectionfree)

• Records and DUs now generate real ToStrings instead of just the type name. The behaviour matches Option for the internal rendering, and this differs from non-reflection-free mode (sprintf "%+A") in the following ways:
  • The floating point number 5. renders as 5.
  • Records render on a single line, avoiding the complex indentation logic in sprintf "%A".
  • Anonymous records render with {| ... |} syntax instead of { ... }

While the exact code of Option couldn't be used directly (since anyToStringShowingNull is internal to FSharp.Core), it was easy to match it, and there are some tests that the behaviour is the same, so that if there are any changes to the above in Option (which there is some ongoing discussion about, e.g. over null rendering), they are synced up with other DU non-reflection-based rendering.

It is obviously easy to make the changes to --reflectionfree mode a decisive improvement, since we are starting with nothing. However the intention would be that the behaviour is good enough to switch as a default in a future version of F#.

Note: this PR and #19971 add an identical v_string_operator_info and mkCallStringOperator.

[github-actions]

❗ Release notes required

You can open this PR in browser to add release notes: open in github.dev

@charlesroddie,

Caution

No release notes found for the changed paths (see table below).

Please make sure to add an entry with an informative description of the change as well as link to this pull request, issue and language suggestion if applicable. Release notes for this repository are based on Keep A Changelog format.

The following format is recommended for this repository:

* <Informative description>. ([PR #XXXXX](https://github.com/dotnet/fsharp/pull/XXXXX))

See examples in the files, listed in the table below or in th full documentation at https://fsharp.github.io/fsharp-compiler-docs/release-notes/About.html.

If you believe that release notes are not necessary for this PR, please add NO_RELEASE_NOTES label to the pull request.

Change path	Release notes path	Description
src/FSharp.Core	docs/release-notes/.FSharp.Core/11.0.100.md	No release notes found or release notes format is not correct

---

✅ Found changes and release notes in following paths:

Change path	Release notes path	Description
src/Compiler	docs/release-notes/.FSharp.Compiler.Service/11.0.100.md

[kerams]

Kickass

[github-actions]

🔍 Tooling Safety Check — Affects-Compiler-Output
Affects-Compiler-Output: IlxGen.fs, prim-types.fs, Optimizer.fs modify IL emission and FSharp.Core runtime

Generated by PR Tooling Safety Check · opus46 4.2M · ◷

[T-Gro]

Wouldn't' the newly added IlxGen feature codegen the same code that was now added by hand?

Thinking out loud:
Maybe this could be a separate feature (which would gent turned on automatically if you had --reflectionfree), but we could apply it even without it - either by project or by type.. ?

[charlesroddie]

Yes it would, if you turned on --reflectionfree, which would then break FSharp.Core since --reflectionfree bans "%A" usage. A separate feature would work for this purpose, at the cost of more plumbing.

The bigger question is when will consumers (both FSharp.Core and end users) who don't set --reflectionfree get the compiled ToStrings in the future. Turning on the flag by default would be dangerous because of the %A ban.

Perhaps the options are:

1. Introduce a new feature now, use it in FSharp.Core straight away, and in the future it could be on by default.
2. Stick with --reflectionfree, and in future make it on by default but remove the %A ban in it at that point (with %A possibly getting safer in future and with trim/AOT warnings available from the regular toolchain). Once this happens these explicit overrides can be removed from FSharp.Core.

[charlesroddie]

How about this @T-Gro :

• Make a LanguageFeature.CompiledToStrings
• FSharp.Core opts into this and then instead of adding manual ToStrings, this PR removes them from Option and ValueOption since they are now auto-generated.
• The implementation checks for either LanguageFeature.CompiledToStrings or --reflectionfree

[T-Gro]

I like that.

Just instead of a LanguageFeature, it shall be a dedicated compiler flag flowing via CompilerConfig.
LanguageFeatures go via a mapping from language version to sets of features, we cannot easily cherry pick one by one (unless this functionality is added).

CompilerConfig is better for dedicated flags that work orthogonal to versions of the language.

Can we still guarantee no breaking changes for regular , non reflectionfree , users of ToString() ?

[charlesroddie]

Happy to implement this. It's an extra flag that is hopefully temporary but presumably that's a small cost and it doesn't need to be publicized to end-users.

Used in FSharp.Core, the affected types will be:

• Option: this will have ToString removed and output will be unchanged.
• Result and Choice... types will go back to having no explicit ToStrings.
• Ref<'T> is affected and will need to be tested in the same way as Result but with record syntax.

Map and Set and List are unaffected since they have explicit ToStrings.

ValueOption has an explicit ToString but that should be removed since it's inconsistent: Some(0).ToString() gives "Some(0)" while ValueSome(0).ToString() gives "0" which should be corrected to "ValueSome(0)".

The behavioural changes for end-users will all be consistency improvements.

[T-Gro]

Isn't there a scenario where a core type will move off the %A path (in its current ToString codegen) to generated ToString(), that however does not have all the sprintfn features?

Considering the variety of (nested) generic arguments there can be in Result,Choice,List,...

We can afford some breaking changes when moving to F# 11, but we must document those.
We must admit the existence of programs rallying on some current stringified layout of their object graph and parsing it ( Not something one should be doing, I know...)

[charlesroddie]

I did a spike on this and there was a problem. The FSharp.Core types are defined early, in prim-types, but this PR's generated ToString requires inline box and string operators to render each field, which can't be inlined that early in FSharp.Core's compilation. The existing explicit overrides call the non-inline equivalent anyToStringShowingNull. So I think explicit overrides is best.

To determine the scope of any FSharp.Core changes that should be made:

Code used by Claude to find complete list of public FSharp.Core DU and record types

open System
open System.Reflection
let asm = typeof<int list>.Assembly  // the running FSharp.Core
let kindOf (t: Type) =
    t.GetCustomAttributesData()
    |> Seq.tryPick (fun a ->
        if a.AttributeType.Name = "CompilationMappingAttribute" && a.ConstructorArguments.Count >= 1 then
            try Some (Convert.ToInt32(a.ConstructorArguments.[0].Value) &&& 31) with _ -> None
        else None)
for t in asm.GetExportedTypes() |> Array.sortBy (fun t -> t.FullName) do
    match kindOf t with
    | Some 1 -> printfn "union  %s" t.FullName
    | Some 2 -> printfn "record %s" t.FullName
    | _ -> ()

This gives the list below, plus AsyncReturn which is excluded as non-user-constructable.

Current ToString behaviour of FSharp.Core public unions/records:

ToString() and string are identical for all 13 types.

There is no use of sprintf "+%A" in these ToStrings, so they do not suffer from the same problems as user-defined code. However, some of them have no explicit ToString and so return the type name.

ToString implementations were either defined explicitly and consistent with option, defined explicitly and inconsistent, or undefined (printing the type name). None of them are defined as sprintf "+%A so they all differ from user-defined DUs and records.

sprintf "%A" is given as a helpful but not definitive reference in the case where ToString has no current implementation.

Type	Class	ToString() ≡ string	sprintf "%A"	Recommendation for ToString
list	union	[1; 2; 3]	[1; 2; 3]	No change
option	union	Some(1)	Some 1	No change
voption	union	1	ValueSome 1	Make consistent with option (ValueSome(1))
Result	union	type name	Ok 1	Implement (Ok(1))
Choice 2..7	union	type name	Choice1OfN 1	Implement (Choice1OfN(1))
ref	record	type name	{ contents = 1 }	Implement ({ contents = 1 })
Async	record	type name	type name	No change

The implementations of Result and Choice are already done, so I would add ref, or they could be taken into another PR if considered independent of this one.

[charlesroddie]

@T-Gro I think the above analysis makes a strong case for:

• Removing the existing FSharp.Core overrides from this PR, since they turn out not to be needed for reflection-free codegen.
• No need for a separate compiler flat since FSharp.Core is not going to be a consumer of this.
• Creating an language suggestion to propose the above changes to ToString behaviour and also to formatted strings.

Sound good?

[T-Gro]

What if Map,Set,List were user-defined and having the newly added ToString behavior?
A hypothetical custom-list is a nice test case.

Leading to a different question - this PR brings special support to some FSharp.Core types. What about collection types?

[charlesroddie]

I'm not sure what's special about collection types.

Generating ToStrings for classes would be possible based on primary constructor inputs, but would need a separate language suggestion I think.

[T-Gro]

Collection types are a good test for stackoverflow exceptions, as one can expect collections to carry a lot of data.

[charlesroddie]

OK I see what you mean. If you implemented the FSharp types in their historical DU forms, without a ToString override:

For Map and Set (balanced binary trees) there would be no stackoverflow problem because of low depth.

For an unbalanced binary tree, you can get stackoverflows on ToString but you would also get them on equality and comparison.

For List the problem emerges as equality and comparison are OK (tail-recursive) but for ToString you are attempting to get a result like Branch(Node(0), Branch(Node(1), ...)), and it will stackoverflow for large data.

So it's a situation where someone defines something linked-list-like, forgets to put a ToString on it... pretty niche in my opinion. A stackoverflow exception is expected behaviour.