Remove StructLayout LayoutKind.Auto from DateTimeOffset

[Tornhoof]

As suggested by @tannergooding in dotnet/csharplang#206 (comment) the [StructLayout(LayoutKind.Auto)] on DateTimeOffset is not necessary and might prevent the wider adoption of the Initial blittable proposal. DateTimeOffset is composed from a DateTime and a short.

This is related to Issue #7719.

[gkhanna79]

@danmosemsft

[tannergooding]

After giving some more thought, this one would actually change behavior on some things since Auto is aware of the surrounding fields with relation to the struct, rather than just the fields in the struct itself.

For example, take the DateTimeOffset struct:

[StructLayout(LayoutKind.Auto)]
struct DateTimeOffset
{
    private DateTime _dateTime;
    private short _offsetMinutes;
}

and take an example struct such as:

// Implicitly [StructLayout(LayoutKind.Sequential)]
struct S
{
    private short _a;
    private short _b;
    private short _c;
    private DateTimeOffset _dateTimeOffset;
}

StructLayout=Auto woudl have sizeof(S) == 16 and the struct is rearranged to be:

[StructLayout(LayoutKind.Sequential)]
struct S
{
    private short _a;
    private short _b;
    private short _c;
    private short _offsetMinutes;
    private DateTime _dateTime;
}

StructLayout=Sequential would have sizeof(S) == 24 and the struct is rearranged to effectively be:

[StructLayout(LayoutKind.Explicit)]
struct S
{
    [FieldOffset(00)] private short _a;
    [FieldOffset(02)] private short _b;
    [FieldOffset(04)] private short _c;
 // [FieldOffset(06)] private short padding;
    [FieldOffset(08)] private DateTime _dateTime;
    [FieldOffset(16)] private short _offsetMinutes;
 // [FieldOffset(18)] private short padding;
 // [FieldOffset(20)] private short padding;
 // [FieldOffset(22)] private short padding;
}

[svick]

@tannergooding That's not what I see. Running this code on .Net Core 1.1, I see sizeof(S) is always 24. And the disassembly of the ToTuple methods is exactly the same, which shows that the fields have the same offsets:

; ToTuple(AutoS)
sub         rsp,18h  
xor         eax,eax  
lea         r8,[rsp+8]  
xorpd       xmm0,xmm0  
movdqu      xmmword ptr [r8],xmm0  
movsx       rax,word ptr [rdx]  
movsx       r8,word ptr [rdx+2]  
movsx       r9,word ptr [rdx+4]  
mov         r10,qword ptr [rdx+10h]  
movsx       rdx,word ptr [rdx+8]  
mov         word ptr [rsp+8],ax  
mov         word ptr [rsp+0Ah],r8w  
mov         word ptr [rsp+0Ch],r9w  
lea         rax,[rsp+10h]  
mov         qword ptr [rax],r10  
mov         word ptr [rsp+0Eh],dx  
movdqu      xmm0,xmmword ptr [rsp+8]  
movdqu      xmmword ptr [rcx],xmm0  
mov         rax,rcx  
add         rsp,18h  
ret  

; ToTuple(SequentialS)
sub         rsp,18h  
xor         eax,eax  
lea         r8,[rsp+8]  
xorpd       xmm0,xmm0  
movdqu      xmmword ptr [r8],xmm0  
movsx       rax,word ptr [rdx]  
movsx       r8,word ptr [rdx+2]  
movsx       r9,word ptr [rdx+4]  
mov         r10,qword ptr [rdx+10h]  
movsx       rdx,word ptr [rdx+8]  
mov         word ptr [rsp+8],ax  
mov         word ptr [rsp+0Ah],r8w  
mov         word ptr [rsp+0Ch],r9w  
lea         rax,[rsp+10h]  
mov         qword ptr [rax],r10  
mov         word ptr [rsp+0Eh],dx  
movdqu      xmm0,xmmword ptr [rsp+8]  
movdqu      xmmword ptr [rcx],xmm0  
mov         rax,rcx  
add         rsp,18h  
ret  

Also, how would accessing the _dateTimeOffset by ref work?

[tannergooding]

@svick, it appears that netcore and netfx have different behaviors here 😄

[svick]

@tannergooding I see the same behavior on .Net Framework 4.6.2.

[danmoseley]

@tannergooding @svick did it become clear whether this and #7719 should actually be done or not?

[svick]

@danmosemsft I'm not sure I'm the right person to ask, but:

• I haven't seen any evidence this would cause a break or inefficiency.
• It will become useful when/if blittable makes it into C# (in version 7.3?).

So, I think it makes sense to do it.

[danmoseley]

@jkotas do you see any issue with removing Auto from DateTime and DateTimeOffset?

[jkotas]

We should understand whether it is a potential breaking change for interop or not. It is not clear to me from the above whether it is the case.

[GSPP]

Are DateTime and DateTimeOffset even supposed to be blittable? That would expose their internals.