-
Notifications
You must be signed in to change notification settings - Fork 5.2k
Disallow inline arrays with explicit size #117578
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
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 disallows inline arrays annotated with an explicit size by enforcing runtime checks and updating related resources and tests.
- Added new loader IL and C# tests for explicit-size inline arrays
- Implemented explicit-size checks in both Mono (
class-init.c) and CoreCLR (methodtablebuilder.cpp,MetadataFieldLayoutAlgorithm.cs) - Introduced new resource strings and exception IDs for inline-array explicit-size errors
Reviewed Changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/tests/Loader/classloader/InlineArray/InvalidCSharpInlineArray.il | Added IL test for explicit-size inline array |
| src/tests/Loader/classloader/InlineArray/InlineArrayInvalid.csproj | Set test priority for invalid inline array tests |
| src/tests/Loader/classloader/InlineArray/InlineArrayInvalid.cs | Added ExplicitSize_Fails Xunit test |
| src/mono/mono/metadata/class-init.c | Added explicit-size inline-array failure callback |
| src/libraries/System.Private.CoreLib/src/Resources/Strings.resx | Added resource string for explicit-size inline-array error |
| src/coreclr/vm/methodtablebuilder.cpp | Throw exception for explicit-size inline-array in method table |
| src/coreclr/tools/Common/TypeSystem/Common/Properties/Resources.resx | Added resource for type-system inline-array explicit-size |
| src/coreclr/tools/Common/TypeSystem/Common/MetadataFieldLayoutAlgorithm.cs | Checked for explicit-size inline-array during layout computation |
| src/coreclr/tools/Common/TypeSystem/Common/ExceptionStringID.cs | Added exception string ID for inline-array explicit-size |
| src/coreclr/nativeaot/System.Private.CoreLib/src/Internal/Runtime/TypeLoaderExceptionHelper.cs | Mapped TypeSystem exception to resource string |
| src/coreclr/dlls/mscorrc/resource.h | Defined new resource ID for explicit-size inline-array error |
| src/coreclr/dlls/mscorrc/mscorrc.rc | Added message text for explicit-size inline-array error |
|
Does this need to be marked as a breaking change? |
This can break some existing program (if such exist), so technically yes. |
|
Added When you commit this breaking change:
Tagging @dotnet/compat for awareness of the breaking change. |
|
/ba-g cryptography failures on musl, wasm timing out |
|
Thanks! |
|
Breaking change issue - dotnet/docs#47435 |
|
Removing |
Fixes: #110962
Specifying
.sizeon an InlineArray struct only brings confusion to whether it applies to the element or to thr entire instance. It Either result is a fairly niche scenario and can be achieved via defining wrapping structs.Notes:
.sizeis disallowed in eithersequentialorautolayouts..packis allowed. Unlike.size, there is no ambiguity with.pack- it has no effect in structs with all fields having the same type, regardless of 1 or N fields.