Using the Maroon Stack for real.#43
Conversation
|
(returned with delays, all as expected) |
There was a problem hiding this comment.
Looking at this PR - nice work on the stack machine refactor! This is a solid architectural move toward a more formal execution model.
What I like:
- The type-safe stack values are a big improvement over the previous runtime approach
- Stack variable count validation at compile time is clever
- The Retrn mechanism for proper call/return semantics is well thought out
- Good incremental approach - keeping the old code commented out during transition
Feedback:
- Those println! debug statements should probably be behind a debug flag or use proper logging before this goes to prod
- The commented code is helpful for now but we should plan to clean it up in follow-up PRs
- Minor: unused_validate_states_vec_or_panic function name suggests it should be removed?
Question: Is there a plan to migrate the other task types (Fibonacci, Divisors, etc.) to this new stack model? The current approach of only refactoring Factorial makes sense for proof-of-concept, but curious about the rollout strategy.
The test updates look good and the new execution model is much cleaner. The stack-based approach will definitely make it easier to reason about execution state.
LGTM with the minor cleanup suggestions!
|
Thanks! I'll merge this one for now, and try to find time this weekend to make the next batch of changes. Cool PRs from you too! |
No description provided.