[WIP] Add TypeLayout API - #84738
Conversation
|
Azure Pipelines: Successfully started running 2 pipeline(s). There may be pipelines that require an authorized user to comment /azp run to run. |
dotnet-policy-service
Bot
added
Community
|
This PR modifies public API files. Please follow the instructions at https://github.com/dotnet/roslyn/blob/main/docs/contributing/API%20Review%20Process.md for ensuring all public APIs are reviewed before merging. |
| DeclarationModifiers modifiers, | ||
| string name, | ||
| SpecialType specialType, | ||
| #if !OLDER_ROSLYN |
There was a problem hiding this comment.
For this file & the next 2 also, idk if this is the correct way to do this at all - please let me know what is if not. It has a few issues - defined all over the place, and all types for CodeGenerationNamedTypeSymbol get auto layout by default w/ no overrides done, since the ctor is not accessible (I was going to make it use auto for non-structs and sequential for structs to get it mostly correct, but doesn't seem to be possible to do that).
| return Hash.Combine(Hash.Combine(this.Size, this.Alignment), _kind); | ||
| return Hash.Combine(Hash.Combine(this.Size, this.PackingSize), _kind); | ||
| } | ||
| } |
There was a problem hiding this comment.
Seems a bit odd to not have == and !=, but they were not approved in the API review - please advise if you want me to add them.
Additionally, we are not exposing all type layout info (which based on the comment for API review, seems like was the intention), in particular field offsets are not available anywhere, which seems like another potential API review oversight - please advise for this also :)
|
/cc @333fred |
There was a problem hiding this comment.
Pull request overview
This PR introduces a new public Roslyn API for exposing type layout metadata via INamedTypeSymbol.TypeLayout, backed by a public TypeLayout struct, and updates emit/NoPia paths plus related wrappers and tests to use the new packing-size naming.
Changes:
- Adds
INamedTypeSymbol.TypeLayoutand makesTypeLayouta public struct (renamingAlignmenttoPackingSize). - Plumbs
TypeLayoutthrough compiler public-model wrappers and workspace code-generation symbols (withOLDER_ROSLYNguards). - Updates NoPia-related code/tests to use
PackingSize, and adds new layout-focused compiler tests.
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Workspaces/SharedUtilitiesAndExtensions/Workspace/Core/CodeGeneration/Symbols/CodeGenerationNamedTypeSymbol.cs | Adds TypeLayout ctor plumbing (guarded by !OLDER_ROSLYN) and preserves compatibility ctor. |
| src/Workspaces/SharedUtilitiesAndExtensions/Workspace/Core/CodeGeneration/Symbols/CodeGenerationConstructedNamedTypeSymbol.cs | Forwards TypeLayout to the base symbol (guarded by !OLDER_ROSLYN). |
| src/Workspaces/SharedUtilitiesAndExtensions/Workspace/Core/CodeGeneration/Symbols/CodeGenerationAbstractNamedTypeSymbol.cs | Stores/exposes TypeLayout for codegen named types under !OLDER_ROSLYN. |
| src/Features/Core/Portable/MetadataAsSource/AbstractMetadataAsSourceService.WrappedNamedTypeSymbol.cs | Forwards TypeLayout through metadata-as-source wrapper. |
| src/Compilers/VisualBasic/Test/Symbol/SymbolsTests/Source/TypeTests.vb | Adds a TypeLayouts test; also adds imports (one appears unused). |
| src/Compilers/VisualBasic/Test/Emit/Emit/NoPiaEmbedTypes.vb | Updates assertions from Alignment to PackingSize. |
| src/Compilers/VisualBasic/Portable/Symbols/NamedTypeSymbol.vb | Implements INamedTypeSymbol.TypeLayout via existing Layout property. |
| src/Compilers/VisualBasic/Portable/Emit/NamedTypeSymbolAdapter.vb | Switches emitted “alignment” value to PackingSize. |
| src/Compilers/CSharp/Test/Symbol/Symbols/TypeTests.cs | Adds a TypeLayouts test (currently uses a non-existent [TypeLayout] attribute in the test source string). |
| src/Compilers/CSharp/Test/Emit/Emit/NoPiaEmbedTypes.cs | Updates assertions from Alignment to PackingSize. |
| src/Compilers/CSharp/Portable/Symbols/PublicModel/NamedTypeSymbol.cs | Adds explicit public-model implementation of INamedTypeSymbol.TypeLayout. |
| src/Compilers/CSharp/Portable/Emitter/Model/NamedTypeSymbolAdapter.cs | Switches emitted “alignment” value to PackingSize. |
| src/Compilers/Core/Portable/Symbols/TypeLayout.cs | Makes TypeLayout public; renames storage/property to PackingSize; makes the non-default ctor internal. |
| src/Compilers/Core/Portable/Symbols/INamedTypeSymbol.cs | Adds new public TypeLayout TypeLayout { get; } member. |
| src/Compilers/Core/Portable/PublicAPI.Unshipped.txt | Declares the new public API surface. |
| src/Compilers/Core/Portable/Emit/NoPia/CommonEmbeddedType.cs | Switches Alignment emission to PackingSize. |
This is really annoying btw, that one run has immediately used ~30% of my monthly student/personal credits.
|
Seems to have no effect at runtime, but it is encoded into the IL/dll.
| case TypeAttributes.AutoLayout: | ||
| return default(TypeLayout); | ||
| kind = LayoutKind.Auto; | ||
| break; |
There was a problem hiding this comment.
Do we want this to not read the IL info like it was before I changed it? Seems current compiler will happily emit these into the IL/dll: https://godbolt.org/z/f78se4aP4. And runtime will happily run with it too, just ignores it: https://godbolt.org/z/EWzfo7f51.
Fixes #76928
Microsoft Reviewers: Open in CodeFlow