Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Wrong StructLayoutAttribute.Size for struct unions with no data fields #18125

Open
glchapman opened this issue Dec 9, 2024 · 1 comment
Open
Labels
Area-Compiler-CodeGen IlxGen, ilwrite and things at the backend Bug good first issue help wanted Impact-Low (Internal MS Team use only) Describes an issue with limited impact on existing code.
Milestone

Comments

@glchapman
Copy link

Consider this type:

[<Struct>]
type ABC = A | B | C

F# 9 compiles it to a struct with this attribute (among others):

[StructLayout(LayoutKind.Sequential, CharSet = CharSet.Auto, Size = 1)]

However, because of the compiler-generated _tag field, the size is actually 4:

> sizeof<ABC>;;                                                        
val it: int = 4

This seems not to cause any problems, but the documentation states "This field must be equal or greater than the total size, in bytes, of the members of the class or structure," which suggests this should be corrected.

@vzarytovskii
Copy link
Member

vzarytovskii commented Dec 9, 2024

It's probably some flaw in logic around:

if
tycon.AllFieldsArray |> Array.exists (fun f -> not f.IsStatic)
||
// Reflection emit doesn't let us emit 'pack' and 'size' for generic structs.
// In that case we generate a dummy field instead
(cenv.options.workAroundReflectionEmitBugs && not tycon.TyparsNoRange.IsEmpty)
then
ILTypeDefLayout.Sequential { Size = None; Pack = None }, ILDefaultPInvokeEncoding.Ansi
else
ILTypeDefLayout.Sequential { Size = Some 1; Pack = Some 0us }, ILDefaultPInvokeEncoding.Ansi

Should be relatively easy to investigate and test. Should be a good first issue, gonna mark it like this.

@vzarytovskii vzarytovskii added good first issue Area-Compiler-CodeGen IlxGen, ilwrite and things at the backend help wanted labels Dec 9, 2024
@0101 0101 added Impact-Low (Internal MS Team use only) Describes an issue with limited impact on existing code. and removed Needs-Triage labels Dec 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Compiler-CodeGen IlxGen, ilwrite and things at the backend Bug good first issue help wanted Impact-Low (Internal MS Team use only) Describes an issue with limited impact on existing code.
Projects
Status: New
Development

No branches or pull requests

3 participants