-
Notifications
You must be signed in to change notification settings - Fork 5.1k
Add support for "extended" layouts in the runtimes and add CStruct layout #116082
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
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds support for an “extended” layout flag in the runtimes, introduces the ExtendedLayoutAttribute
and CStruct
layout kind, and wires through layout info across types and metadata emitters.
- Add
IsExtendedLayout
andGetExtendedLayoutInfo
APIs to core metadata types - Implement
ComputeCStructFieldLayout
and dispatch in field layout algorithms - Update metadata headers, ILASM grammar, and reflection emit to recognize
extended
layout
Reviewed Changes
Copilot reviewed 45 out of 45 changed files in this pull request and generated 2 comments.
Show a summary per file
File | Description |
---|---|
src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/CompilerMetadataFieldLayoutAlgorithm.cs | Dispatch ComputeExtendedFieldLayout before explicit layout |
src/coreclr/tools/Common/TypeSystem/Interop/IL/PInvokeDelegateWrapper.cs | Stub out IsExtendedLayout & GetExtendedLayoutInfo |
src/coreclr/tools/Common/TypeSystem/Interop/IL/NativeStructType.cs | Override extended layout members and simplify GetClassLayout |
src/coreclr/tools/Common/TypeSystem/Interop/IL/MarshalUtils.cs | Treat CStruct as blittable |
src/coreclr/tools/Common/TypeSystem/Interop/IL/InlineArrayType.cs | Stub out IsExtendedLayout & GetExtendedLayoutInfo |
src/coreclr/tools/Common/TypeSystem/Ecma/EcmaType.cs | Parse ExtendedLayoutAttribute and implement IsExtendedLayout |
src/coreclr/tools/Common/TypeSystem/Common/TypeWithRepeatedFields.cs | Forward extended layout members |
src/coreclr/tools/Common/TypeSystem/Common/TypeSystemHelpers.cs | Include IsExtendedLayout in HasLayout |
src/coreclr/tools/Common/TypeSystem/Common/MetadataType.cs | Add abstract IsExtendedLayout , GetExtendedLayoutInfo , and supporting types |
src/coreclr/tools/Common/TypeSystem/Common/MetadataFieldLayoutAlgorithm.cs | Implement ComputeCStructFieldLayout and ComputeExtendedFieldLayout |
src/coreclr/tools/Common/TypeSystem/Common/InstantiatedType.Metadata.cs | Override extended layout members |
src/coreclr/tools/Common/TypeSystem/Canon/CanonTypes.Metadata.cs | Stub out extended layout members |
src/coreclr/nativeaot/Runtime.Base/src/System/Runtime/InteropServices/LayoutKind.cs | Add LayoutKind.Extended enum |
src/coreclr/md/compiler/custattr_emit.cpp | Handle tdExtendedLayout in custom attr emitter |
src/coreclr/inc/il_kywd.h | Add IL keyword extended |
src/coreclr/inc/corhdr.h | Define tdExtendedLayout and IsTdExtendedLayout |
src/coreclr/ilasm/prebuilt/asmparse.grammar | Recognize extended in grammar |
src/coreclr/ilasm/asmparse.y | Add EXTENDED_ token and grammar rules |
src/coreclr/System.Private.CoreLib/src/System/Reflection/RuntimeCustomAttributeData.cs | Map ExtendedLayout to LayoutKind.Extended |
src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/RuntimeTypeBuilder.cs | Remove obsolete layout-mask validation |
Comments suppressed due to low confidence (1)
src/coreclr/tools/Common/TypeSystem/Common/MetadataFieldLayoutAlgorithm.cs:502
- There are no unit tests covering the new CStruct layout path (ComputeCStructFieldLayout and ComputeExtendedFieldLayout). Adding tests for valid field offsets, alignment boundaries, and error conditions will help catch regressions.
private static ComputedInstanceFieldLayout ComputeCStructFieldLayout(MetadataType type, int numInstanceFields)
src/coreclr/tools/Common/TypeSystem/Interop/IL/PInvokeDelegateWrapper.cs
Show resolved
Hide resolved
src/tests/Loader/classloader/ExtendedLayout/ExtendedLayoutTypes.il
Outdated
Show resolved
Hide resolved
src/libraries/System.Private.CoreLib/src/System/Reflection/TypeAttributes.cs
Show resolved
Hide resolved
src/libraries/System.Private.CoreLib/src/System/Reflection/TypeAttributes.cs
Show resolved
Hide resolved
Co-authored-by: Jan Kotas <jkotas@microsoft.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What else needs to be updated?
- Ecma-335-Augments.md?
- https://github.com/jbevain/cecil/?
src/coreclr/tools/Common/TypeSystem/Common/InstantiatedType.Metadata.cs
Outdated
Show resolved
Hide resolved
src/coreclr/tools/Common/TypeSystem/Interop/IL/PInvokeDelegateWrapper.cs
Outdated
Show resolved
Hide resolved
src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/ReadyToRunMetadataFieldLayoutAlgorithm.cs
Outdated
Show resolved
Hide resolved
I'll update the Ecma augments doc. It looks like we don't actually need to do any updates on Cecil to ensure correct behavior. Cecil reads layout info directly from the attributes bits, handles masked values right, and doesn't do any validation of valid or invalid values. Cecil could use nice APIs to read if a type is ExtendedLayout, but it isn't required. Based on my tests, ILSpy will need to react to accurately represent the bits in their decompilation, but that's expected. |
This looks good to me otherwise. Is your Roslyn PR on track for .NET 10? We should make sure that either both this PR and Roslyn PR are in .NET 10, or neither. |
I'm waiting on @jaredpar to let me know if Roslyn will have the resources to review the PR over there for this release. |
Add support for the ExtendedLayout flag, the
System.Runtime.InteropServices.ExtendedLayoutAttribute
and theCStruct
layout from #100896.Contributes to #100896
Leaving other layouts from #100896 for later PRs to minimize diffs.