Feature/allocate strings on heap#33
Merged
Merged
Conversation
Refactored `NativeMemberDispatcher` and `VirtualMachine` to use `HeapManager` for heap-allocated strings and objects. Deprecated direct string handling in `Value.cs` and transitioned to heap-based methods. Added helper methods for heap management and string concatenation. Enabled parallel test execution in `MSTestSettings.cs`. Updated `Lumi.slnx` to adjust build configuration for tests. Added a TODO in `ConstantPool.cs` for dead constant removal.
Removed the `String` property and related methods from the `Value` struct to transition to heap-allocated strings. Updated `NativeMemberDispatcher` to handle this change and throw appropriate errors for mismatched value kinds. Adjusted `ConstantToValue` to reflect the removal of direct string handling.
There was a problem hiding this comment.
Pull request overview
This PR moves runtime string values to heap-allocated HeapStringObject handles (via interning), and updates VM execution and stdlib native dispatch to work with heap-backed strings.
Changes:
- VM: push string constants as heap strings; build concatenation results as heap strings.
- VM: refactor heap allocations behind a helper (
AllocateHeapObject). - Stdlib native dispatcher: return/read/accept heap-allocated strings for File prelude APIs; enable parallel test execution for StdLib tests.
Reviewed changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| Lumi.VM/VirtualMachine.cs | Push constants/concats as heap strings; refactor heap allocations; adds string helpers. |
| Lumi.VM/Value.cs | Removes inline string payload from Value and updates constant/value printing logic. |
| Lumi.VM/NativeMemberDispatcher.cs | Updates File prelude methods to consume/produce heap strings and adjusts argument extraction. |
| Lumi.StdLib.Tests/MSTestSettings.cs | Enables method-level MSTest parallelization for StdLib tests. |
| Lumi.slnx | Changes solution build behavior for Lumi.VM.Heap.Tests in Debug configuration. |
| Lumi.Bytecode/Constants/ConstantPool.cs | Adds a TODO note about dead-constant cleanup after optimizations. |
Comments suppressed due to low confidence (1)
Lumi.VM/Value.cs:65
Value.PrintValue()no longer handles heap-allocated string values (they are nowValueKind.HeapObject). This means composite heap objects that print nestedValues viaValue.PrintValue()(e.g.,HeapArrayObject.PrintValue()/HeapStructObject.PrintValue()) will now throwUnkownValueKind(HeapObject)when they contain strings (or any other heap values).
ValueKind.Boolean => Bool.ToString(),
ValueKind.Null => "null",
ValueKind.Undefined => "undefined",
_ => throw VirtualMachineError.UnkownValueKind(Kind),
};
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Extracted root enumeration to a Roots method for reuse. Updated string concatenation and heap allocation to use precise roots during garbage collection, improving clarity and maintainability.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.