Skip to content

refactor: consolidate RVM instruction variants and clean up VM internals#651

Merged
anakrish merged 1 commit intomicrosoft:mainfrom
anakrish:refactor/consolidate-rvm-instructions
Apr 1, 2026
Merged

refactor: consolidate RVM instruction variants and clean up VM internals#651
anakrish merged 1 commit intomicrosoft:mainfrom
anakrish:refactor/consolidate-rvm-instructions

Conversation

@anakrish
Copy link
Copy Markdown
Collaborator

@anakrish anakrish commented Mar 31, 2026

Merge the three separate Assert* instructions (AssertNot, AssertCondition,
AssertNotUndefined) into a single Guard { register, mode } instruction
with a GuardMode enum. This cuts duplicated match arms across display,
listing, parser, dispatch, and all compiler emit sites.

Drop the unnecessary #[repr(C)] from the Instruction enum. It was never
exposed across FFI, so the C-compatible 4-byte discriminant was pure waste.
Without it Rust picks a 1-byte discriminant, shrinking every instruction
from 8 bytes to 6. A new instruction_size unit test locks this at 6.

While touching these files, also clean up several long-standing issues:

  • Deduplicate the iteration-state setup in loops.rs by extracting a shared
    resolve_iteration_state() helper -- the stack-based and stackless paths
    had near-identical 40-line blocks.
  • Collapse the ExitWithSuccess / ExitWithFailure match arms into one.
  • In rules.rs, stop cloning Arc just to borrow a RuleInfo -- clone
    the small RuleInfo struct directly and extract a get_rule_info() helper.
  • Move the memory check into dispatch (runs per instruction) and remove the
    now-dead enforce_memory_check() entry-point calls.
  • Apply map_or_else style throughout listing.rs for consistency.

Merge the three separate Assert* instructions (AssertNot, AssertCondition,
AssertNotUndefined) into a single `Guard { register, mode }` instruction
with a GuardMode enum. This cuts duplicated match arms across display,
listing, parser, dispatch, and all compiler emit sites.

Drop the unnecessary `#[repr(C)]` from the Instruction enum. It was never
exposed across FFI, so the C-compatible 4-byte discriminant was pure waste.
Without it Rust picks a 1-byte discriminant, shrinking every instruction
from 8 bytes to 6. A new `instruction_size` unit test locks this at 6.

While touching these files, also clean up several long-standing issues:

- Deduplicate the iteration-state setup in loops.rs by extracting a shared
  resolve_iteration_state() helper -- the stack-based and stackless paths
  had near-identical 40-line blocks.
- Collapse the ExitWithSuccess / ExitWithFailure match arms into one.
- In rules.rs, stop cloning Arc<Program> just to borrow a RuleInfo -- clone
  the small RuleInfo struct directly and extract a get_rule_info() helper.
- Move the memory check into dispatch (runs per instruction) and remove the
  now-dead enforce_memory_check() entry-point calls.
- Apply map_or_else style throughout listing.rs for consistency.
@anakrish anakrish force-pushed the refactor/consolidate-rvm-instructions branch from 7708b99 to 31d2555 Compare March 31, 2026 20:23
@anakrish anakrish marked this pull request as ready for review March 31, 2026 20:51
@anakrish anakrish force-pushed the refactor/consolidate-rvm-instructions branch from 5adc9c0 to 31d2555 Compare March 31, 2026 21:28
@anakrish anakrish merged commit 126cc12 into microsoft:main Apr 1, 2026
121 of 122 checks passed
@anakrish anakrish deleted the refactor/consolidate-rvm-instructions branch April 1, 2026 10:34
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.

2 participants