-
Notifications
You must be signed in to change notification settings - Fork 5.2k
Fix Array.Initialize for value types with default constructors in NativeAOT #121834
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
Fix Array.Initialize for value types with default constructors in NativeAOT #121834
Conversation
- Modified UsageBasedMetadataManager.cs to add metadata dependency for array element type default constructors - Added TestArrayInitialize test to verify Array.Initialize works with value types having parameterless constructors Co-authored-by: MichalStrehovsky <[email protected]>
Removed unused Value field and related checks from ValueTypeWithConstructor.
|
Cc @dotnet/ilc-contrib for review. I'm running into this in #121697 This is a simplified change that simply adds a reflectable default constructor whenever we see an array of valuetypes with a default constructor was allocated. In the limit what we could do instead is add this to dataflow analysis and if the array type on which Array.Initialize was called was known, only enable that specific array type, or enable all arrays if the type wasn't known. It is probably not worth the hassle. rt-sz reports pretty much no regression: MichalStrehovsky/rt-sz#198 |
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 fixes a bug in NativeAOT where Array.Initialize() would silently fail to invoke default constructors on value types when the constructor's reflection metadata wasn't preserved. The runtime's TryGetDefaultConstructorForType API returns IntPtr.Zero when a constructor lacks metadata, causing Initialize() to skip all elements.
Key Changes:
- Added dependency tracking in the compiler to preserve default constructors for array element types that are value types
- Included comprehensive test coverage to validate that
Array.Initialize()correctly invokes constructors
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| UsageBasedMetadataManager.cs | Added logic to preserve default constructor metadata for value type array elements when array type becomes reflectable |
| Reflection.cs | Added test that verifies Array.Initialize() correctly invokes constructors on value type elements using a static counter |
src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/UsageBasedMetadataManager.cs
Outdated
Show resolved
Hide resolved
…etadataManager.cs Co-authored-by: Copilot <[email protected]>
Array.Initialize()silently fails in NativeAOT when the element type's default constructor lacks reflection metadata. The runtime API returnsIntPtr.Zerowhen the constructor isn't preserved, causing Initialize to skip all elements.Changes
UsageBasedMetadataManager.cs: Added dependency tracking for array element type default constructors in
GetMetadataDependenciesDueToReflectability. When an array type becomes reflectable, preserve the element type's default constructor if it's a value type.Test: Added
TestArrayInitializeto verify Array.Initialize invokes constructors correctly. Uses a struct with parameterless constructor that increments a static counter to validate execution.The fix follows the same pattern as existing delegate Invoke method preservation (lines 299-312).
Original prompt
This pull request was created as a result of the following prompt from Copilot chat.
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.