Merge pull request #519 from anakrish/opa-rvm-3

Opa rvm 3
This commit is contained in:
Anand Krishnamoorthi
2025-12-16 14:11:08 -06:00
committed by GitHub
13 changed files with 545 additions and 23 deletions

View File

@@ -15,6 +15,8 @@ use anyhow::{bail, Result};
pub fn register(m: &mut builtins::BuiltinsMap<&'static str, builtins::BuiltinFcn>) {
m.insert("intersection", (intersection_of_set_of_sets, 1));
m.insert("union", (union_of_set_of_sets, 1));
m.insert("__builtin_sets.union", (binary_set_union, 2));
m.insert("__builtin_sets.intersection", (binary_set_intersection, 2));
}
pub fn intersection(expr1: &Expr, expr2: &Expr, v1: Value, v2: Value) -> Result<Value> {
@@ -35,6 +37,34 @@ pub fn difference(expr1: &Expr, expr2: &Expr, v1: Value, v2: Value) -> Result<Va
Ok(Value::from_set(s1.difference(&s2).cloned().collect()))
}
fn binary_set_union(
span: &Span,
params: &[Ref<Expr>],
args: &[Value],
_strict: bool,
) -> Result<Value> {
let name = "__builtin_sets.union";
ensure_args_count(span, name, params, args, 2)?;
let left = ensure_set(name, &params[0], args[0].clone())?;
let right = ensure_set(name, &params[1], args[1].clone())?;
Ok(Value::from_set(left.union(&right).cloned().collect()))
}
fn binary_set_intersection(
span: &Span,
params: &[Ref<Expr>],
args: &[Value],
_strict: bool,
) -> Result<Value> {
let name = "__builtin_sets.intersection";
ensure_args_count(span, name, params, args, 2)?;
let left = ensure_set(name, &params[0], args[0].clone())?;
let right = ensure_set(name, &params[1], args[1].clone())?;
Ok(Value::from_set(
left.intersection(&right).cloned().collect(),
))
}
fn intersection_of_set_of_sets(
span: &Span,
params: &[Ref<Expr>],

View File

@@ -15,6 +15,9 @@ pub enum CompilerError {
#[error("Unknown builtin function: {name}")]
UnknownBuiltinFunction { name: String },
#[error("the `with` keyword is not supported by the compiler yet")]
WithKeywordUnsupported,
#[error("internal: missing context for yield")]
MissingYieldContext,

View File

@@ -142,7 +142,7 @@ impl<'a> Compiler<'a> {
match op {
BinOp::Union => {
let builtin_index = self.get_builtin_index("sets.union")?;
let builtin_index = self.get_builtin_index("__builtin_sets.union")?;
let params = BuiltinCallParams {
dest,
builtin_index,
@@ -156,7 +156,7 @@ impl<'a> Compiler<'a> {
self.emit_instruction(Instruction::BuiltinCall { params_index }, span);
}
BinOp::Intersection => {
let builtin_index = self.get_builtin_index("sets.intersection")?;
let builtin_index = self.get_builtin_index("__builtin_sets.intersection")?;
let params = BuiltinCallParams {
dest,
builtin_index,

View File

@@ -10,7 +10,7 @@ mod queries;
mod references;
mod rules;
pub use error::{CompilerError, Result};
pub use error::{CompilerError, Result, SpannedCompilerError};
use crate::ast::ExprRef;
use crate::lexer::Span;

View File

@@ -38,6 +38,9 @@ impl<'a> Compiler<'a> {
stmts: &[&LiteralStmt],
) -> Result<()> {
for (idx, stmt) in stmts.iter().enumerate() {
if !stmt.with_mods.is_empty() {
return Err(CompilerError::WithKeywordUnsupported.at(&stmt.span));
}
let loop_exprs = self.get_statement_loops(stmt)?;
if !loop_exprs.is_empty() {

View File

@@ -471,6 +471,20 @@ impl<'a> Compiler<'a> {
::core::convert::identity(body_idx);
let previous_value_expr = self
.context_stack
.last()
.and_then(|ctx| ctx.value_expr.clone());
let mut body_value_expr =
body.assign.as_ref().map(|assign| assign.value.clone());
if body_value_expr.is_none() && body_idx == 0 {
body_value_expr = previous_value_expr.clone();
}
if let Some(context) = self.context_stack.last_mut() {
context.value_expr = body_value_expr.clone();
}
self.emit_instruction(
Instruction::RuleInit {
result_reg: result_register,
@@ -481,23 +495,23 @@ impl<'a> Compiler<'a> {
if !body.query.stmts.is_empty() {
self.compile_query(&body.query)?;
} else {
let value_expr_opt =
self.context_stack.last().unwrap().value_expr.clone();
if let Some(value_expr) = value_expr_opt {
let value_reg = self.compile_rego_expr(&value_expr)?;
self.emit_instruction(
Instruction::Move {
dest: result_register,
src: value_reg,
},
value_expr.span(),
);
}
} else if let Some(value_expr) = body_value_expr.clone() {
let value_reg = self.compile_rego_expr(&value_expr)?;
self.emit_instruction(
Instruction::Move {
dest: result_register,
src: value_reg,
},
value_expr.span(),
);
}
self.emit_instruction(Instruction::RuleReturn {}, &body.span);
if let Some(context) = self.context_stack.last_mut() {
context.value_expr = previous_value_expr;
}
self.pop_scope();
}
}

View File

@@ -1,6 +1,8 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT License.
use alloc::collections::BTreeSet;
use crate::number::Number;
use crate::value::Value;
@@ -23,6 +25,10 @@ impl RegoVM {
pub(super) fn sub_values(&self, a: &Value, b: &Value) -> Result<Value> {
match (a, b) {
(Value::Number(x), Value::Number(y)) => Ok(Value::from(x.sub(y)?)),
(Value::Set(left), Value::Set(right)) => {
let diff: BTreeSet<Value> = left.difference(right).cloned().collect();
Ok(Value::from_set(diff))
}
_ => Err(VmError::InvalidSubtraction {
left: a.clone(),
right: b.clone(),

View File

@@ -96,6 +96,10 @@ impl RegoVM {
}
}
}
// Once a body in this definition succeeds, remaining bodies
// are treated as else-branches and must not be evaluated.
break;
}
Err(_e) => {
continue;
@@ -502,7 +506,14 @@ impl RegoVM {
}
}
frame_data.current_body_index += 1;
if let Some(definition_bodies) = rule_info
.definitions
.get(frame_data.current_definition_index)
{
frame_data.current_body_index = definition_bodies.len();
} else {
frame_data.current_body_index += 1;
}
self.rule_frame_schedule_segment(frame_data, rule_info)
}

View File

@@ -18,13 +18,14 @@ use walkdir::WalkDir;
const OPA_REPO: &str = "https://github.com/open-policy-agent/opa";
const OPA_BRANCH: &str = "v1.2.0";
const PARTIAL_OBJECT_OVERRIDE_NOTE: &str =
"regression/partial-object override, different key type, query";
const OPA_TODO_FOLDERS: &[&str] = &[
"aggregates",
"baseandvirtualdocs",
"dataderef",
"defaultkeyword",
"elsekeyword",
"every",
"fix1863",
"functions",
@@ -32,10 +33,9 @@ const OPA_TODO_FOLDERS: &[&str] = &[
"partialobjectdoc",
"planner-ir",
"refheads",
"sets",
"type",
"virtualdocs",
"walkbuiltin",
// RVM Compiler does not support 'with' keyword yet.
"withkeyword",
];
@@ -267,6 +267,14 @@ fn is_not_valid_rule_path_error(err: &anyhow::Error) -> bool {
.any(|cause| cause.to_string().contains("not a valid rule path"))
}
fn is_with_keyword_unsupported_error(err: &anyhow::Error) -> bool {
err.chain().any(|cause| {
cause
.to_string()
.contains("`with` keyword is not supported")
})
}
fn maybe_verify_rvm_case(case: &TestCase, is_rego_v0_test: bool, actual: &Value) -> Result<()> {
if case.note == "defaultkeyword/function with var arg, ref head query" {
println!(
@@ -292,6 +300,14 @@ fn maybe_verify_rvm_case(case: &TestCase, is_rego_v0_test: bool, actual: &Value)
return Ok(());
}
if is_with_keyword_unsupported_error(&err) {
println!(
" skipping RVM check for '{}' (with keyword unsupported)",
case.note
);
return Ok(());
}
return Err(err);
}
};
@@ -376,9 +392,15 @@ fn run_opa_tests(opa_tests_dir: String, folders: &[String]) -> Result<()> {
for mut case in test.cases {
let is_json_schema_test = case.note.starts_with("json_verify_schema")
|| case.note.starts_with("json_match_schema");
let skip_rvm_validation = skip_rvm_for_folder;
let mut skip_rvm_validation = skip_rvm_for_folder;
if case.note == "reachable_paths/cycle_1022_3" {
if case.note == PARTIAL_OBJECT_OVERRIDE_NOTE {
println!(
" skipping RVM check for '{}' (needs suffix lookup on rule path)",
case.note
);
skip_rvm_validation = true;
} else if case.note == "reachable_paths/cycle_1022_3" {
// The OPA behavior is not well-defined.
// See: https://github.com/open-policy-agent/opa/issues/5871
// https://github.com/open-policy-agent/opa/issues/6128

View File

@@ -0,0 +1,126 @@
# Rego Else Rules Test Suite
# Exercises compiler support for else bodies, including assignment overrides and boolean fallback logic.
cases:
- note: else_rule_short_circuit
modules:
- |
package test
decision := 1 if {
1 == 1
}
else := 2 if {
1 == 1
}
query: data.test.decision
want_result: 1
- note: else_rule_fallback
modules:
- |
package test
decision := 1 if {
1 == 2
}
else := 2 if {
1 == 2
}
else := 3 if {
1 == 1
}
query: data.test.decision
want_result: 3
- note: else_rule_assignment_only
modules:
- |
package test
decision := 1 if {
1 == 2
}
else := 99
query: data.test.decision
want_result: 99
- note: else_rule_no_assignment_boolean
input:
method: "POST"
modules:
- |
package test
allow if {
input.method == "GET"
}
else if {
input.method == "POST"
}
query: data.test.allow
want_result: true
- note: else_rule_multiple_definitions
modules:
- |
package test
decision := "first" if {
false
}
else := "first-else" if {
false
}
decision := "second" if {
false
}
else := "second-else" if {
true
}
query: data.test.decision
want_result: "second-else"
- note: else_rule_function_fallback
modules:
- |
package test
f(x) := "small" if {
x < 5
}
else := "medium" if {
x < 10
}
f(x) := "large" if {
x >= 10
}
result := f(8)
query: data.test.result
want_result: "medium"
- note: else_rule_multiple_defined_single
modules:
- |
package ex
multiple_defined := false if {
false
}
else if {
true
}
else := false
query: data.ex.multiple_defined
want_result: true
- note: else_rule_boolean_middle_then_assignment
modules:
- |
package corner
corner_case := 7 if {
false
}
else := 6 if {
false
} else if {
true
}
else := 99
query: data.corner.corner_case
want_result: true

View File

@@ -67,3 +67,23 @@ cases:
- set!: [1, 2]
- set!: [3, 4]
- set!: ["a", "b"]
- note: set_difference_literals
data: {}
modules:
- |
package test
x := {2, 3} - {4, 2}
query: data.test.x
want_result:
set!: [3]
- note: set_intersection_literals
data: {}
modules:
- |
package test
y := {2, 3} & {4, 2}
query: data.test.y
want_result:
set!: [2]

View File

@@ -63,7 +63,8 @@ cases:
rule_infos:
- rule_type: Complete
definitions:
- [2, 5]
- [2]
- [5]
instructions:
- "CallRule { dest: 0, rule_index: 0 }"
- "Return { value: 0 }"
@@ -74,3 +75,43 @@ cases:
- "Load { dest: 1, literal_idx: 1 }"
- "RuleReturn {}"
want_result: "#undefined"
- note: call_rule_else_short_circuit
description: Rule definitions stop executing additional bodies once one succeeds
literals:
- 1
- 2
rule_infos:
- rule_type: Complete
definitions:
- [2, 5]
instructions:
- "CallRule { dest: 0, rule_index: 0 }"
- "Return { value: 0 }"
- "RuleInit { result_reg: 1, rule_index: 0 }"
- "Load { dest: 1, literal_idx: 0 }"
- "RuleReturn {}"
- "RuleInit { result_reg: 1, rule_index: 0 }"
- "Load { dest: 1, literal_idx: 1 }"
- "RuleReturn {}"
want_result: 1
- note: call_rule_else_fallback_on_failure
description: Failed bodies can still fall back to the next else body
literals:
- 99
rule_infos:
- rule_type: Complete
definitions:
- [2, 6]
instructions:
- "CallRule { dest: 0, rule_index: 0 }"
- "Return { value: 0 }"
- "RuleInit { result_reg: 1, rule_index: 0 }"
- "LoadBool { dest: 2, value: false }"
- "AssertCondition { condition: 2 }"
- "RuleReturn {}"
- "RuleInit { result_reg: 1, rule_index: 0 }"
- "Load { dest: 1, literal_idx: 0 }"
- "RuleReturn {}"
want_result: 99

View File

@@ -0,0 +1,246 @@
# Else Rule Test Suite
# Validates VM handling of rule bodies with else chains across rule types and execution modes.
cases:
- note: else_complete_short_circuit
description: First body succeeds and remaining else bodies are skipped
literals:
- 1
- 2
rule_infos:
- rule_type: Complete
definitions:
- [2, 6]
instructions:
- "CallRule { dest: 0, rule_index: 0 }"
- "Return { value: 0 }"
- "RuleInit { result_reg: 1, rule_index: 0 }"
- "Load { dest: 1, literal_idx: 0 }"
- "RuleReturn {}"
- "RuleInit { result_reg: 1, rule_index: 0 }"
- "Load { dest: 1, literal_idx: 1 }"
- "RuleReturn {}"
want_result: 1
- note: else_complete_fallback
description: First body fails, second else body succeeds
literals:
- 10
rule_infos:
- rule_type: Complete
definitions:
- [2, 6]
instructions:
- "CallRule { dest: 0, rule_index: 0 }"
- "Return { value: 0 }"
- "RuleInit { result_reg: 1, rule_index: 0 }"
- "LoadBool { dest: 2, value: false }"
- "AssertCondition { condition: 2 }"
- "RuleReturn {}"
- "RuleInit { result_reg: 1, rule_index: 0 }"
- "Load { dest: 1, literal_idx: 0 }"
- "RuleReturn {}"
want_result: 10
- note: else_complete_multi_failure_then_success
description: Multiple failing bodies before a later success
literals:
- 7
rule_infos:
- rule_type: Complete
definitions:
- [2, 6, 10]
instructions:
- "CallRule { dest: 0, rule_index: 0 }"
- "Return { value: 0 }"
- "RuleInit { result_reg: 1, rule_index: 0 }"
- "LoadBool { dest: 2, value: false }"
- "AssertCondition { condition: 2 }"
- "RuleReturn {}"
- "RuleInit { result_reg: 1, rule_index: 0 }"
- "LoadBool { dest: 2, value: false }"
- "AssertCondition { condition: 2 }"
- "RuleReturn {}"
- "RuleInit { result_reg: 1, rule_index: 0 }"
- "Load { dest: 1, literal_idx: 0 }"
- "RuleReturn {}"
want_result: 7
- note: else_complete_all_fail_default_literal
description: All bodies fail and default literal is returned
literals:
- 42
rule_infos:
- rule_type: Complete
default_literal_index: 0
definitions:
- [2, 6]
instructions:
- "CallRule { dest: 0, rule_index: 0 }"
- "Return { value: 0 }"
- "RuleInit { result_reg: 1, rule_index: 0 }"
- "LoadBool { dest: 2, value: false }"
- "AssertCondition { condition: 2 }"
- "RuleReturn {}"
- "RuleInit { result_reg: 1, rule_index: 0 }"
- "LoadBool { dest: 2, value: false }"
- "AssertCondition { condition: 2 }"
- "RuleReturn {}"
want_result: 42
- note: else_partial_set
description: Partial set rule initializes result even if all bodies fail
literals:
- []
rule_infos:
- rule_type: PartialSet
definitions:
- [2, 6]
instructions:
- "CallRule { dest: 0, rule_index: 0 }"
- "Return { value: 0 }"
- "RuleInit { result_reg: 1, rule_index: 0 }"
- "LoadBool { dest: 2, value: false }"
- "AssertCondition { condition: 2 }"
- "RuleReturn {}"
- "RuleInit { result_reg: 1, rule_index: 0 }"
- "LoadBool { dest: 2, value: false }"
- "AssertCondition { condition: 2 }"
- "RuleReturn {}"
want_result:
set!: []
- note: else_partial_object
description: Partial object rule emits value from first successful branch
literals:
- {}
- "key"
- 5
rule_infos:
- rule_type: PartialObject
definitions:
- [2, 6]
instructions:
- "CallRule { dest: 0, rule_index: 0 }"
- "Return { value: 0 }"
- "RuleInit { result_reg: 1, rule_index: 0 }"
- "LoadBool { dest: 2, value: false }"
- "AssertCondition { condition: 2 }"
- "RuleReturn {}"
- "RuleInit { result_reg: 1, rule_index: 0 }"
- "Load { dest: 1, literal_idx: 0 }"
- "Load { dest: 2, literal_idx: 1 }"
- "Load { dest: 3, literal_idx: 2 }"
- "ObjectSet { obj: 1, key: 2, value: 3 }"
- "RuleReturn {}"
want_result: { "key": 5 }
- note: else_function_rule
description: Function rule respects else chain and caches final value
literals:
- 0 # function rule index
- 1 # argument value (unused)
- 2 # return value from else body
instruction_params:
function_call_params:
- func: 0
dest: 3
args: [1]
rule_infos:
- rule_type: Complete
function_info:
param_names: ["x"]
num_params: 1
definitions:
- [4, 8]
instructions:
- "Load { dest: 0, literal_idx: 0 }"
- "Load { dest: 1, literal_idx: 1 }"
- "FunctionCall { params_index: 0 }"
- "Return { value: 3 }"
- "RuleInit { result_reg: 0, rule_index: 0 }"
- "LoadBool { dest: 2, value: false }"
- "AssertCondition { condition: 2 }"
- "RuleReturn {}"
- "RuleInit { result_reg: 0, rule_index: 0 }"
- "Load { dest: 0, literal_idx: 2 }"
- "RuleReturn {}"
want_result: 2
- note: else_assignment_only
description: Assignment-only else block without query statements
literals:
- 99
rule_infos:
- rule_type: Complete
definitions:
- [2, 6]
instructions:
- "CallRule { dest: 0, rule_index: 0 }"
- "Return { value: 0 }"
- "RuleInit { result_reg: 1, rule_index: 0 }"
- "LoadBool { dest: 2, value: false }"
- "AssertCondition { condition: 2 }"
- "RuleReturn {}"
- "RuleInit { result_reg: 1, rule_index: 0 }"
- "Load { dest: 1, literal_idx: 0 }"
- "RuleReturn {}"
want_result: 99
- note: else_nested_callrule
description: Outer rule observes inner rule else short-circuit behaviour
literals:
- 5
rule_infos:
- rule_type: Complete
definitions:
- [2]
- rule_type: Complete
definitions:
- [5, 9]
rule_tree:
data:
outer:
allow: 0
inner:
value: 1
instructions:
- "CallRule { dest: 0, rule_index: 0 }"
- "Return { value: 0 }"
- "RuleInit { result_reg: 1, rule_index: 0 }"
- "CallRule { dest: 1, rule_index: 1 }"
- "RuleReturn {}"
- "RuleInit { result_reg: 1, rule_index: 1 }"
- "LoadBool { dest: 3, value: false }"
- "AssertCondition { condition: 3 }"
- "RuleReturn {}"
- "RuleInit { result_reg: 1, rule_index: 1 }"
- "Load { dest: 1, literal_idx: 0 }"
- "RuleReturn {}"
want_result: 5
- note: else_suspendable_step
description: Suspendable execution mode respects else short-circuiting
literals:
- 4
- "await-else"
rule_infos:
- rule_type: Complete
definitions:
- [2, 7]
execution_mode: suspendable
instructions:
- "CallRule { dest: 0, rule_index: 0 }"
- "Return { value: 0 }"
- "RuleInit { result_reg: 1, rule_index: 0 }"
- "Load { dest: 1, literal_idx: 0 }"
- "Load { dest: 2, literal_idx: 1 }"
- "HostAwait { dest: 3, arg: 1, id: 2 }"
- "RuleReturn {}"
- "RuleInit { result_reg: 1, rule_index: 0 }"
- "Load { dest: 1, literal_idx: 0 }"
- "RuleReturn {}"
host_await_responses:
- id: "await-else"
value: "resume-ok"
want_result: 4