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

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.
This commit is contained in:
Anand Krishnamoorthi
2026-04-01 05:34:33 -05:00
committed by GitHub
parent 1a8fc08773
commit 126cc12eb5
17 changed files with 699 additions and 609 deletions
+27 -16
View File
@@ -10,12 +10,11 @@ pub use params::{
FunctionCallParams, InstructionData, LoopStartParams, ObjectCreateParams, SetCreateParams,
VirtualDataDocumentLookupParams,
};
pub use types::{ComprehensionMode, LiteralOrRegister, LoopMode};
pub use types::{ComprehensionMode, GuardMode, LiteralOrRegister, LoopMode};
use serde::{Deserialize, Serialize};
/// RVM Instructions - simplified enum-based design
#[repr(C)]
#[derive(Debug, Clone, Copy, Serialize, Deserialize)]
pub enum Instruction {
/// Load literal value from literal table into register
@@ -131,8 +130,6 @@ pub enum Instruction {
left: u8,
right: u8,
},
/// Rego negation - produces `true` if operand is `false` or undefined,
/// `false` for any other defined value (including non-booleans).
Not {
dest: u8,
operand: u8,
@@ -251,19 +248,10 @@ pub enum Instruction {
right: u8,
},
/// Assert negation - succeed if operand is false or undefined, fail if true
AssertNot {
operand: u8,
},
/// Assert condition - if register contains false or undefined, return undefined immediately
AssertCondition {
condition: u8,
},
/// Assert not undefined - if register contains undefined, return undefined immediately
AssertNotUndefined {
/// Consolidated guard instruction — replaces AssertNot, AssertCondition, AssertNotUndefined.
Guard {
register: u8,
mode: GuardMode,
},
/// Start a loop over a collection with specified semantics - uses parameter table
@@ -392,3 +380,26 @@ impl Instruction {
Self::ComprehensionEnd {}
}
}
#[cfg(test)]
mod tests {
use super::*;
use core::mem::size_of;
#[test]
fn instruction_size() {
// Lock the instruction size to detect unintended growth.
// Without repr(C), Rust picks a 1-byte discriminant (< 256 variants)
// plus 4 bytes for the largest payload variant + 1 byte alignment
// padding for u16 fields = 6 bytes total.
//
// TODO: Reduce to 4 bytes by making LoopNext zero-payload (both fields
// are redundant with LoopStart params) and moving IndexLiteral to a
// params table.
let size = size_of::<Instruction>();
assert_eq!(
size, 6,
"Instruction size changed from 6 to {size} — review new variants for bloat"
);
}
}