feat: Soft-assert mode for builtin out-params under not

- Add a scoped soft_assert_mode to the compiler so `not` statements compile their subexpressions without emitting hard AssertCondition/AssertNotUndefined instructions.
- Teach binding-plan application to return an optional result register; equality plans now yield a boolean in soft mode, allowing not abs(-5 , 3) to succeed instead of aborting.
- Update function-call, loop, and rule plumbing to consume the new binding-plan outcome, including copying the produced register when an out-parameter equality is used.
- Trim the OPA TODO list to the remaining troublesome folders.

Signed-off-by: Anand Krishnamoorthi <anakrish@microsoft.com>
This commit is contained in:
Anand Krishnamoorthi
2025-12-02 15:06:23 -06:00
parent 30bd134a0b
commit b7b3d3ec87
10 changed files with 83 additions and 80 deletions

View File

@@ -40,16 +40,12 @@ pub fn create_parameter_binding_plan<T: VariableBindingContext>(
scoping: ScopingMode,
) -> Result<BindingPlan> {
let mut newly_bound = BTreeSet::new();
let destructuring_plan = create_destructuring_plan_with_tracking(
param_expr,
context,
scoping,
&mut newly_bound,
)
.ok_or_else(|| BindingPlannerError::FailedToCreateDestructuringPlan {
plan_type: "parameter".to_string(),
span: param_expr.span().clone(),
})?;
let destructuring_plan =
create_destructuring_plan_with_tracking(param_expr, context, scoping, &mut newly_bound)
.ok_or_else(|| BindingPlannerError::FailedToCreateDestructuringPlan {
plan_type: "parameter".to_string(),
span: param_expr.span().clone(),
})?;
if scoping == ScopingMode::AllowShadowing {
validate_pattern_bindings(param_expr, &newly_bound, context)?;

View File

@@ -42,7 +42,7 @@ impl<'a> Compiler<'a> {
rhs_expr, lhs_plan, ..
} => {
let rhs_reg = self.compile_rego_expr_with_span(rhs_expr, rhs_expr.span(), false)?;
self.apply_destructuring_plan(
let _ = self.apply_destructuring_plan(
lhs_plan,
rhs_reg,
span,
@@ -54,21 +54,31 @@ impl<'a> Compiler<'a> {
rhs_expr, lhs_plan, ..
} => {
let rhs_reg = self.compile_rego_expr_with_span(rhs_expr, rhs_expr.span(), false)?;
self.apply_destructuring_plan(lhs_plan, rhs_reg, span, PlanContext::Assignment)?;
let _ = self.apply_destructuring_plan(
lhs_plan,
rhs_reg,
span,
PlanContext::Assignment,
)?;
Ok(self.load_bool_literal(true, span))
}
AssignmentPlan::EqualsBindRight {
lhs_expr, rhs_plan, ..
} => {
let lhs_reg = self.compile_rego_expr_with_span(lhs_expr, lhs_expr.span(), false)?;
self.apply_destructuring_plan(rhs_plan, lhs_reg, span, PlanContext::Assignment)?;
let _ = self.apply_destructuring_plan(
rhs_plan,
lhs_reg,
span,
PlanContext::Assignment,
)?;
Ok(self.load_bool_literal(true, span))
}
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)?;
self.apply_destructuring_plan(
let _ = self.apply_destructuring_plan(
value_plan,
value_reg,
span,
@@ -124,7 +134,7 @@ impl<'a> Compiler<'a> {
plan: &BindingPlan,
value_register: Register,
span: &Span,
) -> Result<()> {
) -> Result<Option<Register>> {
match plan {
BindingPlan::Assignment { .. } => {
bail!("assignment binding plans should be handled via compile_assignment_plan")
@@ -160,9 +170,11 @@ impl<'a> Compiler<'a> {
span: &Span,
) -> Result<()> {
if let (Some(plan), Some(register)) = (key_plan, key_register) {
self.apply_destructuring_plan(plan, register, span, PlanContext::SomeIn)?;
let _ = self.apply_destructuring_plan(plan, register, span, PlanContext::SomeIn)?;
}
self.apply_destructuring_plan(value_plan, value_register, span, PlanContext::SomeIn)
let _ =
self.apply_destructuring_plan(value_plan, value_register, span, PlanContext::SomeIn)?;
Ok(())
}
fn apply_destructuring_plan(
@@ -171,7 +183,7 @@ impl<'a> Compiler<'a> {
value_register: Register,
span: &Span,
context: PlanContext,
) -> Result<()> {
) -> Result<Option<Register>> {
match plan {
DestructuringPlan::Var(name_span) => {
self.bind_variable(name_span, value_register, span, context)?;
@@ -189,6 +201,9 @@ impl<'a> Compiler<'a> {
},
span,
);
if self.soft_assert_mode {
return Ok(Some(cmp_reg));
}
self.emit_instruction(Instruction::AssertCondition { condition: cmp_reg }, span);
}
DestructuringPlan::EqualityValue(expected_value) => {
@@ -202,6 +217,9 @@ impl<'a> Compiler<'a> {
},
span,
);
if self.soft_assert_mode {
return Ok(Some(cmp_reg));
}
self.emit_instruction(Instruction::AssertCondition { condition: cmp_reg }, span);
}
DestructuringPlan::Array { element_plans } => {
@@ -225,7 +243,8 @@ impl<'a> Compiler<'a> {
span,
);
}
self.apply_destructuring_plan(element_plan, element_reg, span, context)?;
let _ =
self.apply_destructuring_plan(element_plan, element_reg, span, context)?;
}
}
DestructuringPlan::Object {
@@ -249,7 +268,7 @@ impl<'a> Compiler<'a> {
},
span,
);
self.apply_destructuring_plan(field_plan, field_reg, span, context)?;
let _ = self.apply_destructuring_plan(field_plan, field_reg, span, context)?;
}
for (key_expr, field_plan) in dynamic_fields {
@@ -270,11 +289,11 @@ impl<'a> Compiler<'a> {
},
span,
);
self.apply_destructuring_plan(field_plan, field_reg, span, context)?;
let _ = self.apply_destructuring_plan(field_plan, field_reg, span, context)?;
}
}
}
Ok(())
Ok(None)
}
fn bind_variable(

View File

@@ -117,9 +117,20 @@ impl<'a> Compiler<'a> {
}
if let Some((plan, plan_span)) = &out_param_plan {
self.apply_binding_plan(plan, dest, plan_span)
let plan_result = self
.apply_binding_plan(plan, dest, plan_span)
.map_err(|err| CompilerError::from(err).at(plan_span))?;
self.emit_instruction(Instruction::LoadBool { dest, value: true }, &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);
}
}
Ok(dest)

View File

@@ -249,7 +249,8 @@ impl<'a> Compiler<'a> {
let body_start = self.program.instructions.len() as u16;
if let Some((binding_plan, plan_span)) = key_binding_plan.as_ref() {
self.apply_binding_plan(binding_plan, key_reg, plan_span)
let _ = self
.apply_binding_plan(binding_plan, key_reg, plan_span)
.map_err(|e| CompilerError::from(e).at(plan_span))?;
}

View File

@@ -125,6 +125,7 @@ pub struct Compiler<'a> {
current_rule_path: String,
current_call_stack: Vec<u16>,
entry_points: IndexMap<String, usize>,
soft_assert_mode: bool,
}
impl<'a> Compiler<'a> {
@@ -157,6 +158,18 @@ impl<'a> Compiler<'a> {
current_rule_path: String::new(),
current_call_stack: Vec::new(),
entry_points: IndexMap::new(),
soft_assert_mode: false,
}
}
pub(super) fn with_soft_assert_mode<F, R>(&mut self, enabled: bool, f: F) -> R
where
F: FnOnce(&mut Self) -> R,
{
let previous = self.soft_assert_mode;
self.soft_assert_mode = enabled;
let result = f(self);
self.soft_assert_mode = previous;
result
}
}

View File

@@ -131,7 +131,7 @@ impl<'a> Compiler<'a> {
if !self.program.builtin_info_table.is_empty() {
self.program
.initialize_resolved_builtins()
.map_err(|err| CompilerError::from(err))?;
.map_err(CompilerError::from)?;
}
Ok(self.program)

View File

@@ -221,7 +221,9 @@ impl<'a> Compiler<'a> {
}
}
ast::Literal::NotExpr { expr, .. } => {
let expr_reg = self.compile_rego_expr_with_span(expr, expr.span(), false)?;
let expr_reg = self.with_soft_assert_mode(true, |compiler| {
compiler.compile_rego_expr_with_span(expr, expr.span(), false)
})?;
let negated_reg = self.alloc_register();
self.emit_instruction(

View File

@@ -368,7 +368,8 @@ impl<'a> Compiler<'a> {
self.expect_binding_plan_for_expr(arg, &context_desc)?;
if let BindingPlan::Parameter { .. } = &binding_plan {
self.apply_binding_plan(&binding_plan, param_reg, arg.span())
let _ = self
.apply_binding_plan(&binding_plan, param_reg, arg.span())
.map_err(|e| CompilerError::from(e).at(arg.span()))?;
} else {
return Err(CompilerError::UnexpectedBindingPlan {

View File

@@ -20,21 +20,9 @@ const OPA_REPO: &str = "https://github.com/open-policy-agent/opa";
const OPA_BRANCH: &str = "v1.2.0";
const OPA_TODO_FOLDERS: &[&str] = &[
"arithmetic",
"aggregates",
"array",
"base64builtins",
"base64urlbuiltins",
"baseandvirtualdocs",
"bitsand",
"bitsnegate",
"bitsor",
"bitsshiftleft",
"bitsshiftright",
"bitsxor",
"casts",
"comparisonexpr",
"comprehensions",
"dataderef",
"defaultkeyword",
"disjunction",
@@ -44,57 +32,17 @@ const OPA_TODO_FOLDERS: &[&str] = &[
"example",
"fix1863",
"functions",
"functionerrors",
"globmatch",
"globquotemeta",
"hexbuiltins",
"indirectreferences",
"intersection",
"jsonbuiltins",
"jsonfilter",
"jsonfilteridempotent",
"jsonremove",
"jsonremoveidempotent",
"jsonschema",
"netcidrcontains",
"netcidrisvalid",
"numbersrange",
"objectfilter",
"objectfilteridempotent",
"objectfilternonstringkey",
"objectget",
"objectremove",
"objectremoveidempotent",
"objectremovenonstringkey",
"objectunion",
"partialdocconstants",
"partialobjectdoc",
"planner-ir",
"rand",
"reachable",
"refheads",
"regexfind",
"regexfindallstringsubmatch",
"regexisvalid",
"regexmatchtemplate",
"regexsplit",
"replacen",
"semvercompare",
"semverisvalid",
"sets",
"strings",
"time",
"trim",
"trimleft",
"trimprefix",
"trimright",
"trimspace",
"trimsuffix",
"type",
"typebuiltin",
"typenamebuiltin",
"union",
"urlbuiltins",
"varreferences",
"virtualdocs",
"walkbuiltin",

View File

@@ -81,3 +81,15 @@ cases:
}
query: data.test.rule5
want_result: "#undefined"
- note: builtin_out_param_negated_equality
data: {}
modules:
- |
package test
rule5 if {
not abs(-5, 3)
}
query: data.test.rule5
want_result: true