Field sharing for [<Struct>] DUs if they have equal name and equal type

[T-Gro]

First see what breaks.. (I am basing this on another change which is now awaiting PR review, sorry for larger diff because of that)

This implements part of the approved suggestion outlined here fsharp/fslang-suggestions#699 (comment) .

Implements #15715

Since such code did not compile before, this does not lead to a breaking change.
Even though this isn't the full approved suggestion, it is one that can be brought in without significant risks or breaking changes.

As of now, this allows field reuse as long as the field is equally named and has the same type after erasure.
(therefore e.g. UoM can be reused and therefore take a lot less space - see the added tests with sizeof<> checks added)

(I also personally believe that the full blown solution would be better done with a runtime change to support data overlap even with reference types and generics, and also utilizing the sizing of structs known at JIT time, but not at compile time)

[edgarfgp]

@T-Gro Any idea when this will make it to the main? I have a couple of PRs that will improve error reporting around the same area. So do not want to make it difficult for you with the potential conflicts

[vzarytovskii]

Does this change the codegen for all language versions, or only preview? If it's all (I saw some baselines have changed), it's too risky to merge it before November.

[edgarfgp]

Does this change the codegen for all language versions, or only preview? If it's all (I saw some baselines have changed), it's too risky to merge it before November.

Maybe I can borrow the diagnostic/error reporting done in here. So this PR will focus in the codegen improvements?

[T-Gro]

Does this change the codegen for all language versions, or only preview? If it's all (I saw some baselines have changed), it's too risky to merge it before November.

This PR builds on top of #15695, which is a bugfix - and it does change codegen, because the nature of the bug was the fact that codegen was hacky and limiting more cases leading to an unexpected and cryptic runtime failure.

The specific part of this PR only:
It does not, since such definitons (sharing names) were considered an error and not compilable.

[abonie]

So what is the conclusion, can this be merged now or should it wait until November?

[psfinaki]

@T-Gro I can review this next week. There are conflicts here though

[T-Gro]

I will resolve conflicts once this has the approvals.

[psfinaki]

Overall LGTM, do you think the changes related to the new functionality can be more isolated in the code?

AFAIU this is just one step on the way to the new feature so it would be helpful for the future contributions to make this more clear.

[T-Gro]

Overall LGTM, do you think the changes related to the new functionality can be more isolated in the code?

AFAIU this is just one step on the way to the new feature so it would be helpful for the future contributions to make this more clear.

The logic is already isolated within EraseUnions.fs, 1 file is already a good level if isolation considering PRs here.
Anything further would not work without redesigning (not just refactoring, but really a risky redesign) of EraseUnions alltogether - therefore causing massive conflicts for nullability, real visibility and other WIP PRs.
=> not worth it.

[T-Gro]

/run fantomas

[github-actions]

Ran fantomas: https://github.com/dotnet/fsharp/actions/runs/7008034087

[xperiandri]

Will this change also expose common fields as properties on a base type? So that they can be accessed without matching

[T-Gro]

For C# consumers, they are exposed.
In F#, pattern matching is to be used.