struct DU with more than 48 cases causes generation of an invalid program #5342
Comments
|
Just for kicks, here's a Sharplab.io with the same content. It fails with the same error when you change the Results View to |
|
If you give them all numbers the output is simply:
|
|
Fun fact, this will work: The generated constructor looks weird as it has more and more arguments - looks like iterating over the different type combination so each case will have a different signature: vs vs Full IL attached: |
|
Structs supposed to be efficient. Is this the only way to create these single field structs with a tag member? I assume the single case du codegen handled differently than a multi-case du. How about using a blittable representation (the struct layout is sequential by default but not packed: #5215) for single case du and simply retype/cast the loaded tag value to the structdu value type? This is how an 1000 case struct du would look like: |
|
I don't know if this is intentional, but I just looked at the IL, and it shows ctors like this: This looks wrong to me. In my mind, there should be one ctor that takes an |
|
And there are seven "dummy" types used to make constructor overloads. The fact that this problem first appears at 49 cases (a multiple of 7) seems unlikely to be a coincidence, no? |
|
If the problem is to have different signatures for each case why not generate simple internally visible only struct for each case? type [<Struct>] CountryUnknown = struct end
type [<Struct>] CountryAndorra = struct endThan the constructors will looks like this: Or even better use something like this (but internal only so another interface implementation will not possible) where the typecheck will be optimized away. : public interface ICountry { }
public struct CountryUnknown : ICountry { }
public struct CountryAndorra : ICountry { }
[MethodImpl(MethodImplOptions.NoInlining)]
static bool IsUnknown<T>() where T : struct, ICountry => typeof(T) == typeof(CountryUnknown);The asm will looks something like this (the asm here is a copy from the source issue) ; IsTrue<True>
mov eax,1
ret
; IsTrue<False>
xor eax,eax
ret Source: |
cartermp
added
Bug
Impact-High
|
@dsyme any idea what's going on here? This one is awful. |
|
I'll take a look (now the labelling is done I'll focus purely on Severity-High bugs) |
|
@dsyme, I've spent a little time on researching this a short while back and the other issues related to struct DU's and at the time, it turned out that the "weird constructor overloads" were created to fix some internal determinism and comments in the PR exist that mark that approach "a temporary fix". I think we can do without them (none of these ctors should be there), which in turn will eventually open the door to improve struct DU's to merge their fields (i.e., overlap etc so they don't occupy unnecessary space). |
KevinRansom
added
Impact-Medium
dsyme
added
Area-Diagnostics
|
Fixed in main now. |
Repro steps
Build this program:
using this .fsproj:
(Targeting
netcoreapp2.1instead ofnet461doesn't make any difference; neither does whether I build as Debug or Release. I didn't play around with specific optimization settings.)Expected behavior
Hong Kong: HongKongshould be printed to the console.Actual behavior
This unhandled exception:
Known workarounds
If you remove any of the DU cases, or remove
[<Struct>], the problem won't appear.Related information / Severity
I know this is kind of an outlandish case; I discovered it while just throwing stuff at a wall during some early performance exploration. But it seemed worth reporting anyway.
VS 15.7.5
Visual F# Tools 10.1 for F# 4.1 15.7.0.0 173513e.
The output of
dotnet --info:The text was updated successfully, but these errors were encountered: