From 90faca4dc248bf975b3da84918c7d14ed2049f8b Mon Sep 17 00:00:00 2001 From: Anand Krishnamoorthi Date: Sun, 8 Nov 2026 10:56:41 -0600 Subject: [PATCH 1/9] Fix RVM match and user-call definedness boundaries Require defined ignored and unused values and correct structural kinds. Keep soft matches local and restore skipped root-register availability; preserve negated relational output evaluation. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 84271ec2-b466-4cfe-bf58-848f4dceb842 --- docs/rvm/architecture.md | 9 + src/languages/rego/compiler/destructuring.rs | 288 +++++++++++------- src/languages/rego/compiler/function_calls.rs | 57 +++- tests/rvm/compiler.rs | 225 ++++++++++++++ 4 files changed, 458 insertions(+), 121 deletions(-) diff --git a/docs/rvm/architecture.md b/docs/rvm/architecture.md index cfe1c149a..78a843ada 100644 --- a/docs/rvm/architecture.md +++ b/docs/rvm/architecture.md @@ -51,6 +51,15 @@ its own soft matching mode; leaving the query restores the caller's mode and inherited input/data root-register availability. Loads in a rejected query, including its yield, are not assumed to dominate subsequent expressions. +Destructuring requires defined values, including ignored and unused bindings. +Array patterns require arrays of the matching length; object patterns require +objects but allow extra fields. Under `not`, a mismatch produces a local failed +match rather than failing the enclosing body. User-call inputs are all evaluated +before checking definedness; an undefined input skips invocation and output +matching. Builtin strictness is unchanged. +Negated relational calls evaluate binding-free expected outputs before equality +comparison; wildcard output patterns still use local matching. + Keyed membership (`key, value in collection`) checks the value at that specific object key or array index, not whether the value appears elsewhere. Missing or invalid keys produce `false`, as do keyed membership checks on sets and scalar diff --git a/src/languages/rego/compiler/destructuring.rs b/src/languages/rego/compiler/destructuring.rs index 372d3b7ae..3f2e2c933 100644 --- a/src/languages/rego/compiler/destructuring.rs +++ b/src/languages/rego/compiler/destructuring.rs @@ -7,8 +7,9 @@ use crate::compiler::destructuring_planner::plans::{ AssignmentPlan, BindingPlan, DestructuringPlan, WildcardSide, }; use crate::lexer::Span; -use crate::rvm::instructions::{GuardMode, Instruction}; +use crate::rvm::instructions::{BuiltinCallParams, GuardMode, Instruction}; use crate::value::Value; +use alloc::vec::Vec; use anyhow::{bail, Result}; #[derive(Clone, Copy, Debug)] @@ -20,19 +21,106 @@ pub enum PlanContext { SomeIn, } -impl PlanContext { - fn require_defined_values(self) -> bool { - matches!( - self, - PlanContext::Assignment - | PlanContext::FunctionParameter - | PlanContext::LoopIndex - | PlanContext::SomeIn - ) +impl<'a> Compiler<'a> { + pub(super) fn emit_match_failure_branch( + &mut self, + register: Register, + mode: GuardMode, + span: &Span, + ) -> Result { + let scratch = self.alloc_register()?; + let operand = if matches!(mode, GuardMode::NotUndefined) { + self.emit_instruction( + Instruction::Eq { + dest: scratch, + left: register, + right: register, + }, + span, + ); + scratch + } else { + register + }; + self.emit_instruction( + Instruction::Not { + dest: scratch, + operand, + }, + span, + ); + let branch = self.program.instructions.len(); + self.emit_instruction( + Instruction::AnyOfNext { + check: scratch, + result: scratch, + end_pc: 0, + }, + span, + ); + Ok(branch) + } + + pub(super) fn patch_match_failure_branches(&mut self, branches: &[usize]) -> Result<()> { + let end_pc = self + .program + .instructions + .len() + .checked_sub(1) + .and_then(|pc| u16::try_from(pc).ok()) + .ok_or_else(|| { + anyhow::anyhow!("match branch target exceeds the RVM instruction limit") + })?; + for &branch in branches { + let Some(Instruction::AnyOfNext { end_pc: target, .. }) = + self.program.instructions.get_mut(branch) + else { + bail!("match branch instruction is missing or invalid"); + }; + *target = end_pc; + } + Ok(()) + } + + fn emit_match_check( + &mut self, + register: Register, + mode: GuardMode, + span: &Span, + failures: &mut Vec, + ) -> Result<()> { + if self.soft_assert_mode { + failures.push(self.emit_match_failure_branch(register, mode, span)?); + } else { + self.emit_instruction(Instruction::Guard { register, mode }, span); + } + Ok(()) + } + + fn with_match_result(&mut self, span: &Span, f: F) -> Result> + where + F: FnOnce(&mut Self, &mut Vec) -> Result<()>, + { + let result_register = if self.soft_assert_mode { + Some(self.load_bool_literal(false, span)?) + } else { + None + }; + let saved_roots = (self.current_input_register, self.current_data_register); + let mut failures = Vec::new(); + let result = f(self, &mut failures); + if result_register.is_some() { + // A failed match skips expressions whose root loads do not dominate the join. + (self.current_input_register, self.current_data_register) = saved_roots; + } + result?; + if let Some(dest) = result_register { + self.emit_instruction(Instruction::LoadBool { dest, value: true }, span); + self.patch_match_failure_branches(&failures)?; + } + Ok(result_register) } -} -impl<'a> Compiler<'a> { pub fn compile_assignment_plan_using_hoisted_destructuring( &mut self, plan: &AssignmentPlan, @@ -55,38 +143,40 @@ impl<'a> Compiler<'a> { rhs_expr, lhs_plan, .. } => { let rhs_reg = self.compile_rego_expr_with_span(rhs_expr, rhs_expr.span(), false)?; - let _ = self.apply_destructuring_plan( + let result = self.apply_destructuring_plan( lhs_plan, rhs_reg, span, PlanContext::Assignment, )?; - self.load_bool_literal(true, span) + result.map_or_else(|| self.load_bool_literal(true, span), Ok) } AssignmentPlan::EqualsBindRight { lhs_expr, rhs_plan, .. } => { let lhs_reg = self.compile_rego_expr_with_span(lhs_expr, lhs_expr.span(), false)?; - let _ = self.apply_destructuring_plan( + let result = self.apply_destructuring_plan( rhs_plan, lhs_reg, span, PlanContext::Assignment, )?; - self.load_bool_literal(true, span) + result.map_or_else(|| self.load_bool_literal(true, span), Ok) } AssignmentPlan::EqualsBothSides { element_pairs, .. } => { - for (value_expr, value_plan) in element_pairs { - let value_reg = - self.compile_rego_expr_with_span(value_expr, value_expr.span(), false)?; - let _ = self.apply_destructuring_plan( - value_plan, - value_reg, - span, - PlanContext::Assignment, - )?; - } - self.load_bool_literal(true, span) + let result = self.with_match_result(span, |compiler, failures| { + for (value_expr, value_plan) in element_pairs { + let value_reg = compiler.compile_rego_expr_with_span( + value_expr, + value_expr.span(), + false, + )?; + compiler + .apply_destructuring_plan_impl(value_plan, value_reg, span, failures)?; + } + Ok(()) + })?; + result.map_or_else(|| self.load_bool_literal(true, span), Ok) } AssignmentPlan::EqualityCheck { lhs_expr, rhs_expr } => { let lhs_reg = self.compile_rego_expr_with_span(lhs_expr, lhs_expr.span(), false)?; @@ -120,26 +210,18 @@ impl<'a> Compiler<'a> { WildcardSide::Lhs => { let rhs_reg = self.compile_rego_expr_with_span(rhs_expr, rhs_expr.span(), false)?; - self.emit_instruction( - Instruction::Guard { - register: rhs_reg, - mode: GuardMode::NotUndefined, - }, - span, - ); - self.load_bool_literal(true, span) + let result = self.with_match_result(span, |compiler, failures| { + compiler.emit_match_check(rhs_reg, GuardMode::NotUndefined, span, failures) + })?; + result.map_or_else(|| self.load_bool_literal(true, span), Ok) } WildcardSide::Rhs => { let lhs_reg = self.compile_rego_expr_with_span(lhs_expr, lhs_expr.span(), false)?; - self.emit_instruction( - Instruction::Guard { - register: lhs_reg, - mode: GuardMode::NotUndefined, - }, - span, - ); - self.load_bool_literal(true, span) + let result = self.with_match_result(span, |compiler, failures| { + compiler.emit_match_check(lhs_reg, GuardMode::NotUndefined, span, failures) + })?; + result.map_or_else(|| self.load_bool_literal(true, span), Ok) } }, } @@ -204,11 +286,29 @@ impl<'a> Compiler<'a> { plan: &DestructuringPlan, value_register: Register, span: &Span, - context: PlanContext, + _context: PlanContext, ) -> Result> { + self.with_match_result(span, |compiler, failures| { + compiler.apply_destructuring_plan_impl(plan, value_register, span, failures) + }) + } + + fn apply_destructuring_plan_impl( + &mut self, + plan: &DestructuringPlan, + value_register: Register, + span: &Span, + failures: &mut Vec, + ) -> Result<()> { + if !matches!( + plan, + DestructuringPlan::Array { .. } | DestructuringPlan::Object { .. } + ) { + self.emit_match_check(value_register, GuardMode::NotUndefined, span, failures)?; + } match plan { DestructuringPlan::Var(name_span) => { - self.bind_variable(name_span, value_register, span, context)?; + self.bind_variable(name_span, value_register, span)?; } DestructuringPlan::Ignore => {} DestructuringPlan::EqualityExpr(expected_expr) => { @@ -223,16 +323,7 @@ impl<'a> Compiler<'a> { }, span, ); - if self.soft_assert_mode { - return Ok(Some(cmp_reg)); - } - self.emit_instruction( - Instruction::Guard { - register: cmp_reg, - mode: GuardMode::Condition, - }, - span, - ); + self.emit_match_check(cmp_reg, GuardMode::Condition, span, failures)?; } DestructuringPlan::EqualityValue(expected_value) => { let expected_reg = self.load_literal_value(expected_value, span)?; @@ -245,19 +336,11 @@ impl<'a> Compiler<'a> { }, span, ); - if self.soft_assert_mode { - return Ok(Some(cmp_reg)); - } - self.emit_instruction( - Instruction::Guard { - register: cmp_reg, - mode: GuardMode::Condition, - }, - span, - ); + self.emit_match_check(cmp_reg, GuardMode::Condition, span, failures)?; } DestructuringPlan::Array { element_plans } => { - self.assert_array_length(value_register, element_plans.len(), span)?; + self.assert_collection_kind(value_register, "is_array", span, failures)?; + self.assert_array_length(value_register, element_plans.len(), span, failures)?; for (index, element_plan) in element_plans.iter().enumerate() { let literal_idx = self.add_literal(Value::from(index)); let element_reg = self.alloc_register()?; @@ -269,23 +352,14 @@ impl<'a> Compiler<'a> { }, span, ); - if context.require_defined_values() { - self.emit_instruction( - Instruction::Guard { - register: element_reg, - mode: GuardMode::NotUndefined, - }, - span, - ); - } - let _ = - self.apply_destructuring_plan(element_plan, element_reg, span, context)?; + self.apply_destructuring_plan_impl(element_plan, element_reg, span, failures)?; } } DestructuringPlan::Object { field_plans, dynamic_fields, } => { + self.assert_collection_kind(value_register, "is_object", span, failures)?; for (key, field_plan) in field_plans { let literal_idx = self.add_literal(key.clone()); let field_reg = self.alloc_register()?; @@ -297,14 +371,7 @@ impl<'a> Compiler<'a> { }, span, ); - self.emit_instruction( - Instruction::Guard { - register: field_reg, - mode: GuardMode::NotUndefined, - }, - span, - ); - let _ = self.apply_destructuring_plan(field_plan, field_reg, span, context)?; + self.apply_destructuring_plan_impl(field_plan, field_reg, span, failures)?; } for (key_expr, field_plan) in dynamic_fields { @@ -319,18 +386,11 @@ impl<'a> Compiler<'a> { }, span, ); - self.emit_instruction( - Instruction::Guard { - register: field_reg, - mode: GuardMode::NotUndefined, - }, - span, - ); - let _ = self.apply_destructuring_plan(field_plan, field_reg, span, context)?; + self.apply_destructuring_plan_impl(field_plan, field_reg, span, failures)?; } } } - Ok(None) + Ok(()) } fn bind_variable( @@ -338,7 +398,6 @@ impl<'a> Compiler<'a> { name_span: &Span, value_register: Register, span: &Span, - context: PlanContext, ) -> Result<()> { let var_name = name_span.text(); if var_name == "_" { @@ -359,16 +418,6 @@ impl<'a> Compiler<'a> { ); self.add_variable(var_name, dest); - if context.require_defined_values() { - self.emit_instruction( - Instruction::Guard { - register: dest, - mode: GuardMode::NotUndefined, - }, - span, - ); - } - Ok(()) } @@ -390,6 +439,7 @@ impl<'a> Compiler<'a> { array_register: Register, expected_length: usize, span: &Span, + failures: &mut Vec, ) -> Result<()> { let expected_literal = self.add_literal(Value::from(expected_length)); let actual_len_reg = self.alloc_register()?; @@ -419,13 +469,25 @@ impl<'a> Compiler<'a> { }, span, ); - self.emit_instruction( - Instruction::Guard { - register: cmp_reg, - mode: GuardMode::Condition, - }, - span, - ); - Ok(()) + self.emit_match_check(cmp_reg, GuardMode::Condition, span, failures) + } + + fn assert_collection_kind( + &mut self, + register: Register, + builtin: &str, + span: &Span, + failures: &mut Vec, + ) -> Result<()> { + let dest = self.alloc_register()?; + let builtin_index = self.get_builtin_index(builtin)?; + let params_index = self.program.add_builtin_call_params(BuiltinCallParams { + dest, + builtin_index, + num_args: 1, + args: [register, 0, 0, 0, 0, 0, 0, 0], + }); + self.emit_instruction(Instruction::BuiltinCall { params_index }, span); + self.emit_match_check(dest, GuardMode::Condition, span, failures) } } diff --git a/src/languages/rego/compiler/function_calls.rs b/src/languages/rego/compiler/function_calls.rs index 0c9c0b1a2..d46e155c4 100644 --- a/src/languages/rego/compiler/function_calls.rs +++ b/src/languages/rego/compiler/function_calls.rs @@ -14,7 +14,9 @@ use crate::ast::ExprRef; use crate::builtins; use crate::compiler::destructuring_planner::plans::BindingPlan; use crate::lexer::Span; -use crate::rvm::instructions::{ArrayCreateParams, BuiltinCallParams, FunctionCallParams}; +use crate::rvm::instructions::{ + ArrayCreateParams, BuiltinCallParams, FunctionCallParams, GuardMode, +}; use crate::rvm::Instruction; use crate::utils::{append_path_component, get_path_string, split_canonical_path_root}; use crate::value::Value; @@ -149,6 +151,20 @@ impl<'a> Compiler<'a> { self.ensure_register_capacity(3)?; } let dest = self.alloc_register()?; + let mut input_failure_branches = Vec::new(); + let input_boundary_roots = (self.current_input_register, self.current_data_register); + if matches!(&call_target, CallTarget::User { .. }) && !arg_regs.is_empty() { + let literal_idx = self.add_literal(Value::Undefined); + self.emit_instruction(Instruction::Load { dest, literal_idx }, &span); + // Evaluate all inputs first, then skip invocation and output matching + // on Undefined without failing the caller's enclosing condition. + for ®ister in &arg_regs { + input_failure_branches.push( + self.emit_match_failure_branch(register, GuardMode::NotUndefined, &span) + .map_err(|err| CompilerError::from(err).at(&span))?, + ); + } + } match call_target { CallTarget::User { rule_index, .. } => { @@ -340,19 +356,39 @@ impl<'a> Compiler<'a> { let saved_input_register = self.current_input_register; let saved_data_register = self.current_data_register; if let Some((plan, plan_span)) = &out_param_plan { - let plan_result = self - .apply_binding_plan(plan, dest, plan_span) - .map_err(|err| CompilerError::from(err).at(plan_span))?; - if let Some(result_reg) = plan_result { + let compare_output = self.soft_assert_mode + && matches!(plan, BindingPlan::Parameter { destructuring_plan, .. } + if !destructuring_plan.introduces_binding()); + if compare_output { + // Negated calls compare an evaluated output expression; they do + // not bind a pattern or skip later operands on an early mismatch. + let expected_expr = params.last().ok_or_else(|| CompilerError::General { + message: "missing relational output expression".to_string(), + })?; + let expected = self.compile_rego_expr_with_span(expected_expr, plan_span, false)?; self.emit_instruction( - Instruction::Move { + Instruction::Eq { dest, - src: result_reg, + left: dest, + right: expected, }, &span, ); } else { - self.emit_instruction(Instruction::LoadBool { dest, value: true }, &span); + let plan_result = self + .apply_binding_plan(plan, dest, plan_span) + .map_err(|err| CompilerError::from(err).at(plan_span))?; + if let Some(result_reg) = plan_result { + self.emit_instruction( + Instruction::Move { + dest, + src: result_reg, + }, + &span, + ); + } else { + self.emit_instruction(Instruction::LoadBool { dest, value: true }, &span); + } } } @@ -394,6 +430,11 @@ impl<'a> Compiler<'a> { } } + if !input_failure_branches.is_empty() { + (self.current_input_register, self.current_data_register) = input_boundary_roots; + self.patch_match_failure_branches(&input_failure_branches) + .map_err(|err| CompilerError::from(err).at(&span))?; + } Ok(dest) } diff --git a/tests/rvm/compiler.rs b/tests/rvm/compiler.rs index d2c600d93..eacb779bf 100644 --- a/tests/rvm/compiler.rs +++ b/tests/rvm/compiler.rs @@ -10,6 +10,231 @@ use regorus::rvm::Instruction; use regorus::{Engine, Rc, Value}; use std::collections::BTreeSet; +#[test] +fn match_boundaries_match_interpreter() -> anyhow::Result<()> { + let cases = [ + ("", "x := input.missing", r#"{}"#, false), + ("", "_ := input.missing", r#"{}"#, false), + ("", "x := false", r#"{}"#, true), + ("", "x := null", r#"{}"#, true), + ("", "x := []", r#"{}"#, true), + ("", "count(input.missing, _)", r#"{}"#, false), + ("f(_) := true", "f(input.missing)", r#"{}"#, false), + ("f(_) := true", "f(false)", r#"{}"#, true), + ("f(_) := true", "f(null)", r#"{}"#, true), + ( + "", + "{0: x} = input.payload; x == 7", + r#"{"payload":[7]}"#, + false, + ), + ( + "", + "[x] = input.payload; x == 7", + r#"{"payload":{"0":7}}"#, + false, + ), + ("", "y := {0: 7}; [x] = y; x == 7", r#"{}"#, false), + ( + "", + "[x] = input.payload; x == 7", + r#"{"payload":[7]}"#, + true, + ), + ( + "", + r#"{"a": x} = input.payload; x == 7"#, + r#"{"payload":{"a":7,"b":8}}"#, + true, + ), + ( + "f({}) := true", + "f(input.payload)", + r#"{"payload":[]}"#, + false, + ), + ( + "f({}) := true", + "f(input.payload)", + r#"{"payload":{"a":7}}"#, + true, + ), + ("", "not _ = input.missing", r#"{}"#, true), + ("f(_) := true", "not f(input.missing)", r#"{}"#, true), + ("", "not _ = false", r#"{}"#, false), + ("", "not _ = null", r#"{}"#, false), + ("", "not {} = input.payload", r#"{"payload":[]}"#, true), + ("", "not [x] = input.payload", r#"{"payload":[]}"#, true), + ("", r#"not split("a,b", ",", ["a"])"#, r#"{}"#, true), + ("", r#"not split("a,b", ",", ["wrong","b"])"#, r#"{}"#, true), + ("", r#"not split("a,b", ",", ["a","b"])"#, r#"{}"#, false), + ("", "not [_, 1] = [7, 2]", r#"{}"#, true), + ("", "not [_, 1] = [7, 1]", r#"{}"#, false), + ("", "[x, y] := [false, null]", r#"{}"#, true), + ( + "", + r#"{"a": _} = input.payload"#, + r#"{"payload":{}}"#, + false, + ), + ( + "", + r#"not {"a": [_]} = input.payload"#, + r#"{"payload":{"a":{}}}"#, + true, + ), + ( + "", + r#"not split("a,b", ",", [input.x]); input.x == 7"#, + r#"{"x":7}"#, + true, + ), + ( + "", + r#"not split("a,b", ",", ["wrong", input.x]); input.x == 7"#, + r#"{"x":7}"#, + true, + ), + ( + "", + r#"not split("a,b", ",", ["wrong", data.x]); data.x == 7"#, + r#"{}"#, + true, + ), + ( + "f(_, _) := true", + "not f(input.missing, input.x); input.x == 7", + r#"{"x":7}"#, + true, + ), + ( + "", + "input.payload[0] == 7; count(input.payload) == 1", + r#"{"payload":[7]}"#, + true, + ), + ("", "_ = _", r#"{}"#, true), + ]; + let mut divergences = Vec::new(); + for (definitions, body, input, want) in cases { + for strict in [false, true] { + let mut engine = Engine::new(); + engine.set_strict_builtin_errors(strict); + engine.add_policy( + "test.rego".into(), + format!("package test\nimport rego.v1\n{definitions}\ndefault p := false\np if {{ {body} }}"), + )?; + let input = Value::from_json_str(input)?; + engine.set_input(input.clone()); + let data = Value::from_json_str(r#"{"x":7}"#)?; + engine.add_data(data.clone())?; + let expected = Value::Bool(want); + assert_eq!(engine.eval_rule("data.test.p".into())?, expected, "{body}"); + let compiled = engine.compile_with_entrypoint(&Rc::from("data.test.p"))?; + let program = Compiler::compile_from_policy(&compiled, &["data.test.p"])?; + for mode in [ExecutionMode::RunToCompletion, ExecutionMode::Suspendable] { + let mut vm = RegoVM::new(); + vm.set_strict_builtin_errors(strict); + vm.set_execution_mode(mode); + vm.load_program(program.clone()); + vm.set_data(data.clone())?; + vm.set_input(input.clone()); + let got = vm.execute_entry_point_by_name("data.test.p")?; + if mode == ExecutionMode::Suspendable { + assert_eq!( + vm.execution_state(), + &ExecutionState::Completed { + result: got.clone() + } + ); + } + if got != expected { + divergences.push(format!( + "{body}: {mode:?}, strict={strict}: {got:?} != {expected:?}" + )); + } + } + } + } + assert!(divergences.is_empty(), "{}", divergences.join("\n")); + Ok(()) +} + +#[test] +fn undefined_user_input_skips_invocation_and_preserves_value_heads() -> anyhow::Result<()> { + for (definition, input, expected) in [ + ("f(_) := true", r#"{}"#, Value::Undefined), + ("f(_) := 1 / 0", r#"{}"#, Value::Undefined), + ("f(x) := x", r#"{"x":false}"#, Value::Bool(false)), + ("f(x) := x", r#"{"x":null}"#, Value::Null), + ("f(x) := x", r#"{"x":[]}"#, Value::new_array()), + ] { + for strict in [false, true] { + let mut engine = Engine::new(); + engine.set_strict_builtin_errors(strict); + engine.add_policy( + "test.rego".into(), + format!("package test\nimport rego.v1\n{definition}\np := f(input.x)"), + )?; + let input = Value::from_json_str(input)?; + engine.set_input(input.clone()); + assert_eq!(engine.eval_rule("data.test.p".into())?, expected); + let compiled = engine.compile_with_entrypoint(&Rc::from("data.test.p"))?; + let program = Compiler::compile_from_policy(&compiled, &["data.test.p"])?; + for mode in [ExecutionMode::RunToCompletion, ExecutionMode::Suspendable] { + let mut vm = RegoVM::new(); + vm.set_strict_builtin_errors(strict); + vm.set_execution_mode(mode); + vm.load_program(program.clone()); + vm.set_input(input.clone()); + let got = vm.execute_entry_point_by_name("data.test.p")?; + assert_eq!(got, expected, "{definition}, {mode:?}, strict={strict}"); + if mode == ExecutionMode::Suspendable { + assert_eq!( + vm.execution_state(), + &ExecutionState::Completed { result: got } + ); + } + } + } + } + Ok(()) +} + +#[test] +fn match_boundaries_preserve_strict_error_evaluation() -> anyhow::Result<()> { + let mut outcomes = Vec::new(); + for body in [ + "p := 1 / 0", + "p := f(input.missing, 1 / 0)", + r#"p if { not split("a,b", ",", ["wrong", 1 / 0]) }"#, + ] { + let mut engine = Engine::new(); + engine.set_strict_builtin_errors(true); + engine.add_policy( + "test.rego".into(), + format!("package test\nimport rego.v1\nf(_, _) := true\n{body}"), + )?; + assert!(engine.eval_rule("data.test.p".into()).is_err(), "{body}"); + let compiled = engine.compile_with_entrypoint(&Rc::from("data.test.p"))?; + let program = Compiler::compile_from_policy(&compiled, &["data.test.p"])?; + let mut modes = Vec::new(); + for mode in [ExecutionMode::RunToCompletion, ExecutionMode::Suspendable] { + let mut vm = RegoVM::new(); + vm.set_strict_builtin_errors(true); + vm.set_execution_mode(mode); + vm.load_program(program.clone()); + modes.push(observe_strict_probe( + vm.execute_entry_point_by_name("data.test.p"), + )); + } + outcomes.push(modes); + } + assert_eq!(outcomes[1], outcomes[0]); + assert_eq!(outcomes[2], outcomes[0]); + Ok(()) +} + /// Compile a single-rule Rego module and return the program. fn compile_rule(module: &str) -> std::sync::Arc { let mut engine = Engine::new(); From dbccfb6c8024e2bbaf05e606c2ab0f991596a6c8 Mon Sep 17 00:00:00 2001 From: Anand Krishnamoorthi Date: Sun, 8 Nov 2026 12:14:48 -0600 Subject: [PATCH 2/9] fix(rvm): preserve skipped-call evaluation and match capacity Join undefined-input failures before negated expected outputs, retain actual failed inputs across program serialization, and reuse sequential match scratch registers. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 84271ec2-b466-4cfe-bf58-848f4dceb842 --- docs/rvm/architecture.md | 6 +- src/languages/rego/compiler/destructuring.rs | 10 +- src/languages/rego/compiler/function_calls.rs | 20 ++- src/languages/rego/compiler/mod.rs | 2 + src/languages/rego/compiler/rules.rs | 1 + tests/rvm/compiler.rs | 150 +++++++++++++++++- 6 files changed, 177 insertions(+), 12 deletions(-) diff --git a/docs/rvm/architecture.md b/docs/rvm/architecture.md index 78a843ada..1031c80d2 100644 --- a/docs/rvm/architecture.md +++ b/docs/rvm/architecture.md @@ -58,7 +58,11 @@ match rather than failing the enclosing body. User-call inputs are all evaluated before checking definedness; an undefined input skips invocation and output matching. Builtin strictness is unchanged. Negated relational calls evaluate binding-free expected outputs before equality -comparison; wildcard output patterns still use local matching. +comparison even when undefined inputs skip invocation; wildcard output patterns +still use local matching. +Sequential failure checks share a temporary register, separate from the match +result. Skipped user calls retain their actual undefined input rather than a +serialized undefined literal. Keyed membership (`key, value in collection`) checks the value at that specific object key or array index, not whether the value appears elsewhere. Missing or diff --git a/src/languages/rego/compiler/destructuring.rs b/src/languages/rego/compiler/destructuring.rs index 3f2e2c933..13d9aece1 100644 --- a/src/languages/rego/compiler/destructuring.rs +++ b/src/languages/rego/compiler/destructuring.rs @@ -28,7 +28,15 @@ impl<'a> Compiler<'a> { mode: GuardMode, span: &Span, ) -> Result { - let scratch = self.alloc_register()?; + // AnyOfNext consumes this temporary before another check can overwrite it. + let scratch = match self.match_scratch_register { + Some(scratch_register) => scratch_register, + None => { + let scratch_register = self.alloc_register()?; + self.match_scratch_register = Some(scratch_register); + scratch_register + } + }; let operand = if matches!(mode, GuardMode::NotUndefined) { self.emit_instruction( Instruction::Eq { diff --git a/src/languages/rego/compiler/function_calls.rs b/src/languages/rego/compiler/function_calls.rs index d46e155c4..e0772e837 100644 --- a/src/languages/rego/compiler/function_calls.rs +++ b/src/languages/rego/compiler/function_calls.rs @@ -154,11 +154,16 @@ impl<'a> Compiler<'a> { let mut input_failure_branches = Vec::new(); let input_boundary_roots = (self.current_input_register, self.current_data_register); if matches!(&call_target, CallTarget::User { .. }) && !arg_regs.is_empty() { - let literal_idx = self.add_literal(Value::Undefined); - self.emit_instruction(Instruction::Load { dest, literal_idx }, &span); - // Evaluate all inputs first, then skip invocation and output matching - // on Undefined without failing the caller's enclosing condition. + // Undefined skips invocation and binding-pattern matching, but not + // a negated call's binding-free expected-output expression. for ®ister in &arg_regs { + self.emit_instruction( + Instruction::Move { + dest, + src: register, + }, + &span, + ); input_failure_branches.push( self.emit_match_failure_branch(register, GuardMode::NotUndefined, &span) .map_err(|err| CompilerError::from(err).at(&span))?, @@ -360,6 +365,13 @@ impl<'a> Compiler<'a> { && matches!(plan, BindingPlan::Parameter { destructuring_plan, .. } if !destructuring_plan.introduces_binding()); if compare_output { + if !input_failure_branches.is_empty() { + (self.current_input_register, self.current_data_register) = + input_boundary_roots; + self.patch_match_failure_branches(&input_failure_branches) + .map_err(|err| CompilerError::from(err).at(&span))?; + input_failure_branches.clear(); + } // Negated calls compare an evaluated output expression; they do // not bind a pattern or skip later operands on an early mismatch. let expected_expr = params.last().ok_or_else(|| CompilerError::General { diff --git a/src/languages/rego/compiler/mod.rs b/src/languages/rego/compiler/mod.rs index cb775e22a..c1fddf2eb 100644 --- a/src/languages/rego/compiler/mod.rs +++ b/src/languages/rego/compiler/mod.rs @@ -139,6 +139,7 @@ pub struct Compiler<'a> { builtin_index_map: BTreeMap, current_input_register: Option, current_data_register: Option, + match_scratch_register: Option, current_rule_path: String, current_call_stack: Vec, entry_points: IndexMap, @@ -178,6 +179,7 @@ impl<'a> Compiler<'a> { builtin_index_map: BTreeMap::new(), current_input_register: None, current_data_register: None, + match_scratch_register: None, current_rule_path: String::new(), current_call_stack: Vec::new(), entry_points: IndexMap::new(), diff --git a/src/languages/rego/compiler/rules.rs b/src/languages/rego/compiler/rules.rs index f02d8e251..5cd228bf1 100644 --- a/src/languages/rego/compiler/rules.rs +++ b/src/languages/rego/compiler/rules.rs @@ -448,6 +448,7 @@ impl<'a> Compiler<'a> { if let Rule::Spec { head, bodies, span } = rule_ref.as_ref() { self.push_scope(); self.register_counter = 0; + self.match_scratch_register = None; let result_register = self.alloc_register()?; diff --git a/tests/rvm/compiler.rs b/tests/rvm/compiler.rs index eacb779bf..071db50f9 100644 --- a/tests/rvm/compiler.rs +++ b/tests/rvm/compiler.rs @@ -61,6 +61,16 @@ fn match_boundaries_match_interpreter() -> anyhow::Result<()> { ), ("", "not _ = input.missing", r#"{}"#, true), ("f(_) := true", "not f(input.missing)", r#"{}"#, true), + ("f(_) := true", "not f(input.missing, 7)", r#"{}"#, true), + ("f(_) := true", "f(input.missing, 1 / 0)", r#"{}"#, false), + ("f(_) := 1 / 0", "not f(input.missing)", r#"{}"#, true), + ("f(_) := true", "f(input.missing, _)", r#"{}"#, false), + ( + "f(_) := true", + "not f(input.missing, data.x); data.x == 7", + r#"{}"#, + true, + ), ("", "not _ = false", r#"{}"#, false), ("", "not _ = null", r#"{}"#, false), ("", "not {} = input.payload", r#"{"payload":[]}"#, true), @@ -201,6 +211,125 @@ fn undefined_user_input_skips_invocation_and_preserves_value_heads() -> anyhow:: Ok(()) } +#[test] +fn wide_soft_match_and_successive_calls_remain_compilable() -> anyhow::Result<()> { + let wildcards = vec!["_"; 128].join(","); + let ones = vec!["1"; 128].join(","); + for (definitions, body, want) in [ + ("", format!("not [{wildcards}] = [{ones}]"), false), + ("f(_) := true", vec!["f(1)"; 100].join(";"), true), + ] { + let mut engine = Engine::new(); + engine.add_policy( + "test.rego".into(), + format!("package test\nimport rego.v1\n{definitions}\ndefault p := false\np if {{ {body} }}"), + )?; + let expected = Value::Bool(want); + assert_eq!(engine.eval_rule("data.test.p".into())?, expected); + let compiled = engine.compile_with_entrypoint(&Rc::from("data.test.p"))?; + let program = Compiler::compile_from_policy(&compiled, &["data.test.p"])?; + for mode in [ExecutionMode::RunToCompletion, ExecutionMode::Suspendable] { + let mut vm = RegoVM::new(); + vm.set_execution_mode(mode); + vm.load_program(program.clone()); + let got = vm.execute_entry_point_by_name("data.test.p")?; + assert_eq!(got, expected, "{mode:?}"); + if mode == ExecutionMode::Suspendable { + assert_eq!( + vm.execution_state(), + &ExecutionState::Completed { result: got } + ); + } + } + } + Ok(()) +} + +#[test] +fn undefined_user_input_survives_program_json_round_trip() -> anyhow::Result<()> { + let input = Value::from_json_str(r#"{"x":7}"#)?; + for args in [ + "input.missing", + "input.missing, input.x", + "input.x, input.missing", + ] { + let parameters = if args.contains(',') { "_, _" } else { "_" }; + let mut engine = Engine::new(); + engine.set_input(input.clone()); + engine.add_policy( + "test.rego".into(), + format!("package test\nimport rego.v1\nf({parameters}) := true\ndefault p := false\np if {{ f({args}) }}"), + )?; + assert_eq!(engine.eval_rule("data.test.p".into())?, Value::Bool(false)); + let compiled = engine.compile_with_entrypoint(&Rc::from("data.test.p"))?; + let program = Compiler::compile_from_policy(&compiled, &["data.test.p"])?; + let json = program.serialize_json().map_err(anyhow::Error::msg)?; + let round_trip = Rc::new(Program::deserialize_json(&json).map_err(anyhow::Error::msg)?); + for program in [program, round_trip] { + for mode in [ExecutionMode::RunToCompletion, ExecutionMode::Suspendable] { + let mut vm = RegoVM::new(); + vm.set_execution_mode(mode); + vm.load_program(program.clone()); + vm.set_input(input.clone()); + let got = vm.execute_entry_point_by_name("data.test.p")?; + assert_eq!(got, Value::Bool(false), "{args}, {mode:?}"); + if mode == ExecutionMode::Suspendable { + assert_eq!( + vm.execution_state(), + &ExecutionState::Completed { result: got } + ); + } + } + } + } + Ok(()) +} + +#[test] +fn undefined_user_input_preserves_negated_output_evaluation() -> anyhow::Result<()> { + for strict in [true, false] { + let mut outcomes = Vec::new(); + let control = if strict { + "not input.missing == 1 / 0" + } else { + "not f(input.missing, input.missing)" + }; + for body in [control, "not f(input.missing, 1 / 0)"] { + let mut engine = Engine::new(); + engine.set_strict_builtin_errors(strict); + engine.add_policy( + "test.rego".into(), + format!( + "package test\nimport rego.v1\nf(_) := true\ndefault p := false\np if {{ {body} }}" + ), + )?; + let input = Value::from_json_str("{}")?; + engine.set_input(input.clone()); + let interpreted = engine.eval_rule("data.test.p".into()); + if strict { + assert!(interpreted.is_err(), "{body}"); + } else { + assert_eq!(interpreted?, Value::Bool(false), "{body}"); + } + let compiled = engine.compile_with_entrypoint(&Rc::from("data.test.p"))?; + let program = Compiler::compile_from_policy(&compiled, &["data.test.p"])?; + let mut modes = Vec::new(); + for mode in [ExecutionMode::RunToCompletion, ExecutionMode::Suspendable] { + let mut vm = RegoVM::new(); + vm.set_strict_builtin_errors(strict); + vm.set_execution_mode(mode); + vm.load_program(program.clone()); + vm.set_input(input.clone()); + let result = vm.execute_entry_point_by_name("data.test.p"); + modes.push(observe_strict_probe(&vm, result)); + } + outcomes.push(modes); + } + assert_eq!(outcomes[1], outcomes[0], "strict={strict}"); + } + Ok(()) +} + #[test] fn match_boundaries_preserve_strict_error_evaluation() -> anyhow::Result<()> { let mut outcomes = Vec::new(); @@ -224,9 +353,8 @@ fn match_boundaries_preserve_strict_error_evaluation() -> anyhow::Result<()> { vm.set_strict_builtin_errors(true); vm.set_execution_mode(mode); vm.load_program(program.clone()); - modes.push(observe_strict_probe( - vm.execute_entry_point_by_name("data.test.p"), - )); + let result = vm.execute_entry_point_by_name("data.test.p"); + modes.push(observe_strict_probe(&vm, result)); } outcomes.push(modes); } @@ -892,7 +1020,17 @@ enum StrictProbeOutcome { DivisionError, } -fn observe_strict_probe(result: Result) -> StrictProbeOutcome { +fn observe_strict_probe(vm: &RegoVM, result: Result) -> StrictProbeOutcome { + if vm.get_execution_mode() == ExecutionMode::Suspendable { + if let Ok(value) = &result { + assert_eq!( + vm.execution_state(), + &ExecutionState::Completed { + result: value.clone() + } + ); + } + } match result { Ok(value) => StrictProbeOutcome::Value(value), Err(VmError::InvalidDivision { .. }) => StrictProbeOutcome::DivisionError, @@ -1073,7 +1211,7 @@ fn registered_host_await_strict_output_division_matches_explicit_control_in_both )); vm.resume(Some(Value::from("first-response"))) }; - let first_outcome = observe_strict_probe(first_result); + let first_outcome = observe_strict_probe(&vm, first_result); let second_result = if is_run_to_completion { vm.execute_entry_point_by_index(1) @@ -1099,7 +1237,7 @@ fn registered_host_await_strict_output_division_matches_explicit_control_in_both vm.resume(Some(Value::from("second-response"))) }; - Ok((first_outcome, observe_strict_probe(second_result))) + Ok((first_outcome, observe_strict_probe(&vm, second_result))) }; let mut division_program = Program::new(); From df1048ed3649836d0a85ffbc3a813b5e0221cd51 Mon Sep 17 00:00:00 2001 From: Anand Krishnamoorthi Date: Sun, 8 Nov 2026 13:22:32 -0600 Subject: [PATCH 3/9] test(rvm): cover nested match and negation lifetimes Exercise four-level output structures, shared scratch across nested call checks, skipped expected outputs, and supported nested query negations. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 84271ec2-b466-4cfe-bf58-848f4dceb842 --- tests/rvm/compiler.rs | 62 ++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 61 insertions(+), 1 deletion(-) diff --git a/tests/rvm/compiler.rs b/tests/rvm/compiler.rs index 071db50f9..fed45c6e0 100644 --- a/tests/rvm/compiler.rs +++ b/tests/rvm/compiler.rs @@ -117,6 +117,61 @@ fn match_boundaries_match_interpreter() -> anyhow::Result<()> { r#"{"x":7}"#, true, ), + ( + r#"f(_) := {"a":[{"b":[7,1]}]}"#, + r#"not f(1, {"a":[{"b":[7,2]}]})"#, + r#"{}"#, + true, + ), + ( + r#"f(_) := {"a":[{"b":[7,1]}]}"#, + r#"not f(1, {"a":[{"b":[7,1]}]})"#, + r#"{}"#, + false, + ), + ( + "", + r#"not {"a":[{"b":[_,data.x]}]} = input.payload; data.x == 7"#, + r#"{"payload":{"a":[{"b":[9,2]}]}}"#, + true, + ), + ( + "", + r#"not {"a":[{"b":[_,data.x]}]} = input.payload"#, + r#"{"payload":{"a":[{"b":[9,7]}]}}"#, + false, + ), + ( + "", + r#"not {"a":[{"b":[_,data.x]}]} = input.payload; data.x == 7"#, + r#"{"payload":{"a":[]}}"#, + true, + ), + ( + "f(_) := 1", + "not [_, f(1)] = [7,2]; data.x == 7", + r#"{}"#, + true, + ), + ("f(_) := 1", "not [_, f(1)] = [7,1]", r#"{}"#, false), + ( + "f(_) := 1", + "not [_, f(input.missing)] = [7,2]; data.x == 7", + r#"{}"#, + true, + ), + ( + "", + "not [1 | not [1 | not 1 = input.x] == [1]] == [1]", + r#"{"x":2}"#, + true, + ), + ( + "", + "not [1 | every y in [input.x] { not [1 | not 1 = y] == [1] }] == [1]", + r#"{"x":1}"#, + false, + ), ( "", "input.payload[0] == 7; count(input.payload) == 1", @@ -294,7 +349,11 @@ fn undefined_user_input_preserves_negated_output_evaluation() -> anyhow::Result< } else { "not f(input.missing, input.missing)" }; - for body in [control, "not f(input.missing, 1 / 0)"] { + for body in [ + control, + "not f(input.missing, 1 / 0)", + r#"not f(input.missing, {"a":[{"b":[1 / 0]}]})"#, + ] { let mut engine = Engine::new(); engine.set_strict_builtin_errors(strict); engine.add_policy( @@ -326,6 +385,7 @@ fn undefined_user_input_preserves_negated_output_evaluation() -> anyhow::Result< outcomes.push(modes); } assert_eq!(outcomes[1], outcomes[0], "strict={strict}"); + assert_eq!(outcomes[2], outcomes[0], "strict={strict}"); } Ok(()) } From be34c2693ad36a8a43cc9c75f966e26f552c999f Mon Sep 17 00:00:00 2001 From: Anand Krishnamoorthi Date: Mon, 9 Nov 2026 16:34:24 -0600 Subject: [PATCH 4/9] fix(rvm): reuse match predicates and preserve negated output effects Reuse collection-kind predicate storage within each rule window and evaluate wildcard expected-output prefixes independently of the actual result shape. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 84271ec2-b466-4cfe-bf58-848f4dceb842 --- docs/rvm/architecture.md | 14 ++-- src/languages/rego/compiler/destructuring.rs | 22 +++-- src/languages/rego/compiler/function_calls.rs | 72 +++++++++++++--- tests/rvm/compiler.rs | 83 ++++++++++++++++++- 4 files changed, 162 insertions(+), 29 deletions(-) diff --git a/docs/rvm/architecture.md b/docs/rvm/architecture.md index 1031c80d2..954a4e63a 100644 --- a/docs/rvm/architecture.md +++ b/docs/rvm/architecture.md @@ -58,11 +58,15 @@ match rather than failing the enclosing body. User-call inputs are all evaluated before checking definedness; an undefined input skips invocation and output matching. Builtin strictness is unchanged. Negated relational calls evaluate binding-free expected outputs before equality -comparison even when undefined inputs skip invocation; wildcard output patterns -still use local matching. -Sequential failure checks share a temporary register, separate from the match -result. Skipped user calls retain their actual undefined input rather than a -serialized undefined literal. +comparison even when undefined inputs skip invocation. Wildcard outputs evaluate +the prefix before the first wildcard in expression order, stopping when an operand +is undefined; a structural mismatch in the call result does not suppress those +effects. Reaching an unbound wildcard in the interpreter still reports an +unsafe-variable error; the VM does not reproduce that existing error. +Collection-kind checks and sequential failure checks share a temporary register +per rule-definition window, separate from preserved match results. Skipped user +calls retain their actual undefined input rather than a serialized undefined +literal. Keyed membership (`key, value in collection`) checks the value at that specific object key or array index, not whether the value appears elsewhere. Missing or diff --git a/src/languages/rego/compiler/destructuring.rs b/src/languages/rego/compiler/destructuring.rs index 13d9aece1..d59f29568 100644 --- a/src/languages/rego/compiler/destructuring.rs +++ b/src/languages/rego/compiler/destructuring.rs @@ -22,6 +22,17 @@ pub enum PlanContext { } impl<'a> Compiler<'a> { + fn match_scratch_register(&mut self) -> Result { + match self.match_scratch_register { + Some(scratch_register) => Ok(scratch_register), + None => { + let scratch_register = self.alloc_register()?; + self.match_scratch_register = Some(scratch_register); + Ok(scratch_register) + } + } + } + pub(super) fn emit_match_failure_branch( &mut self, register: Register, @@ -29,14 +40,7 @@ impl<'a> Compiler<'a> { span: &Span, ) -> Result { // AnyOfNext consumes this temporary before another check can overwrite it. - let scratch = match self.match_scratch_register { - Some(scratch_register) => scratch_register, - None => { - let scratch_register = self.alloc_register()?; - self.match_scratch_register = Some(scratch_register); - scratch_register - } - }; + let scratch = self.match_scratch_register()?; let operand = if matches!(mode, GuardMode::NotUndefined) { self.emit_instruction( Instruction::Eq { @@ -487,7 +491,7 @@ impl<'a> Compiler<'a> { span: &Span, failures: &mut Vec, ) -> Result<()> { - let dest = self.alloc_register()?; + let dest = self.match_scratch_register()?; let builtin_index = self.get_builtin_index(builtin)?; let params_index = self.program.add_builtin_call_params(BuiltinCallParams { dest, diff --git a/src/languages/rego/compiler/function_calls.rs b/src/languages/rego/compiler/function_calls.rs index e0772e837..5e425aeea 100644 --- a/src/languages/rego/compiler/function_calls.rs +++ b/src/languages/rego/compiler/function_calls.rs @@ -10,7 +10,7 @@ )] use super::{Compiler, CompilerError, Register, Result}; -use crate::ast::ExprRef; +use crate::ast::{Expr, ExprRef}; use crate::builtins; use crate::compiler::destructuring_planner::plans::BindingPlan; use crate::lexer::Span; @@ -363,7 +363,7 @@ impl<'a> Compiler<'a> { if let Some((plan, plan_span)) = &out_param_plan { let compare_output = self.soft_assert_mode && matches!(plan, BindingPlan::Parameter { destructuring_plan, .. } - if !destructuring_plan.introduces_binding()); + if destructuring_plan.bound_vars().is_empty()); if compare_output { if !input_failure_branches.is_empty() { (self.current_input_register, self.current_data_register) = @@ -377,15 +377,28 @@ impl<'a> Compiler<'a> { let expected_expr = params.last().ok_or_else(|| CompilerError::General { message: "missing relational output expression".to_string(), })?; - let expected = self.compile_rego_expr_with_span(expected_expr, plan_span, false)?; - self.emit_instruction( - Instruction::Eq { - dest, - left: dest, - right: expected, - }, - &span, - ); + let has_wildcards = matches!(plan, BindingPlan::Parameter { destructuring_plan, .. } + if destructuring_plan.contains_wildcards()); + if has_wildcards { + self.emit_instruction(Instruction::LoadBool { dest, value: false }, &span); + let roots = (self.current_input_register, self.current_data_register); + let mut failures = Vec::new(); + self.compile_negated_output_prefix(expected_expr, &mut failures)?; + self.patch_match_failure_branches(&failures) + .map_err(|err| CompilerError::from(err).at(&span))?; + (self.current_input_register, self.current_data_register) = roots; + } else { + let expected = + self.compile_rego_expr_with_span(expected_expr, plan_span, false)?; + self.emit_instruction( + Instruction::Eq { + dest, + left: dest, + right: expected, + }, + &span, + ); + } } else { let plan_result = self .apply_binding_plan(plan, dest, plan_span) @@ -450,6 +463,43 @@ impl<'a> Compiler<'a> { Ok(dest) } + // A wildcard cannot produce a defined expected output. Evaluate only the + // prefix before it, in expression order, rather than matching the call's + // result (whose shape or earlier mismatches must not suppress these effects). + fn compile_negated_output_prefix( + &mut self, + expr: &ExprRef, + failures: &mut Vec, + ) -> Result { + match expr.as_ref() { + Expr::Var { span, .. } if span.text() == "_" => return Ok(false), + Expr::Array { items, .. } | Expr::Set { items, .. } => { + for item in items { + if !self.compile_negated_output_prefix(item, failures)? { + return Ok(false); + } + } + } + Expr::Object { fields, .. } => { + for (_, key, value) in fields { + if !self.compile_negated_output_prefix(key, failures)? + || !self.compile_negated_output_prefix(value, failures)? + { + return Ok(false); + } + } + } + _ => { + let register = self.compile_rego_expr_with_span(expr, expr.span(), false)?; + failures.push( + self.emit_match_failure_branch(register, GuardMode::NotUndefined, expr.span()) + .map_err(|err| CompilerError::from(err).at(expr.span()))?, + ); + } + } + Ok(true) + } + /// Rewrite an import-aliased call path to its target, e.g. `b.f(1)` to /// `data.a.b.f` after `import data.a.b`. Resolves only when the target is /// a known function (the `rules` map cannot be used here: it also indexes diff --git a/tests/rvm/compiler.rs b/tests/rvm/compiler.rs index fed45c6e0..25f83477b 100644 --- a/tests/rvm/compiler.rs +++ b/tests/rvm/compiler.rs @@ -73,6 +73,18 @@ fn match_boundaries_match_interpreter() -> anyhow::Result<()> { ), ("", "not _ = false", r#"{}"#, false), ("", "not _ = null", r#"{}"#, false), + ( + "", + r#"not split("a,b,c", ",", [input.missing, 1 / 0, _]); input.x == 7"#, + r#"{"x":7}"#, + true, + ), + ( + "", + r#"not split("a,b,c", ",", {input.missing: [1 / 0, _]}); data.x == 7"#, + r#"{}"#, + true, + ), ("", "not {} = input.payload", r#"{"payload":[]}"#, true), ("", "not [x] = input.payload", r#"{"payload":[]}"#, true), ("", r#"not split("a,b", ",", ["a"])"#, r#"{}"#, true), @@ -270,9 +282,26 @@ fn undefined_user_input_skips_invocation_and_preserves_value_heads() -> anyhow:: fn wide_soft_match_and_successive_calls_remain_compilable() -> anyhow::Result<()> { let wildcards = vec!["_"; 128].join(","); let ones = vec!["1"; 128].join(","); + let fields = (0..43) + .map(|i| format!(r#""k{i}":[_]"#)) + .collect::>() + .join(","); + let values = (0..43) + .map(|i| format!(r#""k{i}":[false]"#)) + .collect::>() + .join(","); for (definitions, body, want) in [ - ("", format!("not [{wildcards}] = [{ones}]"), false), - ("f(_) := true", vec!["f(1)"; 100].join(";"), true), + ( + String::new(), + format!("not [{wildcards}] = [{ones}]"), + false, + ), + ("f(_) := true".into(), vec!["f(1)"; 100].join(";"), true), + ( + format!("f({{{fields}}}) := true"), + format!("f({{{values}}});\nf({{{values}}})"), + true, + ), ] { let mut engine = Engine::new(); engine.add_policy( @@ -397,6 +426,8 @@ fn match_boundaries_preserve_strict_error_evaluation() -> anyhow::Result<()> { "p := 1 / 0", "p := f(input.missing, 1 / 0)", r#"p if { not split("a,b", ",", ["wrong", 1 / 0]) }"#, + r#"p if { not split("a,b,c", ",", ["wrong", 1 / 0, _]) }"#, + r#"p if { not split("a,b,c", ",", {"z": [1 / 0, _]}) }"#, ] { let mut engine = Engine::new(); engine.set_strict_builtin_errors(true); @@ -418,8 +449,52 @@ fn match_boundaries_preserve_strict_error_evaluation() -> anyhow::Result<()> { } outcomes.push(modes); } - assert_eq!(outcomes[1], outcomes[0]); - assert_eq!(outcomes[2], outcomes[0]); + for outcome in &outcomes[1..] { + assert_eq!(*outcome, outcomes[0]); + } + Ok(()) +} + +#[test] +fn negated_wildcard_output_preserves_non_strict_undefined_prefix() -> anyhow::Result<()> { + for body in [ + r#"not split("a,b,c", ",", ["wrong", 1 / 0, _]); input.x == 7"#, + r#"not split("a,b,c", ",", {"z":[1 / 0, _]}); input.x == 7"#, + r#"not f(1, [1 / 0, _]); input.x == 7"#, + ] { + let mut engine = Engine::new(); + engine.set_strict_builtin_errors(false); + engine.add_policy( + "test.rego".into(), + format!( + "package test\nimport rego.v1\nf(_) := true\ndefault p := false\np if {{ {body} }}" + ), + )?; + let input = Value::from_json_str(r#"{"x":7}"#)?; + engine.set_input(input.clone()); + assert_eq!( + engine.eval_rule("data.test.p".into())?, + Value::Bool(true), + "{body}" + ); + let compiled = engine.compile_with_entrypoint(&Rc::from("data.test.p"))?; + let program = Compiler::compile_from_policy(&compiled, &["data.test.p"])?; + for mode in [ExecutionMode::RunToCompletion, ExecutionMode::Suspendable] { + let mut vm = RegoVM::new(); + vm.set_strict_builtin_errors(false); + vm.set_execution_mode(mode); + vm.load_program(program.clone()); + vm.set_input(input.clone()); + let got = vm.execute_entry_point_by_name("data.test.p")?; + assert_eq!(got, Value::Bool(true), "{body}, {mode:?}"); + if mode == ExecutionMode::Suspendable { + assert_eq!( + vm.execution_state(), + &ExecutionState::Completed { result: got } + ); + } + } + } Ok(()) } From db2b529035bb7318a459d3f9eed3e031f48113e5 Mon Sep 17 00:00:00 2001 From: Anand Krishnamoorthi Date: Mon, 9 Nov 2026 18:31:00 -0600 Subject: [PATCH 5/9] test(rvm): preserve match coverage with supported negation Keep positive destructuring inside nested comprehensions, and negate == instead of direct unification. Preserve matching, missing-value, root-restoration, and capacity controls ahead of the independent stricter admission change. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 84271ec2-b466-4cfe-bf58-848f4dceb842 --- tests/rvm/compiler.rs | 51 ++++++++++++++++++++++++++++--------------- 1 file changed, 33 insertions(+), 18 deletions(-) diff --git a/tests/rvm/compiler.rs b/tests/rvm/compiler.rs index 25f83477b..cadbd1fb0 100644 --- a/tests/rvm/compiler.rs +++ b/tests/rvm/compiler.rs @@ -59,7 +59,7 @@ fn match_boundaries_match_interpreter() -> anyhow::Result<()> { r#"{"payload":{"a":7}}"#, true, ), - ("", "not _ = input.missing", r#"{}"#, true), + ("", "not [1 | _ = input.missing] == [1]", r#"{}"#, true), ("f(_) := true", "not f(input.missing)", r#"{}"#, true), ("f(_) := true", "not f(input.missing, 7)", r#"{}"#, true), ("f(_) := true", "f(input.missing, 1 / 0)", r#"{}"#, false), @@ -71,8 +71,8 @@ fn match_boundaries_match_interpreter() -> anyhow::Result<()> { r#"{}"#, true, ), - ("", "not _ = false", r#"{}"#, false), - ("", "not _ = null", r#"{}"#, false), + ("", "not [1 | _ = false] == [1]", r#"{}"#, false), + ("", "not [1 | _ = null] == [1]", r#"{}"#, false), ( "", r#"not split("a,b,c", ",", [input.missing, 1 / 0, _]); input.x == 7"#, @@ -85,13 +85,23 @@ fn match_boundaries_match_interpreter() -> anyhow::Result<()> { r#"{}"#, true, ), - ("", "not {} = input.payload", r#"{"payload":[]}"#, true), - ("", "not [x] = input.payload", r#"{"payload":[]}"#, true), + ( + "", + "not [1 | {} = input.payload] == [1]", + r#"{"payload":[]}"#, + true, + ), + ( + "", + "not [1 | [x] = input.payload] == [1]", + r#"{"payload":[]}"#, + true, + ), ("", r#"not split("a,b", ",", ["a"])"#, r#"{}"#, true), ("", r#"not split("a,b", ",", ["wrong","b"])"#, r#"{}"#, true), ("", r#"not split("a,b", ",", ["a","b"])"#, r#"{}"#, false), - ("", "not [_, 1] = [7, 2]", r#"{}"#, true), - ("", "not [_, 1] = [7, 1]", r#"{}"#, false), + ("", "not [1 | [_, 1] = [7, 2]] == [1]", r#"{}"#, true), + ("", "not [1 | [_, 1] = [7, 1]] == [1]", r#"{}"#, false), ("", "[x, y] := [false, null]", r#"{}"#, true), ( "", @@ -101,7 +111,7 @@ fn match_boundaries_match_interpreter() -> anyhow::Result<()> { ), ( "", - r#"not {"a": [_]} = input.payload"#, + r#"not [1 | {"a": [_]} = input.payload] == [1]"#, r#"{"payload":{"a":{}}}"#, true, ), @@ -143,44 +153,49 @@ fn match_boundaries_match_interpreter() -> anyhow::Result<()> { ), ( "", - r#"not {"a":[{"b":[_,data.x]}]} = input.payload; data.x == 7"#, + r#"not [1 | {"a":[{"b":[_,data.x]}]} = input.payload] == [1]; data.x == 7"#, r#"{"payload":{"a":[{"b":[9,2]}]}}"#, true, ), ( "", - r#"not {"a":[{"b":[_,data.x]}]} = input.payload"#, + r#"not [1 | {"a":[{"b":[_,data.x]}]} = input.payload] == [1]"#, r#"{"payload":{"a":[{"b":[9,7]}]}}"#, false, ), ( "", - r#"not {"a":[{"b":[_,data.x]}]} = input.payload; data.x == 7"#, + r#"not [1 | {"a":[{"b":[_,data.x]}]} = input.payload] == [1]; data.x == 7"#, r#"{"payload":{"a":[]}}"#, true, ), ( "f(_) := 1", - "not [_, f(1)] = [7,2]; data.x == 7", + "not [1 | [_, f(1)] = [7,2]] == [1]; data.x == 7", r#"{}"#, true, ), - ("f(_) := 1", "not [_, f(1)] = [7,1]", r#"{}"#, false), ( "f(_) := 1", - "not [_, f(input.missing)] = [7,2]; data.x == 7", + "not [1 | [_, f(1)] = [7,1]] == [1]", + r#"{}"#, + false, + ), + ( + "f(_) := 1", + "not [1 | [_, f(input.missing)] = [7,2]] == [1]; data.x == 7", r#"{}"#, true, ), ( "", - "not [1 | not [1 | not 1 = input.x] == [1]] == [1]", + "not [1 | not [1 | not 1 == input.x] == [1]] == [1]", r#"{"x":2}"#, true, ), ( "", - "not [1 | every y in [input.x] { not [1 | not 1 = y] == [1] }] == [1]", + "not [1 | every y in [input.x] { not [1 | not 1 == y] == [1] }] == [1]", r#"{"x":1}"#, false, ), @@ -279,7 +294,7 @@ fn undefined_user_input_skips_invocation_and_preserves_value_heads() -> anyhow:: } #[test] -fn wide_soft_match_and_successive_calls_remain_compilable() -> anyhow::Result<()> { +fn wide_nested_match_and_successive_calls_remain_compilable() -> anyhow::Result<()> { let wildcards = vec!["_"; 128].join(","); let ones = vec!["1"; 128].join(","); let fields = (0..43) @@ -293,7 +308,7 @@ fn wide_soft_match_and_successive_calls_remain_compilable() -> anyhow::Result<() for (definitions, body, want) in [ ( String::new(), - format!("not [{wildcards}] = [{ones}]"), + format!("not [1 | [{wildcards}] = [{ones}]] == [1]"), false, ), ("f(_) := true".into(), vec!["f(1)"; 100].join(";"), true), From bde1d63f36aed1d29a96cbe002cc7316b65c51e7 Mon Sep 17 00:00:00 2001 From: Anand Krishnamoorthi Date: Tue, 10 Nov 2026 10:32:43 -0600 Subject: [PATCH 6/9] fix(rvm): preserve undefined negated structural outputs Keep invocation results intact while evaluating negated relational output expressions. Short-circuit expected collections in source order on Undefined and compare Undefined locally without changing general VM equality. Assert interpreter parity in both VM modes, retaining only explicit F25 strict-error transport controls. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 84271ec2-b466-4cfe-bf58-848f4dceb842 --- docs/rvm/architecture.md | 7 +- src/languages/rego/compiler/function_calls.rs | 144 ++++++++++++++---- tests/rvm/compiler.rs | 100 +++++++++++- 3 files changed, 217 insertions(+), 34 deletions(-) diff --git a/docs/rvm/architecture.md b/docs/rvm/architecture.md index 954a4e63a..e142ed995 100644 --- a/docs/rvm/architecture.md +++ b/docs/rvm/architecture.md @@ -61,7 +61,12 @@ Negated relational calls evaluate binding-free expected outputs before equality comparison even when undefined inputs skip invocation. Wildcard outputs evaluate the prefix before the first wildcard in expression order, stopping when an operand is undefined; a structural mismatch in the call result does not suppress those -effects. Reaching an unbound wildcard in the interpreter still reports an +effects. Wildcard-free expected collections also stop at the first undefined +element or object key/value. This call-local comparison preserves the invocation +result while evaluating the expected output: two undefined outputs compare equal, +and exactly one undefined output compares unequal. General VM equality and other +collection-expression evaluation are unchanged. +Reaching an unbound wildcard in the interpreter still reports an unsafe-variable error; the VM does not reproduce that existing error. Collection-kind checks and sequential failure checks share a temporary register per rule-definition window, separate from preserved match results. Skipped user diff --git a/src/languages/rego/compiler/function_calls.rs b/src/languages/rego/compiler/function_calls.rs index 5e425aeea..0dd39b983 100644 --- a/src/languages/rego/compiler/function_calls.rs +++ b/src/languages/rego/compiler/function_calls.rs @@ -9,13 +9,15 @@ clippy::pattern_type_mismatch )] +use super::expressions::try_eval_const; use super::{Compiler, CompilerError, Register, Result}; use crate::ast::{Expr, ExprRef}; use crate::builtins; use crate::compiler::destructuring_planner::plans::BindingPlan; use crate::lexer::Span; use crate::rvm::instructions::{ - ArrayCreateParams, BuiltinCallParams, FunctionCallParams, GuardMode, + ArrayCreateParams, BuiltinCallParams, FunctionCallParams, GuardMode, ObjectCreateParams, + SetCreateParams, }; use crate::rvm::Instruction; use crate::utils::{append_path_component, get_path_string, split_canonical_path_root}; @@ -377,28 +379,54 @@ impl<'a> Compiler<'a> { let expected_expr = params.last().ok_or_else(|| CompilerError::General { message: "missing relational output expression".to_string(), })?; - let has_wildcards = matches!(plan, BindingPlan::Parameter { destructuring_plan, .. } - if destructuring_plan.contains_wildcards()); - if has_wildcards { - self.emit_instruction(Instruction::LoadBool { dest, value: false }, &span); - let roots = (self.current_input_register, self.current_data_register); - let mut failures = Vec::new(); - self.compile_negated_output_prefix(expected_expr, &mut failures)?; - self.patch_match_failure_branches(&failures) + let roots = (self.current_input_register, self.current_data_register); + let result = self.alloc_register()?; + // Keep the invocation value intact. If expected evaluation exits + // Undefined, relational equality succeeds only when both are Undefined. + self.emit_instruction( + Instruction::Eq { + dest: result, + left: dest, + right: dest, + }, + &span, + ); + self.emit_instruction( + Instruction::Not { + dest: result, + operand: result, + }, + &span, + ); + let mut undefined_expected = Vec::new(); + let expected = + self.compile_negated_expected_output(expected_expr, &mut undefined_expected)?; + self.emit_instruction( + Instruction::LoadBool { + dest: result, + value: false, + }, + &span, + ); + if let Some(expected) = expected { + let undefined_actual = self + .emit_match_failure_branch(dest, GuardMode::NotUndefined, &span) .map_err(|err| CompilerError::from(err).at(&span))?; - (self.current_input_register, self.current_data_register) = roots; - } else { - let expected = - self.compile_rego_expr_with_span(expected_expr, plan_span, false)?; self.emit_instruction( Instruction::Eq { - dest, + dest: result, left: dest, right: expected, }, &span, ); + self.patch_match_failure_branches(&[undefined_actual]) + .map_err(|err| CompilerError::from(err).at(&span))?; } + self.patch_match_failure_branches(&undefined_expected) + .map_err(|err| CompilerError::from(err).at(&span))?; + self.emit_instruction(Instruction::Move { dest, src: result }, &span); + (self.current_input_register, self.current_data_register) = roots; } else { let plan_result = self .apply_binding_plan(plan, dest, plan_span) @@ -463,31 +491,86 @@ impl<'a> Compiler<'a> { Ok(dest) } - // A wildcard cannot produce a defined expected output. Evaluate only the - // prefix before it, in expression order, rather than matching the call's - // result (whose shape or earlier mismatches must not suppress these effects). - fn compile_negated_output_prefix( + // Evaluate expected collections in source order, exiting on Undefined before + // later effects. None denotes reaching a wildcard rather than a value. + fn compile_negated_expected_output( &mut self, expr: &ExprRef, failures: &mut Vec, - ) -> Result { - match expr.as_ref() { - Expr::Var { span, .. } if span.text() == "_" => return Ok(false), + ) -> Result> { + if try_eval_const(expr.as_ref()).is_some() { + return self + .compile_rego_expr_with_span(expr, expr.span(), false) + .map(Some); + } + let register = match expr.as_ref() { + Expr::Var { span, .. } if span.text() == "_" => return Ok(None), Expr::Array { items, .. } | Expr::Set { items, .. } => { + let mut elements = Vec::with_capacity(items.len()); for item in items { - if !self.compile_negated_output_prefix(item, failures)? { - return Ok(false); - } + let Some(register) = self.compile_negated_expected_output(item, failures)? + else { + return Ok(None); + }; + elements.push(register); + } + let dest = self.alloc_register()?; + if matches!(expr.as_ref(), Expr::Array { .. }) { + let params_index = self + .program + .instruction_data + .add_array_create_params(ArrayCreateParams { dest, elements }); + self.emit_instruction(Instruction::ArrayCreate { params_index }, expr.span()); + } else { + let params_index = self + .program + .instruction_data + .add_set_create_params(SetCreateParams { dest, elements }); + self.emit_instruction(Instruction::SetCreate { params_index }, expr.span()); } + dest } Expr::Object { fields, .. } => { + let mut literal_key_fields = Vec::new(); + let mut dynamic_fields = Vec::new(); for (_, key, value) in fields { - if !self.compile_negated_output_prefix(key, failures)? - || !self.compile_negated_output_prefix(value, failures)? - { - return Ok(false); + let literal_key = try_eval_const(key.as_ref()); + let key_register = if literal_key.is_none() { + let Some(register) = self.compile_negated_expected_output(key, failures)? + else { + return Ok(None); + }; + Some(register) + } else { + None + }; + let Some(value_register) = + self.compile_negated_expected_output(value, failures)? + else { + return Ok(None); + }; + if let Some(literal_value) = literal_key { + literal_key_fields.push((self.add_literal(literal_value), value_register)); + } else if let Some(dynamic_key_register) = key_register { + dynamic_fields.push((dynamic_key_register, value_register)); } } + literal_key_fields.sort_by(|a, b| { + self.program.literals[a.0 as usize].cmp(&self.program.literals[b.0 as usize]) + }); + let dest = self.alloc_register()?; + let template_literal_idx = self.add_literal(Value::new_object()); + let params_index = + self.program + .instruction_data + .add_object_create_params(ObjectCreateParams { + dest, + template_literal_idx, + literal_key_fields, + fields: dynamic_fields, + }); + self.emit_instruction(Instruction::ObjectCreate { params_index }, expr.span()); + dest } _ => { let register = self.compile_rego_expr_with_span(expr, expr.span(), false)?; @@ -495,9 +578,10 @@ impl<'a> Compiler<'a> { self.emit_match_failure_branch(register, GuardMode::NotUndefined, expr.span()) .map_err(|err| CompilerError::from(err).at(expr.span()))?, ); + register } - } - Ok(true) + }; + Ok(Some(register)) } /// Rewrite an import-aliased call path to its target, e.g. `b.f(1)` to diff --git a/tests/rvm/compiler.rs b/tests/rvm/compiler.rs index cadbd1fb0..e597c89f1 100644 --- a/tests/rvm/compiler.rs +++ b/tests/rvm/compiler.rs @@ -62,6 +62,18 @@ fn match_boundaries_match_interpreter() -> anyhow::Result<()> { ("", "not [1 | _ = input.missing] == [1]", r#"{}"#, true), ("f(_) := true", "not f(input.missing)", r#"{}"#, true), ("f(_) := true", "not f(input.missing, 7)", r#"{}"#, true), + ( + "f(_) := true", + "not f(input.missing, [input.missing, _])", + r#"{}"#, + false, + ), + ( + "", + "not count(input.missing, [input.missing, 1 / 0])", + r#"{}"#, + false, + ), ("f(_) := true", "f(input.missing, 1 / 0)", r#"{}"#, false), ("f(_) := 1 / 0", "not f(input.missing)", r#"{}"#, true), ("f(_) := true", "f(input.missing, _)", r#"{}"#, false), @@ -388,6 +400,8 @@ fn undefined_user_input_survives_program_json_round_trip() -> anyhow::Result<()> fn undefined_user_input_preserves_negated_output_evaluation() -> anyhow::Result<()> { for strict in [true, false] { let mut outcomes = Vec::new(); + // F25 strict-error transport differs by VM mode; non-strict values below + // must agree with the interpreter rather than merely another VM probe. let control = if strict { "not input.missing == 1 / 0" } else { @@ -409,11 +423,14 @@ fn undefined_user_input_preserves_negated_output_evaluation() -> anyhow::Result< let input = Value::from_json_str("{}")?; engine.set_input(input.clone()); let interpreted = engine.eval_rule("data.test.p".into()); - if strict { + let expected = if strict { assert!(interpreted.is_err(), "{body}"); + None } else { - assert_eq!(interpreted?, Value::Bool(false), "{body}"); - } + let expected = interpreted?; + assert_eq!(expected, Value::Bool(false), "{body}"); + Some(expected) + }; let compiled = engine.compile_with_entrypoint(&Rc::from("data.test.p"))?; let program = Compiler::compile_from_policy(&compiled, &["data.test.p"])?; let mut modes = Vec::new(); @@ -424,6 +441,13 @@ fn undefined_user_input_preserves_negated_output_evaluation() -> anyhow::Result< vm.load_program(program.clone()); vm.set_input(input.clone()); let result = vm.execute_entry_point_by_name("data.test.p"); + if let Some(expected) = &expected { + assert_eq!( + result.as_ref().expect("non-strict execution"), + expected, + "{body}, {mode:?}" + ); + } modes.push(observe_strict_probe(&vm, result)); } outcomes.push(modes); @@ -434,6 +458,76 @@ fn undefined_user_input_preserves_negated_output_evaluation() -> anyhow::Result< Ok(()) } +#[test] +fn negated_structural_outputs_preserve_undefined_and_short_circuit() -> anyhow::Result<()> { + let mut divergences = Vec::new(); + for (definition, call, expected) in [ + ("", "count(input.missing, [input.missing, _])", "[]"), + ("", "count(input.missing, [input.missing])", "[]"), + ("", "count(input.missing, [input.missing, 1 / 0])", "[]"), + ("f(_) := true", "f(input.missing, [input.missing, _])", "[]"), + ("f(_) := true", "f(input.missing, [input.missing])", "[]"), + ( + "", + r#"count(input.missing, {"a":[input.missing, 1 / 0]})"#, + "[]", + ), + ("", "count(input.missing, {input.missing, 1 / 0})", "[]"), + ("", "count(input.missing, {input.missing: 1 / 0})", "[]"), + ("", "count([], [input.missing, 1 / 0])", "[1]"), + ("f(_) := [false, null]", "f(1, [1 == 2, null])", "[]"), + ("f(_) := [false, null]", "f(1, [null, false])", "[1]"), + ( + r#"f(_) := {"a":[false, null]}"#, + r#"f(1, {concat("", ["a"]): [1 == 2, null]})"#, + "[]", + ), + ("f(_) := {false, null}", "f(1, {1 == 2, null})", "[]"), + ] { + for strict in [false, true] { + let mut engine = Engine::new(); + engine.set_strict_builtin_errors(strict); + engine.set_input(Value::new_object()); + engine.add_policy( + "test.rego".into(), + format!("package test\nimport rego.v1\n{definition}\np := [1 | not {call}]"), + )?; + let expected = Value::from_json_str(expected)?; + let interpreted = engine.eval_rule("data.test.p".into())?; + assert_eq!(interpreted, expected, "{call}, strict={strict}"); + let compiled = engine.compile_with_entrypoint(&Rc::from("data.test.p"))?; + let program = Compiler::compile_from_policy(&compiled, &["data.test.p"])?; + for mode in [ExecutionMode::RunToCompletion, ExecutionMode::Suspendable] { + let mut vm = RegoVM::new(); + vm.set_strict_builtin_errors(strict); + vm.set_execution_mode(mode); + vm.load_program(program.clone()); + vm.set_input(Value::new_object()); + match vm.execute_entry_point_by_name("data.test.p") { + Ok(actual) => { + if actual != interpreted { + divergences.push(format!( + "{call}, {mode:?}, strict={strict}: {actual:?} != {interpreted:?}" + )); + } + if mode == ExecutionMode::Suspendable { + assert_eq!( + vm.execution_state(), + &ExecutionState::Completed { result: actual } + ); + } + } + Err(error) => { + divergences.push(format!("{call}, {mode:?}, strict={strict}: {error}")) + } + } + } + } + } + assert!(divergences.is_empty(), "{}", divergences.join("\n")); + Ok(()) +} + #[test] fn match_boundaries_preserve_strict_error_evaluation() -> anyhow::Result<()> { let mut outcomes = Vec::new(); From d989ac7d4824cd87d008f396817c34121edafc03 Mon Sep 17 00:00:00 2001 From: Anand Krishnamoorthi Date: Tue, 10 Nov 2026 11:39:05 -0600 Subject: [PATCH 7/9] fix(rvm): reject unsafe outputs and preserve expected object keys Reject a reached negated-output wildcard through the existing query guard while retaining Undefined-prefix short-circuiting. Preserve source-order object insertion after the first dynamic key, reusing match scratch for constant suffix keys. Fold only expected-output literals whose nested object key types survive program JSON serialization, without changing the wire schema or general collection compiler. Remove unused destructuring context plumbing and use Arc for round-tripped programs. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 84271ec2-b466-4cfe-bf58-848f4dceb842 --- docs/rvm/architecture.md | 14 +- src/languages/rego/compiler/destructuring.rs | 56 +------ src/languages/rego/compiler/function_calls.rs | 102 +++++++++++-- tests/rvm/compiler.rs | 141 +++++++++++++++++- 4 files changed, 247 insertions(+), 66 deletions(-) diff --git a/docs/rvm/architecture.md b/docs/rvm/architecture.md index e142ed995..cf287b83b 100644 --- a/docs/rvm/architecture.md +++ b/docs/rvm/architecture.md @@ -66,8 +66,18 @@ element or object key/value. This call-local comparison preserves the invocation result while evaluating the expected output: two undefined outputs compare equal, and exactly one undefined output compares unequal. General VM equality and other collection-expression evaluation are unchanged. -Reaching an unbound wildcard in the interpreter still reports an -unsafe-variable error; the VM does not reproduce that existing error. +Expected-output object fields retain source-order insertion across constant and +dynamic keys, including collisions. Whole constant collections remain hoisted +only when their literal representation preserves key types through program JSON +serialization; objects with non-string keys, including nested constants, are +constructed at runtime. Constant keys following a dynamic key reuse the match +scratch for insertion. General literal serialization and its wire schema are +unchanged. +Reaching an unbound wildcard rejects the active VM query using an assertion +guard at the wildcard, rather than producing a comparison that negation could +turn into success. An earlier undefined prefix skips this guard. The interpreter +instead reports an unsafe-variable error; VM query rejection (for example, an +empty comprehension) is not diagnostic/error parity. Collection-kind checks and sequential failure checks share a temporary register per rule-definition window, separate from preserved match results. Skipped user calls retain their actual undefined input rather than a serialized undefined diff --git a/src/languages/rego/compiler/destructuring.rs b/src/languages/rego/compiler/destructuring.rs index d59f29568..2be46e6a4 100644 --- a/src/languages/rego/compiler/destructuring.rs +++ b/src/languages/rego/compiler/destructuring.rs @@ -12,17 +12,8 @@ use crate::value::Value; use alloc::vec::Vec; use anyhow::{bail, Result}; -#[derive(Clone, Copy, Debug)] -pub enum PlanContext { - ColonAssignment, - Assignment, - FunctionParameter, - LoopIndex, - SomeIn, -} - impl<'a> Compiler<'a> { - fn match_scratch_register(&mut self) -> Result { + pub(super) fn match_scratch_register(&mut self) -> Result { match self.match_scratch_register { Some(scratch_register) => Ok(scratch_register), None => { @@ -143,36 +134,21 @@ impl<'a> Compiler<'a> { rhs_expr, lhs_plan, .. } => { let rhs_reg = self.compile_rego_expr_with_span(rhs_expr, rhs_expr.span(), false)?; - let _ = self.apply_destructuring_plan( - lhs_plan, - rhs_reg, - span, - PlanContext::ColonAssignment, - )?; + let _ = self.apply_destructuring_plan(lhs_plan, rhs_reg, span)?; Ok(rhs_reg) } AssignmentPlan::EqualsBindLeft { rhs_expr, lhs_plan, .. } => { let rhs_reg = self.compile_rego_expr_with_span(rhs_expr, rhs_expr.span(), false)?; - let result = self.apply_destructuring_plan( - lhs_plan, - rhs_reg, - span, - PlanContext::Assignment, - )?; + let result = self.apply_destructuring_plan(lhs_plan, rhs_reg, span)?; result.map_or_else(|| self.load_bool_literal(true, span), Ok) } AssignmentPlan::EqualsBindRight { lhs_expr, rhs_plan, .. } => { let lhs_reg = self.compile_rego_expr_with_span(lhs_expr, lhs_expr.span(), false)?; - let result = self.apply_destructuring_plan( - rhs_plan, - lhs_reg, - span, - PlanContext::Assignment, - )?; + let result = self.apply_destructuring_plan(rhs_plan, lhs_reg, span)?; result.map_or_else(|| self.load_bool_literal(true, span), Ok) } AssignmentPlan::EqualsBothSides { element_pairs, .. } => { @@ -251,20 +227,10 @@ impl<'a> Compiler<'a> { } BindingPlan::LoopIndex { destructuring_plan, .. - } => self.apply_destructuring_plan( - destructuring_plan, - value_register, - span, - PlanContext::LoopIndex, - ), + } => self.apply_destructuring_plan(destructuring_plan, value_register, span), BindingPlan::Parameter { destructuring_plan, .. - } => self.apply_destructuring_plan( - destructuring_plan, - value_register, - span, - PlanContext::FunctionParameter, - ), + } => self.apply_destructuring_plan(destructuring_plan, value_register, span), BindingPlan::SomeIn { .. } => { bail!("use apply_some_in_binding_plan for SomeIn bindings") } @@ -282,14 +248,9 @@ impl<'a> Compiler<'a> { ) -> Result<()> { if let (Some(plan), Some(register)) = (key_plan, key_register) { let span = key_span.unwrap_or(value_span); - let _ = self.apply_destructuring_plan(plan, register, span, PlanContext::SomeIn)?; + let _ = self.apply_destructuring_plan(plan, register, span)?; } - let _ = self.apply_destructuring_plan( - value_plan, - value_register, - value_span, - PlanContext::SomeIn, - )?; + let _ = self.apply_destructuring_plan(value_plan, value_register, value_span)?; Ok(()) } @@ -298,7 +259,6 @@ impl<'a> Compiler<'a> { plan: &DestructuringPlan, value_register: Register, span: &Span, - _context: PlanContext, ) -> Result> { self.with_match_result(span, |compiler, failures| { compiler.apply_destructuring_plan_impl(plan, value_register, span, failures) diff --git a/src/languages/rego/compiler/function_calls.rs b/src/languages/rego/compiler/function_calls.rs index 0dd39b983..feaf13235 100644 --- a/src/languages/rego/compiler/function_calls.rs +++ b/src/languages/rego/compiler/function_calls.rs @@ -16,8 +16,8 @@ use crate::builtins; use crate::compiler::destructuring_planner::plans::BindingPlan; use crate::lexer::Span; use crate::rvm::instructions::{ - ArrayCreateParams, BuiltinCallParams, FunctionCallParams, GuardMode, ObjectCreateParams, - SetCreateParams, + ArrayCreateParams, BuiltinCallParams, FunctionCallParams, GuardMode, LiteralOrRegister, + ObjectCreateParams, SetCreateParams, }; use crate::rvm::Instruction; use crate::utils::{append_path_component, get_path_string, split_canonical_path_root}; @@ -57,6 +57,23 @@ enum CallTarget { }, } +fn output_literal_round_trips(value: &Value) -> bool { + match value { + Value::Array(values) => values.iter().all(output_literal_round_trips), + Value::Set(values) => values.iter().all(output_literal_round_trips), + // JSON object literals stringify non-string keys. Construct those + // objects at runtime, even when nested in a constant collection. + Value::Object(fields) => fields.iter().all(|(key, field_value)| { + matches!(key, Value::String(_)) && output_literal_round_trips(field_value) + }), + _ => true, + } +} + +fn try_eval_output_literal(expr: &Expr) -> Option { + try_eval_const(expr).filter(output_literal_round_trips) +} + impl<'a> Compiler<'a> { pub(super) fn compile_function_call( &mut self, @@ -492,19 +509,41 @@ impl<'a> Compiler<'a> { } // Evaluate expected collections in source order, exiting on Undefined before - // later effects. None denotes reaching a wildcard rather than a value. + // later effects. None denotes a wildcard that rejects the active query. fn compile_negated_expected_output( &mut self, expr: &ExprRef, failures: &mut Vec, ) -> Result> { - if try_eval_const(expr.as_ref()).is_some() { - return self - .compile_rego_expr_with_span(expr, expr.span(), false) - .map(Some); + if let Some(value) = try_eval_output_literal(expr.as_ref()) { + let dest = self.alloc_register()?; + let literal_idx = self.add_literal(value); + self.emit_instruction(Instruction::Load { dest, literal_idx }, expr.span()); + return Ok(Some(dest)); } let register = match expr.as_ref() { - Expr::Var { span, .. } if span.text() == "_" => return Ok(None), + Expr::Var { span, .. } if span.text() == "_" => { + let scratch = self + .match_scratch_register() + .map_err(|err| CompilerError::from(err).at(span))?; + self.emit_instruction( + Instruction::LoadBool { + dest: scratch, + value: false, + }, + span, + ); + // This must reject the query, not become a failed comparison + // that the surrounding negation turns into success. + self.emit_instruction( + Instruction::Guard { + register: scratch, + mode: GuardMode::Condition, + }, + span, + ); + return Ok(None); + } Expr::Array { items, .. } | Expr::Set { items, .. } => { let mut elements = Vec::with_capacity(items.len()); for item in items { @@ -532,9 +571,9 @@ impl<'a> Compiler<'a> { } Expr::Object { fields, .. } => { let mut literal_key_fields = Vec::new(); - let mut dynamic_fields = Vec::new(); + let mut ordered_fields = Vec::new(); for (_, key, value) in fields { - let literal_key = try_eval_const(key.as_ref()); + let literal_key = try_eval_output_literal(key.as_ref()); let key_register = if literal_key.is_none() { let Some(register) = self.compile_negated_expected_output(key, failures)? else { @@ -550,9 +589,18 @@ impl<'a> Compiler<'a> { return Ok(None); }; if let Some(literal_value) = literal_key { - literal_key_fields.push((self.add_literal(literal_value), value_register)); + let literal_idx = self.add_literal(literal_value); + if ordered_fields.is_empty() { + literal_key_fields.push((literal_idx, value_register)); + } else { + ordered_fields + .push((LiteralOrRegister::Literal(literal_idx), value_register)); + } } else if let Some(dynamic_key_register) = key_register { - dynamic_fields.push((dynamic_key_register, value_register)); + ordered_fields.push(( + LiteralOrRegister::Register(dynamic_key_register), + value_register, + )); } } literal_key_fields.sort_by(|a, b| { @@ -567,9 +615,37 @@ impl<'a> Compiler<'a> { dest, template_literal_idx, literal_key_fields, - fields: dynamic_fields, + fields: Vec::new(), }); self.emit_instruction(Instruction::ObjectCreate { params_index }, expr.span()); + // Literal keys after a dynamic key can collide with it. Insert + // this suffix in source order without allocating a register per key. + for (key, value) in ordered_fields { + let key_register = match key { + LiteralOrRegister::Register(register) => register, + LiteralOrRegister::Literal(literal_idx) => { + let scratch = self + .match_scratch_register() + .map_err(|err| CompilerError::from(err).at(expr.span()))?; + self.emit_instruction( + Instruction::Load { + dest: scratch, + literal_idx, + }, + expr.span(), + ); + scratch + } + }; + self.emit_instruction( + Instruction::ObjectSet { + obj: dest, + key: key_register, + value, + }, + expr.span(), + ); + } dest } _ => { diff --git a/tests/rvm/compiler.rs b/tests/rvm/compiler.rs index e597c89f1..21986f955 100644 --- a/tests/rvm/compiler.rs +++ b/tests/rvm/compiler.rs @@ -375,7 +375,8 @@ fn undefined_user_input_survives_program_json_round_trip() -> anyhow::Result<()> let compiled = engine.compile_with_entrypoint(&Rc::from("data.test.p"))?; let program = Compiler::compile_from_policy(&compiled, &["data.test.p"])?; let json = program.serialize_json().map_err(anyhow::Error::msg)?; - let round_trip = Rc::new(Program::deserialize_json(&json).map_err(anyhow::Error::msg)?); + let round_trip = + std::sync::Arc::new(Program::deserialize_json(&json).map_err(anyhow::Error::msg)?); for program in [program, round_trip] { for mode in [ExecutionMode::RunToCompletion, ExecutionMode::Suspendable] { let mut vm = RegoVM::new(); @@ -460,6 +461,19 @@ fn undefined_user_input_preserves_negated_output_evaluation() -> anyhow::Result< #[test] fn negated_structural_outputs_preserve_undefined_and_short_circuit() -> anyhow::Result<()> { + let constant_set = format!( + "f(1, [{{{}}}, input.missing, _])", + (0..300) + .map(|i| i.to_string()) + .collect::>() + .join(",\n") + ); + let object_fields = (0..200) + .map(|i| format!(r#""k{i}":{i}"#)) + .collect::>() + .join(",\n"); + let wide_definition = format!("f(_) := {{[1]: 2, {object_fields}}}"); + let wide_call = format!("f(1, {{[1 + 0]: 1, [1]: 2, {object_fields}}})"); let mut divergences = Vec::new(); for (definition, call, expected) in [ ("", "count(input.missing, [input.missing, _])", "[]"), @@ -483,11 +497,27 @@ fn negated_structural_outputs_preserve_undefined_and_short_circuit() -> anyhow:: "[]", ), ("f(_) := {false, null}", "f(1, {1 == 2, null})", "[]"), + ("f(_) := {[1]: 2}", "f(1, {[1 + 0]: 1, [1]: 2})", "[]"), + ("f(_) := {[1]: 1}", "f(1, {[1]: 2, [1 + 0]: 1})", "[]"), + ( + r#"f(_) := {"a": 2}"#, + r#"f(1, {concat("", ["a"]): 1, "a": 2})"#, + "[]", + ), + (r#"f(_) := {"k": 2}"#, r#"f(1, {input.k: 1, "k": 2})"#, "[]"), + ( + "f(_) := {[1]: 3}", + "f(1, {[1]: 0, [1 + 0]: 1, [1]: 2, [1 + 0]: 3})", + "[]", + ), + ("f(_) := true", constant_set.as_str(), "[1]"), + (wide_definition.as_str(), wide_call.as_str(), "[]"), ] { for strict in [false, true] { let mut engine = Engine::new(); engine.set_strict_builtin_errors(strict); - engine.set_input(Value::new_object()); + let input = Value::from_json_str(r#"{"k":"k"}"#)?; + engine.set_input(input.clone()); engine.add_policy( "test.rego".into(), format!("package test\nimport rego.v1\n{definition}\np := [1 | not {call}]"), @@ -502,7 +532,7 @@ fn negated_structural_outputs_preserve_undefined_and_short_circuit() -> anyhow:: vm.set_strict_builtin_errors(strict); vm.set_execution_mode(mode); vm.load_program(program.clone()); - vm.set_input(Value::new_object()); + vm.set_input(input.clone()); match vm.execute_entry_point_by_name("data.test.p") { Ok(actual) => { if actual != interpreted { @@ -528,6 +558,111 @@ fn negated_structural_outputs_preserve_undefined_and_short_circuit() -> anyhow:: Ok(()) } +#[test] +fn reached_negated_output_wildcards_reject_the_active_query() -> anyhow::Result<()> { + let mut divergences = Vec::new(); + for (definition, call) in [ + ("", "count([1], _)"), + ("", "count([1], [_, 1 / 0])"), + ("", "count([1], {_})"), + ("f(_) := true", "f(input.missing, _)"), + ("f(_) := true", r#"f(1, {"a": [_, 1 / 0]})"#), + ] { + for strict in [false, true] { + let mut engine = Engine::new(); + engine.set_strict_builtin_errors(strict); + engine.set_input(Value::new_object()); + engine.add_policy( + "test.rego".into(), + format!("package test\nimport rego.v1\n{definition}\np := [1 | not {call}]"), + )?; + let error = engine.eval_rule("data.test.p".into()).unwrap_err(); + assert!( + error.to_string().contains("unsafe"), + "{call}, strict={strict}: {error}" + ); + eprintln!("{call}, strict={strict}: interpreter {error}"); + let compiled = engine.compile_with_entrypoint(&Rc::from("data.test.p"))?; + let program = Compiler::compile_from_policy(&compiled, &["data.test.p"])?; + for mode in [ExecutionMode::RunToCompletion, ExecutionMode::Suspendable] { + let mut vm = RegoVM::new(); + vm.set_strict_builtin_errors(strict); + vm.set_execution_mode(mode); + vm.load_program(program.clone()); + vm.set_input(Value::new_object()); + // RVM query rejection is not interpreter unsafe-variable error parity. + let actual = vm.execute_entry_point_by_name("data.test.p")?; + if actual != Value::new_array() { + divergences.push(format!("{call}, strict={strict}, {mode:?}: {actual:?}")); + } + if mode == ExecutionMode::Suspendable { + assert_eq!( + vm.execution_state(), + &ExecutionState::Completed { result: actual } + ); + } + } + } + } + assert!(divergences.is_empty(), "{}", divergences.join("\n")); + Ok(()) +} + +#[test] +fn negated_output_constants_survive_json_round_trip() -> anyhow::Result<()> { + let mut divergences = Vec::new(); + for (definition, output) in [ + (r#"f(x) := {x: "v"}"#, r#"{1: "v"}"#), + (r#"f(x) := [{x: "v"}]"#, r#"[{1: "v"}]"#), + (r#"f(x) := {"a": {x: "v"}}"#, r#"{"a": {1: "v"}}"#), + (r#"f(x) := {{x: "v"}}"#, r#"{{1: "v"}}"#), + (r#"f(x) := {[x]: "v"}"#, r#"{[1]: "v"}"#), + (r#"f(x) := {{x: "v"}: "outer"}"#, r#"{{1: "v"}: "outer"}"#), + (r#"f(x) := {"a": x}"#, r#"{"a": 1}"#), + ] { + for strict in [false, true] { + let mut engine = Engine::new(); + engine.set_strict_builtin_errors(strict); + engine.add_policy( + "test.rego".into(), + format!( + "package test\nimport rego.v1\n{definition}\ndefault p := false\np if {{ not f(1, {output}) }}" + ), + )?; + let interpreted = engine.eval_rule("data.test.p".into())?; + assert_eq!(interpreted, Value::Bool(false), "{output}"); + let compiled = engine.compile_with_entrypoint(&Rc::from("data.test.p"))?; + let program = Compiler::compile_from_policy(&compiled, &["data.test.p"])?; + let json = program.serialize_json().map_err(anyhow::Error::msg)?; + let round_trip = + std::sync::Arc::new(Program::deserialize_json(&json).map_err(anyhow::Error::msg)?); + for (snapshot, program) in [("original", program), ("round-trip", round_trip)] { + for mode in [ExecutionMode::RunToCompletion, ExecutionMode::Suspendable] { + let mut vm = RegoVM::new(); + vm.set_strict_builtin_errors(strict); + vm.set_execution_mode(mode); + vm.load_program(program.clone()); + let actual = vm.execute_entry_point_by_name("data.test.p")?; + eprintln!("{output}, {snapshot}, strict={strict}, {mode:?}: {actual:?}"); + if actual != interpreted { + divergences.push(format!( + "{output}, {snapshot}, strict={strict}, {mode:?}: {actual:?} != {interpreted:?}" + )); + } + if mode == ExecutionMode::Suspendable { + assert_eq!( + vm.execution_state(), + &ExecutionState::Completed { result: actual } + ); + } + } + } + } + } + assert!(divergences.is_empty(), "{}", divergences.join("\n")); + Ok(()) +} + #[test] fn match_boundaries_preserve_strict_error_evaluation() -> anyhow::Result<()> { let mut outcomes = Vec::new(); From 4413a164e0baf40ef9d9b797dd1da53e26784099 Mon Sep 17 00:00:00 2001 From: Anand Krishnamoorthi Date: Tue, 10 Nov 2026 12:00:48 -0600 Subject: [PATCH 8/9] Preserve non-string keys in compiled return literals Share the JSON-lossless literal folding filter with collection compilation so constant function results retain typed object keys, including nested constants. Keep non-string keys out of embedded runtime templates and add original/round-trip differential controls. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 84271ec2-b466-4cfe-bf58-848f4dceb842 --- docs/rvm/architecture.md | 11 ++++--- src/languages/rego/compiler/expressions.rs | 2 +- .../expressions/collection_literals.rs | 29 ++++++++++++++++--- src/languages/rego/compiler/function_calls.rs | 23 ++------------- tests/rvm/compiler.rs | 5 ++++ 5 files changed, 41 insertions(+), 29 deletions(-) diff --git a/docs/rvm/architecture.md b/docs/rvm/architecture.md index cf287b83b..03c0d96f4 100644 --- a/docs/rvm/architecture.md +++ b/docs/rvm/architecture.md @@ -65,14 +65,17 @@ effects. Wildcard-free expected collections also stop at the first undefined element or object key/value. This call-local comparison preserves the invocation result while evaluating the expected output: two undefined outputs compare equal, and exactly one undefined output compares unequal. General VM equality and other -collection-expression evaluation are unchanged. +collection-expression evaluation order are unchanged. Expected-output object fields retain source-order insertion across constant and dynamic keys, including collisions. Whole constant collections remain hoisted only when their literal representation preserves key types through program JSON serialization; objects with non-string keys, including nested constants, are -constructed at runtime. Constant keys following a dynamic key reuse the match -scratch for insertion. General literal serialization and its wire schema are -unchanged. +constructed at runtime. This lossless-folding check also applies to ordinary +collection literals, so constant function results retain the same key types as +their expected outputs after serialization. Runtime object templates contain +only string keys; other keys are inserted from typed registers or scalar +literals. Constant keys following a dynamic key reuse the match scratch for +insertion. Value serialization and its wire schema are unchanged. Reaching an unbound wildcard rejects the active VM query using an assertion guard at the wildcard, rather than producing a comparison that negation could turn into success. An earlier undefined prefix skips this guard. The interpreter diff --git a/src/languages/rego/compiler/expressions.rs b/src/languages/rego/compiler/expressions.rs index 91da208e4..ec13063ab 100644 --- a/src/languages/rego/compiler/expressions.rs +++ b/src/languages/rego/compiler/expressions.rs @@ -5,7 +5,7 @@ mod collection_literals; mod operations; -pub(super) use collection_literals::try_eval_const; +pub(super) use collection_literals::{try_eval_const, try_eval_literal}; use super::{Compiler, CompilerError, Register, Result}; use crate::ast::{Expr, ExprRef}; diff --git a/src/languages/rego/compiler/expressions/collection_literals.rs b/src/languages/rego/compiler/expressions/collection_literals.rs index 91b9ed290..6bebb9065 100644 --- a/src/languages/rego/compiler/expressions/collection_literals.rs +++ b/src/languages/rego/compiler/expressions/collection_literals.rs @@ -50,13 +50,31 @@ pub(in crate::languages::rego::compiler) fn try_eval_const(expr: &Expr) -> Optio } } +fn literal_round_trips(value: &Value) -> bool { + match value { + Value::Array(values) => values.iter().all(literal_round_trips), + Value::Set(values) => values.iter().all(literal_round_trips), + // JSON object literals stringify non-string keys. Construct those + // objects at runtime, even when nested in a constant collection. + Value::Object(fields) => fields.iter().all(|(key, field_value)| { + matches!(key, Value::String(_)) && literal_round_trips(field_value) + }), + _ => true, + } +} + +pub(in crate::languages::rego::compiler) fn try_eval_literal(expr: &Expr) -> Option { + try_eval_const(expr).filter(literal_round_trips) +} + impl<'a> Compiler<'a> { pub(super) fn compile_array_literal( &mut self, items: &[ExprRef], span: &Span, ) -> Result { - let all_const: Option> = items.iter().map(|i| try_eval_const(i.as_ref())).collect(); + let all_const: Option> = + items.iter().map(|i| try_eval_literal(i.as_ref())).collect(); if let Some(values) = all_const { let dest = self.alloc_register()?; let literal_idx = self.add_literal(Value::from_array(values)); @@ -89,7 +107,7 @@ impl<'a> Compiler<'a> { span: &Span, ) -> Result { let all_const: Option> = - items.iter().map(|i| try_eval_const(i.as_ref())).collect(); + items.iter().map(|i| try_eval_literal(i.as_ref())).collect(); if let Some(values) = all_const { let dest = self.alloc_register()?; let literal_idx = self.add_literal(Value::from_set(values)); @@ -120,9 +138,11 @@ impl<'a> Compiler<'a> { ) -> Result { let all_const: Option = fields .iter() - .map(|(_, k, v)| Some((try_eval_const(k.as_ref())?, try_eval_const(v.as_ref())?))) + .map(|(_, k, v)| Some((try_eval_const(k.as_ref())?, try_eval_literal(v.as_ref())?))) .collect(); - if let Some(obj) = all_const { + if let Some(obj) = + all_const.filter(|object| object.keys().all(|key| matches!(key, Value::String(_)))) + { let dest = self.alloc_register()?; let literal_idx = self.add_literal(Value::Object(Rc::new(obj))); self.emit_instruction(Instruction::Load { dest, literal_idx }, span); @@ -165,6 +185,7 @@ impl<'a> Compiler<'a> { let template_literal_idx = { let mut template_keys = literal_keys.clone(); + template_keys.retain(|key| matches!(key, Value::String(_))); template_keys.sort(); let mut template_obj = Object::new(); diff --git a/src/languages/rego/compiler/function_calls.rs b/src/languages/rego/compiler/function_calls.rs index feaf13235..9f7839f9d 100644 --- a/src/languages/rego/compiler/function_calls.rs +++ b/src/languages/rego/compiler/function_calls.rs @@ -9,7 +9,7 @@ clippy::pattern_type_mismatch )] -use super::expressions::try_eval_const; +use super::expressions::try_eval_literal; use super::{Compiler, CompilerError, Register, Result}; use crate::ast::{Expr, ExprRef}; use crate::builtins; @@ -57,23 +57,6 @@ enum CallTarget { }, } -fn output_literal_round_trips(value: &Value) -> bool { - match value { - Value::Array(values) => values.iter().all(output_literal_round_trips), - Value::Set(values) => values.iter().all(output_literal_round_trips), - // JSON object literals stringify non-string keys. Construct those - // objects at runtime, even when nested in a constant collection. - Value::Object(fields) => fields.iter().all(|(key, field_value)| { - matches!(key, Value::String(_)) && output_literal_round_trips(field_value) - }), - _ => true, - } -} - -fn try_eval_output_literal(expr: &Expr) -> Option { - try_eval_const(expr).filter(output_literal_round_trips) -} - impl<'a> Compiler<'a> { pub(super) fn compile_function_call( &mut self, @@ -515,7 +498,7 @@ impl<'a> Compiler<'a> { expr: &ExprRef, failures: &mut Vec, ) -> Result> { - if let Some(value) = try_eval_output_literal(expr.as_ref()) { + if let Some(value) = try_eval_literal(expr.as_ref()) { let dest = self.alloc_register()?; let literal_idx = self.add_literal(value); self.emit_instruction(Instruction::Load { dest, literal_idx }, expr.span()); @@ -573,7 +556,7 @@ impl<'a> Compiler<'a> { let mut literal_key_fields = Vec::new(); let mut ordered_fields = Vec::new(); for (_, key, value) in fields { - let literal_key = try_eval_output_literal(key.as_ref()); + let literal_key = try_eval_literal(key.as_ref()); let key_register = if literal_key.is_none() { let Some(register) = self.compile_negated_expected_output(key, failures)? else { diff --git a/tests/rvm/compiler.rs b/tests/rvm/compiler.rs index 21986f955..e451fd84c 100644 --- a/tests/rvm/compiler.rs +++ b/tests/rvm/compiler.rs @@ -618,6 +618,11 @@ fn negated_output_constants_survive_json_round_trip() -> anyhow::Result<()> { (r#"f(x) := {{x: "v"}}"#, r#"{{1: "v"}}"#), (r#"f(x) := {[x]: "v"}"#, r#"{[1]: "v"}"#), (r#"f(x) := {{x: "v"}: "outer"}"#, r#"{{1: "v"}: "outer"}"#), + ("f(_) := {[1]: 2}", "{[1 + 0]: 1, [1]: 2}"), + (r#"f(_) := {1: "v"}"#, r#"{1: "v"}"#), + (r#"f(_) := [{1: "v"}]"#, r#"[{1: "v"}]"#), + (r#"f(_) := {"a": {1: "v"}}"#, r#"{"a": {1: "v"}}"#), + (r#"f(_) := {{1: "v"}}"#, r#"{{1: "v"}}"#), (r#"f(x) := {"a": x}"#, r#"{"a": 1}"#), ] { for strict in [false, true] { From 8c7f62dd312192c3c07a2b2f694f0d9f7d58edea Mon Sep 17 00:00:00 2001 From: Anand Krishnamoorthi Date: Tue, 10 Nov 2026 13:56:42 -0600 Subject: [PATCH 9/9] Preserve typed constants with bounded construction registers Construct sets and non-string-key objects without lossy JSON literals. Reuse a separate per-definition temporary pool so constant collection width does not exhaust registers, including expected outputs and nested values. Add typed interpreter/native/JSON regressions across both VM modes and strict settings, and document the constant construction boundary. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 84271ec2-b466-4cfe-bf58-848f4dceb842 --- docs/rvm/architecture.md | 13 +- .../expressions/collection_literals.rs | 121 ++++++++++-- src/languages/rego/compiler/function_calls.rs | 9 +- src/languages/rego/compiler/mod.rs | 2 + src/languages/rego/compiler/rules.rs | 4 + tests/rvm/compiler.rs | 187 +++++++++++++++++- 6 files changed, 302 insertions(+), 34 deletions(-) diff --git a/docs/rvm/architecture.md b/docs/rvm/architecture.md index 03c0d96f4..d8cc1cd31 100644 --- a/docs/rvm/architecture.md +++ b/docs/rvm/architecture.md @@ -68,14 +68,19 @@ and exactly one undefined output compares unequal. General VM equality and other collection-expression evaluation order are unchanged. Expected-output object fields retain source-order insertion across constant and dynamic keys, including collisions. Whole constant collections remain hoisted -only when their literal representation preserves key types through program JSON -serialization; objects with non-string keys, including nested constants, are -constructed at runtime. This lossless-folding check also applies to ordinary +only when their literal representation preserves types through program JSON +serialization; sets and objects with non-string keys, including nested constants, +are constructed at runtime. This lossless-folding check also applies to ordinary collection literals, so constant function results retain the same key types as their expected outputs after serialization. Runtime object templates contain only string keys; other keys are inserted from typed registers or scalar literals. Constant keys following a dynamic key reuse the match scratch for -insertion. Value serialization and its wire schema are unchanged. +insertion. Constant construction uses a separate per-definition temporary pool, +reusing registers after each insertion; temporary demand follows nesting depth, +not collection width. Pure constant subtrees use this path even inside dynamic +collections, without retaining their construction temporaries in parent elements. +Undefined is never an eligible folded literal. Value serialization and its wire +schema are unchanged. Reaching an unbound wildcard rejects the active VM query using an assertion guard at the wildcard, rather than producing a comparison that negation could turn into success. An earlier undefined prefix skips this guard. The interpreter diff --git a/src/languages/rego/compiler/expressions/collection_literals.rs b/src/languages/rego/compiler/expressions/collection_literals.rs index 6bebb9065..a88b843c5 100644 --- a/src/languages/rego/compiler/expressions/collection_literals.rs +++ b/src/languages/rego/compiler/expressions/collection_literals.rs @@ -6,7 +6,7 @@ clippy::pattern_type_mismatch )] -use super::{Compiler, Register, Result}; +use super::{Compiler, CompilerError, Register, Result}; use crate::ast::{Expr, ExprRef}; use crate::lexer::Span; use crate::rvm::instructions::{ArrayCreateParams, ObjectCreateParams, SetCreateParams}; @@ -53,7 +53,7 @@ pub(in crate::languages::rego::compiler) fn try_eval_const(expr: &Expr) -> Optio fn literal_round_trips(value: &Value) -> bool { match value { Value::Array(values) => values.iter().all(literal_round_trips), - Value::Set(values) => values.iter().all(literal_round_trips), + Value::Set(_) | Value::Undefined => false, // JSON object literals stringify non-string keys. Construct those // objects at runtime, even when nested in a constant collection. Value::Object(fields) => fields.iter().all(|(key, field_value)| { @@ -68,18 +68,105 @@ pub(in crate::languages::rego::compiler) fn try_eval_literal(expr: &Expr) -> Opt } impl<'a> Compiler<'a> { + fn constant_scratch_register(&mut self, depth: usize) -> Result { + while self.constant_scratch_registers.len() <= depth { + let register = self.alloc_register()?; + self.constant_scratch_registers.push(register); + } + Ok(self.constant_scratch_registers[depth]) + } + + pub(in crate::languages::rego::compiler) fn compile_constant_value( + &mut self, + value: &Value, + span: &Span, + ) -> Result { + let dest = self.alloc_register()?; + self.emit_constant_value(value, dest, 0, span)?; + Ok(dest) + } + + fn emit_constant_value( + &mut self, + value: &Value, + dest: Register, + depth: usize, + span: &Span, + ) -> Result<()> { + if literal_round_trips(value) { + let literal_idx = self.add_literal(value.clone()); + self.emit_instruction(Instruction::Load { dest, literal_idx }, span); + return Ok(()); + } + + // Only constant construction uses this pool: children have no effects, + // bindings or root caches. Their temporaries die after each insertion. + match value { + Value::Array(values) => { + self.emit_instruction(Instruction::ArrayNew { dest }, span); + for child in values.iter() { + let element = self.constant_scratch_register(depth)?; + self.emit_constant_value(child, element, depth.saturating_add(1), span)?; + self.emit_instruction( + Instruction::ArrayPush { + arr: dest, + value: element, + }, + span, + ); + } + } + Value::Set(values) => { + self.emit_instruction(Instruction::SetNew { dest }, span); + for child in values.iter() { + let element = self.constant_scratch_register(depth)?; + self.emit_constant_value(child, element, depth.saturating_add(1), span)?; + self.emit_instruction( + Instruction::SetAdd { + set: dest, + value: element, + }, + span, + ); + } + } + Value::Object(fields) => { + let literal_idx = self.add_literal(Value::new_object()); + self.emit_instruction(Instruction::Load { dest, literal_idx }, span); + for (field_key, field_value) in fields.iter() { + let key_register = self.constant_scratch_register(depth)?; + let value_register = self.constant_scratch_register(depth.saturating_add(1))?; + let child_depth = depth.saturating_add(2); + self.emit_constant_value(field_key, key_register, child_depth, span)?; + self.emit_constant_value(field_value, value_register, child_depth, span)?; + self.emit_instruction( + Instruction::ObjectSet { + obj: dest, + key: key_register, + value: value_register, + }, + span, + ); + } + } + _ => { + return Err(CompilerError::General { + message: "constant is not representable as a typed literal".into(), + } + .into()); + } + } + Ok(()) + } + pub(super) fn compile_array_literal( &mut self, items: &[ExprRef], span: &Span, ) -> Result { - let all_const: Option> = - items.iter().map(|i| try_eval_literal(i.as_ref())).collect(); + let all_const: Option> = items.iter().map(|i| try_eval_const(i.as_ref())).collect(); if let Some(values) = all_const { - let dest = self.alloc_register()?; - let literal_idx = self.add_literal(Value::from_array(values)); - self.emit_instruction(Instruction::Load { dest, literal_idx }, span); - return Ok(dest); + return self.compile_constant_value(&Value::from_array(values), span); } let mut element_registers = Vec::with_capacity(items.len()); @@ -107,12 +194,9 @@ impl<'a> Compiler<'a> { span: &Span, ) -> Result { let all_const: Option> = - items.iter().map(|i| try_eval_literal(i.as_ref())).collect(); + items.iter().map(|i| try_eval_const(i.as_ref())).collect(); if let Some(values) = all_const { - let dest = self.alloc_register()?; - let literal_idx = self.add_literal(Value::from_set(values)); - self.emit_instruction(Instruction::Load { dest, literal_idx }, span); - return Ok(dest); + return self.compile_constant_value(&Value::from_set(values), span); } let mut element_registers = Vec::with_capacity(items.len()); @@ -138,15 +222,10 @@ impl<'a> Compiler<'a> { ) -> Result { let all_const: Option = fields .iter() - .map(|(_, k, v)| Some((try_eval_const(k.as_ref())?, try_eval_literal(v.as_ref())?))) + .map(|(_, k, v)| Some((try_eval_const(k.as_ref())?, try_eval_const(v.as_ref())?))) .collect(); - if let Some(obj) = - all_const.filter(|object| object.keys().all(|key| matches!(key, Value::String(_)))) - { - let dest = self.alloc_register()?; - let literal_idx = self.add_literal(Value::Object(Rc::new(obj))); - self.emit_instruction(Instruction::Load { dest, literal_idx }, span); - return Ok(dest); + if let Some(obj) = all_const { + return self.compile_constant_value(&Value::Object(Rc::new(obj)), span); } let dest = self.alloc_register()?; diff --git a/src/languages/rego/compiler/function_calls.rs b/src/languages/rego/compiler/function_calls.rs index 9f7839f9d..97ff7258d 100644 --- a/src/languages/rego/compiler/function_calls.rs +++ b/src/languages/rego/compiler/function_calls.rs @@ -9,7 +9,7 @@ clippy::pattern_type_mismatch )] -use super::expressions::try_eval_literal; +use super::expressions::{try_eval_const, try_eval_literal}; use super::{Compiler, CompilerError, Register, Result}; use crate::ast::{Expr, ExprRef}; use crate::builtins; @@ -498,11 +498,8 @@ impl<'a> Compiler<'a> { expr: &ExprRef, failures: &mut Vec, ) -> Result> { - if let Some(value) = try_eval_literal(expr.as_ref()) { - let dest = self.alloc_register()?; - let literal_idx = self.add_literal(value); - self.emit_instruction(Instruction::Load { dest, literal_idx }, expr.span()); - return Ok(Some(dest)); + if let Some(value) = try_eval_const(expr.as_ref()) { + return Ok(Some(self.compile_constant_value(&value, expr.span())?)); } let register = match expr.as_ref() { Expr::Var { span, .. } if span.text() == "_" => { diff --git a/src/languages/rego/compiler/mod.rs b/src/languages/rego/compiler/mod.rs index c1fddf2eb..9ed20f686 100644 --- a/src/languages/rego/compiler/mod.rs +++ b/src/languages/rego/compiler/mod.rs @@ -140,6 +140,7 @@ pub struct Compiler<'a> { current_input_register: Option, current_data_register: Option, match_scratch_register: Option, + constant_scratch_registers: Vec, current_rule_path: String, current_call_stack: Vec, entry_points: IndexMap, @@ -180,6 +181,7 @@ impl<'a> Compiler<'a> { current_input_register: None, current_data_register: None, match_scratch_register: None, + constant_scratch_registers: Vec::new(), current_rule_path: String::new(), current_call_stack: Vec::new(), entry_points: IndexMap::new(), diff --git a/src/languages/rego/compiler/rules.rs b/src/languages/rego/compiler/rules.rs index 5cd228bf1..0d617d472 100644 --- a/src/languages/rego/compiler/rules.rs +++ b/src/languages/rego/compiler/rules.rs @@ -401,6 +401,8 @@ impl<'a> Compiler<'a> { self.current_module_index = module_index; let saved_register_counter = self.register_counter; + let saved_constant_scratch_registers = + core::mem::take(&mut self.constant_scratch_registers); if let Some(rule_definitions) = rules.get(rule_path) { let Some(rule_index) = self.rule_index_map.get(rule_path).copied() else { return Err(CompilerError::General { @@ -449,6 +451,7 @@ impl<'a> Compiler<'a> { self.push_scope(); self.register_counter = 0; self.match_scratch_register = None; + self.constant_scratch_registers.clear(); let result_register = self.alloc_register()?; @@ -797,6 +800,7 @@ impl<'a> Compiler<'a> { self.current_module_index = saved_module_index; } + self.constant_scratch_registers = saved_constant_scratch_registers; Ok(()) } } diff --git a/tests/rvm/compiler.rs b/tests/rvm/compiler.rs index e451fd84c..dac21b3de 100644 --- a/tests/rvm/compiler.rs +++ b/tests/rvm/compiler.rs @@ -608,6 +608,180 @@ fn reached_negated_output_wildcards_reject_the_active_query() -> anyhow::Result< Ok(()) } +#[test] +fn typed_constant_collections_reuse_construction_registers() -> anyhow::Result<()> { + let fields = (0..300) + .map(|n| format!("{n}: {n}")) + .collect::>() + .join(",\n"); + let object = format!("{{{fields}}}"); + let set = format!( + "{{{}}}", + (0..300) + .map(|n| n.to_string()) + .collect::>() + .join(",\n") + ); + let items = (0..300) + .map(|n| format!("{{{n}: {n}}}")) + .collect::>() + .join(",\n"); + let nested_fields = (0..300) + .map(|n| format!(r#""k{n}": {{{n}: {n}}}"#)) + .collect::>() + .join(",\n"); + let compound_fields = (0..300) + .map(|n| format!("[{{{n}: {n}}}]: {n}")) + .collect::>() + .join(",\n"); + let mixed_items = (0..200) + .map(|n| format!("{{{n}: {n}}}")) + .collect::>() + .join(",\n"); + let mixed_array = format!("[{mixed_items},\ninput.marker]"); + let mut failures = Vec::new(); + for (name, value, body) in [ + ( + "actual count", + object.clone(), + "p := [1 | not count(f(1), 300)]".into(), + ), + ( + "expected object", + object.clone(), + format!("p := [1 | not f(1, {object})]"), + ), + ("typed object", object, "p := f(1)".into()), + ("constant set", set, "p := f(1)".into()), + ("nested array", format!("[{items}]"), "p := f(1)".into()), + ("nested set", format!("{{{items}}}"), "p := f(1)".into()), + ( + "nested object", + format!("{{{nested_fields}}}"), + "p := f(1)".into(), + ), + ( + "compound keys", + format!("{{{compound_fields}}}"), + "p := f(1)".into(), + ), + ("mixed array", mixed_array.clone(), "p := f(1)".into()), + ( + "mixed expected array", + mixed_array.clone(), + format!("p := [1 | not f(1, {mixed_array})]"), + ), + ] { + for strict in [false, true] { + let mut engine = Engine::new(); + engine.set_strict_builtin_errors(strict); + engine.add_policy( + "test.rego".into(), + format!("package test\nimport rego.v1\nf(_) := {value}\n{body}"), + )?; + let input = Value::from_json_str(r#"{"marker":true}"#)?; + engine.set_input(input.clone()); + let interpreted = engine.eval_rule("data.test.p".into())?; + if name == "actual count" + || name.starts_with("expected") + || name == "mixed expected array" + { + assert_eq!(interpreted, Value::from_json_str("[]")?, "{name}"); + } + let compiled = engine.compile_with_entrypoint(&Rc::from("data.test.p"))?; + let program = match Compiler::compile_from_policy(&compiled, &["data.test.p"]) { + Ok(program) => program, + Err(error) => { + failures.push(format!("{name}, strict={strict}: {error}")); + continue; + } + }; + let json = program.serialize_json().map_err(anyhow::Error::msg)?; + let round_trip = + std::sync::Arc::new(Program::deserialize_json(&json).map_err(anyhow::Error::msg)?); + for (snapshot, program) in [("original", program), ("round-trip", round_trip)] { + for mode in [ExecutionMode::RunToCompletion, ExecutionMode::Suspendable] { + let mut vm = RegoVM::new(); + vm.set_strict_builtin_errors(strict); + vm.set_execution_mode(mode); + vm.load_program(program.clone()); + vm.set_input(input.clone()); + let actual = vm.execute_entry_point_by_name("data.test.p")?; + if actual != interpreted { + failures.push(format!( + "{name}, {snapshot}, strict={strict}, {mode:?}: typed value mismatch" + )); + } + if mode == ExecutionMode::Suspendable { + assert_eq!( + vm.execution_state(), + &ExecutionState::Completed { result: actual } + ); + } + } + } + } + } + assert!(failures.is_empty(), "{}", failures.join("\n")); + Ok(()) +} + +#[test] +fn typed_constant_collections_preserve_sets_after_json_round_trip() -> anyhow::Result<()> { + let mut failures = Vec::new(); + for body in [ + r#"p := [1 | not object.union({"s": object.keys({"a": 1})}, {}, {"s": {"a"}})]"#, + r#"p := [1 | not object.keys({"a": 1}, {"a"})]"#, + r#"p := [1 | not array.concat([object.keys({"a": 1})], [], [{"a"}])]"#, + r#"p := [1 | not object.union({"s": [object.keys({"a": 1})]}, {}, {"s": [{"a"}]})]"#, + r#"p := {"a"}"#, + r#"p := {{"a"}}"#, + r#"p := {"s": {"a"}}"#, + r#"p := [1 | not object.union({"s": object.keys({"": 1})}, {}, {"s": {""}})]"#, + r#"f(x) := {x, {"a"}} +p := f("b")"#, + r#"f(x) := {"s": {"a"}, "x": x} +p := f(1)"#, + r#"f(x) := {x: "v"} +p := [1 | not f(object.keys({"a": 1}), {{"a"}: "v"})]"#, + ] { + for strict in [false, true] { + let mut engine = Engine::new(); + engine.set_strict_builtin_errors(strict); + engine.add_policy( + "test.rego".into(), + format!("package test\nimport rego.v1\n{body}"), + )?; + let interpreted = engine.eval_rule("data.test.p".into())?; + let compiled = engine.compile_with_entrypoint(&Rc::from("data.test.p"))?; + let program = Compiler::compile_from_policy(&compiled, &["data.test.p"])?; + let json = program.serialize_json().map_err(anyhow::Error::msg)?; + let round_trip = + std::sync::Arc::new(Program::deserialize_json(&json).map_err(anyhow::Error::msg)?); + for (snapshot, program) in [("original", program), ("round-trip", round_trip)] { + for mode in [ExecutionMode::RunToCompletion, ExecutionMode::Suspendable] { + let mut vm = RegoVM::new(); + vm.set_strict_builtin_errors(strict); + vm.set_execution_mode(mode); + vm.load_program(program.clone()); + let actual = vm.execute_entry_point_by_name("data.test.p")?; + if actual != interpreted { + failures.push(format!("{body}, {snapshot}, strict={strict}, {mode:?}: {actual:?} != {interpreted:?}")); + } + if mode == ExecutionMode::Suspendable { + assert_eq!( + vm.execution_state(), + &ExecutionState::Completed { result: actual } + ); + } + } + } + } + } + assert!(failures.is_empty(), "{}", failures.join("\n")); + Ok(()) +} + #[test] fn negated_output_constants_survive_json_round_trip() -> anyhow::Result<()> { let mut divergences = Vec::new(); @@ -911,21 +1085,28 @@ fn constant_array_is_hoisted() { } #[test] -fn constant_set_is_hoisted() { +fn constant_set_preserves_type_after_json_round_trip() -> anyhow::Result<()> { let program = compile_rule( r#" package test p := x if { x := {1, 2, 3} } "#, ); - assert_no_collection_create(&program); let expected_set = Value::from( [1, 2, 3] .into_iter() .map(Value::from) .collect::>(), ); - assert_literal_exists(&program, &expected_set); + let json = program.serialize_json().map_err(anyhow::Error::msg)?; + let round_trip = + std::sync::Arc::new(Program::deserialize_json(&json).map_err(anyhow::Error::msg)?); + for program in [program, round_trip] { + let mut vm = RegoVM::new(); + vm.load_program(program); + assert_eq!(vm.execute_entry_point_by_name("data.test.p")?, expected_set); + } + Ok(()) } #[test]