Leniency for generic types with [StructLayout(LayoutKind.Explicit)]

[tagcode]

[StructLayout(LayoutKind.Explicit)] isn't supported for generic types. I think a little bit leniency could be added here, if the field with generic type T is the last field, OR if there is no T field at all and generics is for properties and methods only.

using System.Runtime.InteropServices;

MyStruct<long> myStruct = new MyStruct<long>();
// ^ thrown at run-time: System.TypeLoadException: 'generic types cannot have explicit layout.'

[StructLayout(LayoutKind.Explicit)]
public struct MyStruct<T> where T : struct
{
    [FieldOffset(0)]
    public long Type;
    [FieldOffset(0)]
    public ushort Id;
    [FieldOffset(8)]
    public T Value;
}

[behdad088]

[tagcode]

So it is not iffy after all since MemoryMarshal.AsMemory is implemented with the same assumption.

No, for the CoreCLR team it is obviously not an assumption. Because the CoreCLR team controls/defines both the the memory layout of Memory and ReadOnlyMemory as well as the implementation of MemoryMarshal.AsMemory, and they can change anything regarding these in any way they see fit for any new .NET release.

You don't control either of these, and such changes wouldn't be publicly telegraphed in an obvious manner (you basically would need to continually monitor CoreCLR development to maintain awareness of such changes).

Within the context of already published standard libraries, the layout are not just "assumptions" but written facts.

So you assume that the current status quo regarding the current memory layout and the current implementation of MemoryMarshal.AsMemory won't change. Depending on how long into the future you intend to maintain your project, your initial judgement of it being a slightly-iffy assumption appears quite reasonable.

As per future compability I had already stated: "But atleast this way MS takes responsibility for future compability [of using MemoryMarshal]." expressing that I was aware that future API compability can only go though API call.

After which I though it would be so plainly obvious that it would not needed to be vented any more, but you started assuming what I assumed.

And in practical terms, I seriously doubt that CoreCLR team actually can divert the layout of those two types even if they wanted, because something somewhere will break. Tiniest changes have tendency to propagate all over the place.

So it is not iffy after all since MemoryMarshal.AsMemory is implemented with the same assumption.

But yes, the last word here is poorly phrased. I've been recovering from three days of fewer and I was making typos and having wrong word choices all over the place. Every word must be exact or else somebody flips their mind.

[tagcode]

Conversation gets "white noise" from here on. This doesn't serve any useful purpose, only personal need to bicker about wording and irrelevant points.

[elgonzo]

Conversation gets "white noise" from here on. This doesn't serve any useful purpose, only personal need to bicker about wording and irrelevant points.

Someone invent a mirror, please...

[tagcode]

Conversation gets "white noise" from here on. This doesn't serve any useful purpose, only personal need to bicker about wording and irrelevant points.

Someone invent a mirror, please...

You are there too pal. The point was made for other readers.

[jnm2]

@tagcode I can't think of a way in which it is more safe to have a Memory<T> union member than to have only a ReadOnlyMemory<T> member and to use MemoryMarshal on it. You can access both things without regard to the struct's Type field.

[jnm2]

For the provided example, here's something you can do today:

public struct MyStruct<T> where T : struct
{
    public MyStructHeader Header;
    public T Value;

    [StructLayout(LayoutKind.Explicit)]
    public struct MyStructHeader
    {
        [FieldOffset(0)]
        public long Type;
        [FieldOffset(0)]
        public ushort Id;
    }
}

I see at your #108063 (reply in thread) comment that the union in your actual use case is between two T-dependent fields, so this wouldn't work for that.

[hez2010]

@jkotas @AaronRobinsonMSFT I think we can remove this restriction now? We already have code to validate if the type is blittable, and it will throw another exception during type loading if it's not, so there should be no issue around GC reporting. With generic support in function pointers now I can see some use cases on this.
This can also help us remove some workaround in the BCL like EntryInfo in GenericCache.cs.

I can make a PR for this. While DO note that this restriction comes from CLI - Partition 1, section 9.5 - Generic types shall not be marked explicitlayout, by allowing this we will need to alter the spec as well.

[jkotas]

I do not see a problem with requiring a helper type for overlapping non-generic fields in generic types. I think the current restriction where we are disallowing explicit layout for generics is fine and easy to explain.

[colejohnson66]

If it is to be kept as disallowed, I would argue that the compiler should error (or at least warn) on these situations.

[AaronRobinsonMSFT]

I think the current restriction where we are disallowing explicit layout for generics is fine and easy to explain.

Agree. Changing how this works doesn't seem to offer all that much and creates places where more details about interop are necessary. If this isn't for an interop scenario, then I would argue we should be improving the JIT and type loader so uses of LayoutKind.Explicit aren't necessary.

If it is to be kept as disallowed, I would argue that the compiler should error (or at least warn) on these situations.

@colejohnson66 I'm going to assume you would be okay with this being a new analyzer?

[colejohnson66]

@AaronRobinsonMSFT Yep. An analyzer would probably be fine, but I'd also argue it should be enabled by default. Anything that compiles, but unconditionally throws at runtime should either be illegal to write, or warned about.

[jkotas]

Anything that compiles, but unconditionally throws at runtime

There are many constructs like that. It does not make sense to duplicate all error handling from the runtime and libraries in analyzers so that you get a warning at compile time for these constructs.

Issuing warning at compile time for a construct that unconditionally throw at runtime only makes sense if it is very frequent one.