Repository navigation
fix(rvm): enforce definedness and structural match types - #930
Draft
Anand Krishnamoorthi (anakrish) wants to merge 9 commits into
Draft
Anand Krishnamoorthi (anakrish) wants to merge 9 commits into
Anand Krishnamoorthi (anakrish) wants to merge 9 commits into
Conversation
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved critical findings expose RVM/interpreter mismatches in undefined handling, negated-call evaluation, and nested matching, including skipped or incorrectly raised strict-mode errors.
7 open issues
What changed in this PR
This PR tightens Rego matching in the RVM to align with interpreter semantics, distinguishing undefined values from valid false and null results and enforcing collection-pattern types.
Changes:
- Require defined values for assignments, ignored outputs, and user-function inputs.
- Add local failure handling under negation and comparison of binding-free relational outputs.
- Add interpreter/native/suspendable regression coverage and document the intended matching behavior.
| File | Description |
|---|---|
| tests/rvm/compiler.rs | Adds cross-engine regressions for definedness, structural matching, negation, and strict errors. |
| src/languages/rego/compiler/function_calls.rs | Checks user-input definedness and handles negated relational outputs. |
| src/languages/rego/compiler/destructuring.rs | Adds definedness checks, collection-type validation, and local match-failure branches. |
| docs/rvm/architecture.md | Documents definedness, structural matching, and negated-call behavior. |
Review effort: Max
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Critical destructuring issues cause VM/interpreter divergence by discarding soft-match results and trigger register overflow on previously supported wide nested patterns.
2 open issues
7 resolved since last review
Review effort: Max
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
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
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
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
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
Anand Krishnamoorthi (anakrish)
force-pushed
the
rvm-priority-one-match-boundaries
branch
from
November 10, 2026 00:31
9a4cdd5 to
db2b529
Compare
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved findings identify incorrect negated-call results, a test compilation failure when arc is disabled, and register overflow for large constant sets.
5 open issues
2 resolved since last review
Review effort: Max
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
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Critical issues cause interpreter/RVM disagreements in negated assignments and object matching, alter non-string keys during serialization, and prevent RVM tests from compiling without the arc feature.
5 open issues
2 resolved since last review
Review effort: Max
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
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
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved critical compiler bugs can trigger register overflow and change policy results through incorrect duplicate-key ordering, unary negation, and type preservation during serialization.
8 open issues
4 resolved since last review
Review effort: Max
| 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), |
Comment on lines
+66
to
+67
| pub(in crate::languages::rego::compiler) fn try_eval_literal(expr: &Expr) -> Option<Value> { | ||
| try_eval_const(expr).filter(literal_round_trips) |
Comment on lines
+143
to
+144
| if let Some(obj) = | ||
| all_const.filter(|object| object.keys().all(|key| matches!(key, Value::String(_)))) |
Comment on lines
+143
to
+144
| if let Some(obj) = | ||
| all_const.filter(|object| object.keys().all(|key| matches!(key, Value::String(_)))) |
Comment on lines
+143
to
+144
| if let Some(obj) = | ||
| all_const.filter(|object| object.keys().all(|key| matches!(key, Value::String(_)))) |
| error.to_string().contains("unsafe"), | ||
| "{call}, strict={strict}: {error}" | ||
| ); | ||
| eprintln!("{call}, strict={strict}: interpreter {error}"); |
| 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:?}"); |
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
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Require defined values for unused assignments, ignored outputs, and user-function inputs. Reject array/object patterns against the wrong collection type while preserving false/null values and local match failure under negation.
Adds interpreter/native/suspendable regression coverage. Debug CI, formatting, clippy, feature checks, and no_std checks pass.