Skip to content

Replace heap-allocated Stack with fixed-size array#7

Merged
garyschulte merged 2 commits into10d9e:mainfrom
daniellehrner:feat/stack-refactor
Feb 6, 2026
Merged

Replace heap-allocated Stack with fixed-size array#7
garyschulte merged 2 commits into10d9e:mainfrom
daniellehrner:feat/stack-refactor

Conversation

@daniellehrner
Copy link
Copy Markdown
Collaborator

  • Replace ?std.ArrayList(U256) with [STACK_LIMIT]U256 fixed array + length: usize,
    eliminating heap allocation and deinit() calls.
  • Add unsafe methods (pushUnsafe, popUnsafe, peekUnsafe, topMutUnsafe, dupUnsafe, swapUnsafe, shrinkUnsafe) for opcode implementations.
  • Add 30 inline tests in stack.zig using idiomatic zig test blocks.

The idea is that opcodes follow the pattern, example for ADD : peek-peek-shrink-overwrite. This reduces the stack mutations to a minum.

Replace `?std.ArrayList(U256)` with `[STACK_LIMIT]U256` fixed array + `length: usize`,
eliminating heap allocation and deinit() calls. Add unsafe methods (pushUnsafe, popUnsafe,
peekUnsafe, topMutUnsafe, dupUnsafe, swapUnsafe, shrinkUnsafe) for opcode implementations.
Opcode pattern: peek-peek-shrink-overwrite via peekUnsafe + shrinkUnsafe + topMutUnsafe.
Add 30 inline tests in stack.zig using idiomatic zig test blocks.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@daniellehrner
Copy link
Copy Markdown
Collaborator Author

CC @garyschulte

Copy link
Copy Markdown
Collaborator

@garyschulte garyschulte left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed sized stack LGTM, where are we intending to use the unsafe methods?

Comment on lines +332 to +341
const interpreter_tests = b.addTest(.{
.root_module = b.createModule(.{
.root_source_file = .{ .src_path = .{ .owner = b, .sub_path = "src/interpreter/stack.zig" } },
.target = target,
.optimize = optimize,
}),
});
interpreter_tests.root_module.addImport("primitives", primitives_module);
const run_interpreter_tests = b.addRunArtifact(interpreter_tests);
test_step.dependOn(&run_interpreter_tests.step);
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we add this to test_exe instead?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@10d9e is there a strong reason to use test_exe versus build addTest ?

@garyschulte garyschulte merged commit 7311c4e into 10d9e:main Feb 6, 2026
5 checks passed
garyschulte added a commit to garyschulte/zevm that referenced this pull request Mar 11, 2026
Signed-off-by: garyschulte <garyschulte@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants