From 832348ba434e5b587a9eb66af966567e06f2c153 Mon Sep 17 00:00:00 2001 From: Alex Plotnick Date: Wed, 20 Jan 2021 17:28:42 -0700 Subject: [PATCH 01/18] Refactor some *_op_helper methods into query_op_helper Done: arithmetic, comparisons. Todo: in, isa. Probably not: unify, dot. --- polar-core/src/partial/partial.rs | 7 +- polar-core/src/vm.rs | 376 +++++++++++--------------- polar-core/tests/integration_tests.rs | 2 - 3 files changed, 160 insertions(+), 225 deletions(-) diff --git a/polar-core/src/partial/partial.rs b/polar-core/src/partial/partial.rs index 3b1fcfe915..ecf1c2cf53 100644 --- a/polar-core/src/partial/partial.rs +++ b/polar-core/src/partial/partial.rs @@ -163,7 +163,7 @@ impl Operation { | (Value::Boolean(_), Value::Number(_)) | (Value::Boolean(_), Value::Boolean(_)) | (Value::String(_), Value::String(_)) => { - if compare(&o.operator, left, right) { + if compare(o.operator, &left, &right) { TRUE } else { self.consistent = false; @@ -937,9 +937,8 @@ mod test { let p = Polar::new(); p.load_str("f(x) if x = x + 0;")?; let mut q = p.new_query_from_term(term!(call!("f", [sym!("x")])), false); - let error = q.next_event().unwrap_err(); - assert!(matches!(error, PolarError { - kind: ErrorKind::Runtime(RuntimeError::Unsupported { .. }), ..})); + assert_partial_expression!(next_binding(&mut q)?, "x", "_this + 0 = _this"); + assert_query_done!(q); Ok(()) } diff --git a/polar-core/src/vm.rs b/polar-core/src/vm.rs index d3dbe3da56..3a7abe87ef 100644 --- a/polar-core/src/vm.rs +++ b/polar-core/src/vm.rs @@ -194,23 +194,18 @@ pub fn cycle_constraints(cycle: Vec) -> Operation { constraints } -pub fn compare(op: &Operator, mut left: Term, mut right: Term) -> bool { +// TODO(ap): don't panic. +pub fn compare(op: Operator, left: &Term, right: &Term) -> bool { // Coerce booleans to integers. - fn to_int(x: bool) -> i64 { + fn to_int(x: bool) -> Numeric { if x { - 1 + Numeric::Integer(1) } else { - 0 + Numeric::Integer(0) } } - if let Value::Boolean(x) = left.value() { - left = left.clone_with_value(Value::Number(Numeric::Integer(to_int(*x)))); - } - if let Value::Boolean(x) = right.value() { - right = right.clone_with_value(Value::Number(Numeric::Integer(to_int(*x)))); - } - fn compare(op: &Operator, left: T, right: T) -> bool { + fn compare(op: Operator, left: T, right: T) -> bool { match op { Operator::Lt => left < right, Operator::Leq => left <= right, @@ -218,14 +213,17 @@ pub fn compare(op: &Operator, mut left: Term, mut right: Term) -> bool { Operator::Geq => left >= right, Operator::Eq => left == right, Operator::Neq => left != right, - _ => unreachable!("{:?} is not a comparison operator", op), + _ => panic!("`{}` is not a comparison operator", op.to_polar()), } } match (left.value(), right.value()) { + (Value::Boolean(l), Value::Boolean(r)) => compare(op, &to_int(*l), &to_int(*r)), + (Value::Boolean(l), Value::Number(r)) => compare(op, &to_int(*l), r), + (Value::Number(l), Value::Boolean(r)) => compare(op, l, &to_int(*r)), (Value::Number(l), Value::Number(r)) => compare(op, l, r), (Value::String(l), Value::String(r)) => compare(op, l, r), - _ => unreachable!("{} {} {}", left.to_polar(), op.to_polar(), right.to_polar()), + _ => panic!("{} {} {}", left.to_polar(), op.to_polar(), right.to_polar()), } } @@ -1551,8 +1549,8 @@ impl PolarVirtualMachine { Value::Call(predicate) => { self.query_for_predicate(predicate.clone())?; } - Value::Expression(Operation { operator, args }) => { - return self.query_for_operation(&term, *operator, args.clone()); + Value::Expression(_) => { + return self.query_for_operation(&term); } _ => { let term = self.deref(term); @@ -1593,13 +1591,10 @@ impl PolarVirtualMachine { self.append_goals(goals) } - fn query_for_operation( - &mut self, - term: &Term, - operator: Operator, - mut args: Vec, - ) -> PolarResult { - match operator { + fn query_for_operation(&mut self, term: &Term) -> PolarResult { + let operation = term.value().as_expression().unwrap(); + let mut args = operation.args.clone(); + match operation.operator { Operator::And => { // Query for each conjunct. self.push_goal(Goal::TraceStackPop)?; @@ -1660,24 +1655,24 @@ impl PolarVirtualMachine { self.push_goal(Goal::Unify { left, right })? } Operator::Dot => self.dot_op_helper(args)?, - op @ Operator::Lt - | op @ Operator::Gt - | op @ Operator::Leq - | op @ Operator::Geq - | op @ Operator::Eq - | op @ Operator::Neq => { - return self.comparison_op_helper(term, op, args); + Operator::Lt + | Operator::Gt + | Operator::Leq + | Operator::Geq + | Operator::Eq + | Operator::Neq => { + return self.comparison_op_helper(term); } - op @ Operator::Add - | op @ Operator::Sub - | op @ Operator::Mul - | op @ Operator::Div - | op @ Operator::Mod - | op @ Operator::Rem => { - return self.arithmetic_op_helper(term, op, args); + Operator::Add + | Operator::Sub + | Operator::Mul + | Operator::Div + | Operator::Mod + | Operator::Rem => { + return self.arithmetic_op_helper(term); } - op @ Operator::In => return self.in_op_helper(term, op, args), + Operator::In => return self.in_op_helper(term), Operator::Debug => { let mut message = "".to_string(); @@ -1893,13 +1888,13 @@ impl PolarVirtualMachine { } #[allow(clippy::many_single_char_names)] - fn in_op_helper( - &mut self, - term: &Term, - op: Operator, - args: Vec, - ) -> PolarResult { + fn in_op_helper(&mut self, term: &Term) -> PolarResult { + let operation = term.value().as_expression().unwrap(); + let op = operation.operator; + + let args = operation.args.clone(); assert_eq!(args.len(), 2); + let item = &args[0]; let iterable = &args[1]; match (item.value(), iterable.value()) { @@ -1926,25 +1921,15 @@ impl PolarVirtualMachine { match (self.variable_state(l), self.variable_state(r)) { (VariableState::Bound(item), _) => { let args = vec![item, iterable.clone()]; - return self.in_op_helper( - &term.clone_with_value(Value::Expression(Operation { - operator: op, - args: args.clone(), - })), - op, - args, - ); + return self.in_op_helper(&term.clone_with_value(Value::Expression( + Operation { operator: op, args }, + ))); } (_, VariableState::Bound(iterable)) => { let args = vec![item.clone(), iterable]; - return self.in_op_helper( - &term.clone_with_value(Value::Expression(Operation { - operator: op, - args: args.clone(), - })), - op, - args, - ); + return self.in_op_helper(&term.clone_with_value(Value::Expression( + Operation { operator: op, args }, + ))); } (VariableState::Unbound, VariableState::Unbound) => { let constraint = op!(And, term.clone()); @@ -1983,12 +1968,7 @@ impl PolarVirtualMachine { VariableState::Bound(iterable) => { let args = vec![item.clone(), iterable]; return self.in_op_helper( - &term.clone_with_value(Value::Expression(Operation { - operator: op, - args: args.clone(), - })), - op, - args, + &term.clone_with_value(Value::Expression(Operation { operator: op, args })), ); } VariableState::Cycle(c) => { @@ -2090,171 +2070,72 @@ impl PolarVirtualMachine { Ok(QueryEvent::None) } - /// Evaluate arithmetic operations. - fn arithmetic_op_helper( - &mut self, - term: &Term, - op: Operator, - args: Vec, - ) -> PolarResult { - assert_eq!(args.len(), 3); - let left_term = self.deref(&args[0]); - let right_term = self.deref(&args[1]); - let result = &args[2]; - assert!(matches!(result.value(), Value::Variable(_))); - - self.log_with( - || { - format!( - "MATH: {} {} {} = {}", - left_term.to_polar(), - op.to_polar(), - right_term.to_polar(), - result.to_polar() - ) - }, - &[&left_term, &right_term, result], - ); - - match (left_term.value(), right_term.value()) { - (Value::Number(left), Value::Number(right)) => { - if let Some(answer) = match op { - Operator::Add => *left + *right, - Operator::Sub => *left - *right, - Operator::Mul => *left * *right, - Operator::Div => *left / *right, - Operator::Mod => (*left).modulo(*right), - Operator::Rem => *left % *right, - _ => { - return Err(self.set_error_context( - term, - error::RuntimeError::Unsupported { - msg: format!("numeric operation {}", op.to_polar()), - }, - )) - } - } { - self.push_goal(Goal::Unify { - left: term.clone_with_value(Value::Number(answer)), - right: result.clone(), - })?; - } else { - return Err(self.set_error_context( - term, - error::RuntimeError::ArithmeticError { - msg: term.to_polar(), - }, - )); - } - } - (_, _) => { - return Err(self.set_error_context( - term, - error::RuntimeError::Unsupported { - msg: format!("unsupported arithmetic operands: {}", term.to_polar()), - }, - )) - } - } - Ok(QueryEvent::None) - } - - /// Evaluate comparisons. #[allow(clippy::many_single_char_names)] - fn comparison_op_helper( - &mut self, - term: &Term, - op: Operator, - args: Vec, - ) -> PolarResult { - assert_eq!(args.len(), 2); - let left = args[0].clone(); - let right = args[1].clone(); - self.log_with( - || { - format!( - "CMP: {} {} {}", - left.to_polar(), - op.to_polar(), - right.to_polar(), - ) - }, - &[&left, &right], - ); + fn query_op_helper(&mut self, term: &Term, eval: F) -> PolarResult + where + F: Fn(&Operation) -> Result, + { + let operation = term.value().as_expression().unwrap(); + let op = operation.operator; - // Do the comparison. + let mut args = operation.args.clone(); + assert!(args.len() >= 2); + let left = &args[0]; + let right = &args[1]; match (left.value(), right.value()) { (Value::Expression(_), _) | (_, Value::Expression(_)) | (Value::RestVariable(_), _) | (_, Value::RestVariable(_)) => { - unreachable!("invalid syntax") + panic!("invalid query"); } - (Value::Number(_), Value::Number(_)) - | (Value::Number(_), Value::Boolean(_)) - | (Value::Boolean(_), Value::Number(_)) - | (Value::Boolean(_), Value::Boolean(_)) - | (Value::String(_), Value::String(_)) => { - if !compare(&op, left, right) { - self.push_goal(Goal::Backtrack)?; - } - Ok(QueryEvent::None) - } (Value::ExternalInstance(_), Value::ExternalInstance(_)) => { - // Generate symbol for external op result and bind to `false` (default) + // Generate symbol for external result and bind to `false` (default). let (call_id, answer) = self.new_call_var("external_op_result", Value::Boolean(false)); - // append unify goal to be evaluated after external op result is returned & bound + // Unify the answer with the external result. + // TODO(ap): true??? self.push_goal(Goal::Unify { left: answer, right: Term::new_temporary(Value::Boolean(true)), })?; + + // Emit an event for the external operation. Ok(QueryEvent::ExternalOp { call_id, operator: op, - args: vec![left, right], + args: vec![left.clone(), right.clone()], }) } + (Value::Variable(l), Value::Variable(r)) => { // Two variables. match (self.variable_state(l), self.variable_state(r)) { - (VariableState::Unbound, VariableState::Unbound) => { - self.constrain(&op!(And, term.clone()))?; - } (VariableState::Bound(x), _) => { - let args = vec![x, right]; - return self.comparison_op_helper( - &term.clone_with_value(Value::Expression(Operation { - operator: op, - args: args.clone(), - })), - op, - args, - ); + args[0] = x; + self.push_goal(Goal::Query { + term: Operation { operator: op, args }.into_term(), + })?; } (_, VariableState::Bound(y)) => { - let args = vec![left, y]; - return self.comparison_op_helper( - &term.clone_with_value(Value::Expression(Operation { - operator: op, - args: args.clone(), - })), - op, - args, - ); + args[1] = y; + self.push_goal(Goal::Query { + term: Operation { operator: op, args }.into_term(), + })?; + } + (VariableState::Unbound, VariableState::Unbound) => { + self.constrain(&op!(And, term.clone()))?; } (VariableState::Cycle(c), VariableState::Cycle(d)) => { let mut e = cycle_constraints(c); e.merge_constraints(cycle_constraints(d)); - let e = e.clone_with_new_constraint(term.clone()); - self.constrain(&e)?; + self.constrain(&e.clone_with_new_constraint(term.clone()))?; } (VariableState::Partial(e), VariableState::Unbound) | (VariableState::Unbound, VariableState::Partial(e)) => { - let e = e.clone_with_new_constraint(term.clone()); - self.constrain(&e)?; + self.constrain(&e.clone_with_new_constraint(term.clone()))?; } (VariableState::Partial(mut e), VariableState::Partial(f)) => { e.merge_constraints(f); @@ -2263,13 +2144,12 @@ impl PolarVirtualMachine { (VariableState::Partial(mut e), VariableState::Cycle(c)) | (VariableState::Cycle(c), VariableState::Partial(mut e)) => { e.merge_constraints(cycle_constraints(c)); - let e = e.clone_with_new_constraint(term.clone()); - self.constrain(&e)?; + self.constrain(&e.clone_with_new_constraint(term.clone()))?; } (VariableState::Cycle(c), VariableState::Unbound) | (VariableState::Unbound, VariableState::Cycle(c)) => { - let e = cycle_constraints(c).clone_with_new_constraint(term.clone()); - self.constrain(&e)?; + let e = cycle_constraints(c); + self.constrain(&e.clone_with_new_constraint(term.clone()))?; } } Ok(QueryEvent::None) @@ -2277,16 +2157,18 @@ impl PolarVirtualMachine { (Value::Variable(l), _) => { // A variable on the left, ground on the right. match self.variable_state(l) { + VariableState::Bound(x) => { + args[0] = x; + self.push_goal(Goal::Query { + term: Operation { operator: op, args }.into_term(), + })?; + } VariableState::Unbound => { self.constrain(&op!(And, term.clone()))?; } - VariableState::Bound(x) => { - return self.comparison_op_helper(term, op, vec![x, right]); - } VariableState::Cycle(c) => { let e = cycle_constraints(c); - let e = e.clone_with_new_constraint(term.clone()); - self.constrain(&e)?; + self.constrain(&e.clone_with_new_constraint(term.clone()))?; } VariableState::Partial(e) => { self.constrain(&e.clone_with_new_constraint(term.clone()))?; @@ -2297,16 +2179,18 @@ impl PolarVirtualMachine { (_, Value::Variable(r)) => { // Ground on the left, a variable on the right. match self.variable_state(r) { + VariableState::Bound(y) => { + args[1] = y; + self.push_goal(Goal::Query { + term: Operation { operator: op, args }.into_term(), + })?; + } VariableState::Unbound => { self.constrain(&op!(And, term.clone()))?; } - VariableState::Bound(y) => { - return self.comparison_op_helper(term, op, vec![left, y]); - } VariableState::Cycle(d) => { - let e = cycle_constraints(d); - let e = e.clone_with_new_constraint(term.clone()); - self.constrain(&e)?; + let f = cycle_constraints(d); + self.constrain(&f.clone_with_new_constraint(term.clone()))?; } VariableState::Partial(f) => { self.constrain(&f.clone_with_new_constraint(term.clone()))?; @@ -2314,18 +2198,72 @@ impl PolarVirtualMachine { } Ok(QueryEvent::None) } - (left, right) => Err(self.type_error( - term, - format!( - "{} expects comparable arguments, got: {}, {}", - op.to_polar(), - left.to_polar(), - right.to_polar() - ), - )), + (_, _) => match eval(operation) { + Ok(goal) => { + self.push_goal(goal)?; + Ok(QueryEvent::None) + } + Err(error) => Err(self.set_error_context(term, error)), + }, } } + /// Evaluate comparison operations. + fn comparison_op_helper(&mut self, term: &Term) -> PolarResult { + self.query_op_helper(term, |Operation { operator: op, args }| { + assert_eq!(args.len(), 2); + let left = &args[0]; + let right = &args[1]; + + if !compare(*op, left, right) { + Ok(Goal::Backtrack) + } else { + Ok(Goal::Noop) + } + }) + } + + /// Evaluate arithmetic operations. + fn arithmetic_op_helper(&mut self, term: &Term) -> PolarResult { + self.query_op_helper(term, |Operation { operator: op, args }| { + assert_eq!(args.len(), 3); + let left = &args[0]; + let right = &args[1]; + let result = &args[2]; + assert!(matches!(result.value(), Value::Variable(_))); + + match (left.value(), right.value()) { + (Value::Number(left), Value::Number(right)) => { + if let Some(answer) = match op { + Operator::Add => *left + *right, + Operator::Sub => *left - *right, + Operator::Mul => *left * *right, + Operator::Div => *left / *right, + Operator::Mod => (*left).modulo(*right), + Operator::Rem => *left % *right, + _ => { + return Err(error::RuntimeError::Unsupported { + msg: format!("numeric operation {}", op.to_polar()), + }) + } + } { + Ok(Goal::Unify { + left: term.clone_with_value(Value::Number(answer)), + right: result.clone(), + }) + } else { + Err(error::RuntimeError::ArithmeticError { + msg: term.to_polar(), + }) + } + } + (_, _) => Err(error::RuntimeError::Unsupported { + msg: format!("unsupported arithmetic operands: {}", term.to_polar()), + }), + } + }) + } + /// Unify `left` and `right` terms. /// /// Outcomes of a unification are: diff --git a/polar-core/tests/integration_tests.rs b/polar-core/tests/integration_tests.rs index ea9ce2ee23..4670284899 100644 --- a/polar-core/tests/integration_tests.rs +++ b/polar-core/tests/integration_tests.rs @@ -1111,8 +1111,6 @@ fn test_modulo_and_remainder() { qeval(&mut p, "0 rem 1 == 0"); qeval(&mut p, "0 mod -1 == 0"); qeval(&mut p, "0 rem -1 == 0"); - qruntime!("1 mod 0 = x", RuntimeError::ArithmeticError { .. }); - qruntime!("1 rem 0 = x", RuntimeError::ArithmeticError { .. }); let res = var(&mut p, "1 mod 0.0 = x", "x")[0].clone(); if let Value::Number(Numeric::Float(x)) = res { assert!(x.is_nan()); From f4cb6f540b8914bf0122daf42ebdcbd81c849307 Mon Sep 17 00:00:00 2001 From: Alex Plotnick Date: Thu, 21 Jan 2021 10:28:38 -0700 Subject: [PATCH 02/18] Refactor in_op_helper to use new query_op_helper --- polar-core/src/vm.rs | 421 +++++++++++++++++++------------------------ 1 file changed, 182 insertions(+), 239 deletions(-) diff --git a/polar-core/src/vm.rs b/polar-core/src/vm.rs index 3a7abe87ef..ea9ef18275 100644 --- a/polar-core/src/vm.rs +++ b/polar-core/src/vm.rs @@ -1655,24 +1655,28 @@ impl PolarVirtualMachine { self.push_goal(Goal::Unify { left, right })? } Operator::Dot => self.dot_op_helper(args)?, + Operator::Lt | Operator::Gt | Operator::Leq | Operator::Geq | Operator::Eq | Operator::Neq => { - return self.comparison_op_helper(term); + return self.query_op_helper(term, Self::comparison_op_helper, true); } + Operator::Add | Operator::Sub | Operator::Mul | Operator::Div | Operator::Mod | Operator::Rem => { - return self.arithmetic_op_helper(term); + return self.query_op_helper(term, Self::arithmetic_op_helper, true); } - Operator::In => return self.in_op_helper(term), + Operator::In => { + return self.query_op_helper(term, Self::in_op_helper, false); + } Operator::Debug => { let mut message = "".to_string(); @@ -1887,193 +1891,17 @@ impl PolarVirtualMachine { Ok(()) } + /// Handle variables & constraints as arguments to various operations. + /// Calls the `eval` method to handle ground terms. #[allow(clippy::many_single_char_names)] - fn in_op_helper(&mut self, term: &Term) -> PolarResult { - let operation = term.value().as_expression().unwrap(); - let op = operation.operator; - - let args = operation.args.clone(); - assert_eq!(args.len(), 2); - - let item = &args[0]; - let iterable = &args[1]; - match (item.value(), iterable.value()) { - (Value::Expression(_), _) - | (_, Value::Expression(_)) - | (Value::RestVariable(_), _) - | (_, Value::RestVariable(_)) => unreachable!("invalid syntax"), - - (_, Value::List(list)) if list.is_empty() => { - // Nothing is in an empty list. - self.backtrack()?; - } - (_, Value::String(s)) if s.is_empty() => { - // Nothing is in an empty string. - self.backtrack()?; - } - (_, Value::Dictionary(d)) if d.is_empty() => { - // Nothing is in an empty dict. - self.backtrack()?; - } - - (Value::Variable(l), Value::Variable(r)) => { - // Two variables. - match (self.variable_state(l), self.variable_state(r)) { - (VariableState::Bound(item), _) => { - let args = vec![item, iterable.clone()]; - return self.in_op_helper(&term.clone_with_value(Value::Expression( - Operation { operator: op, args }, - ))); - } - (_, VariableState::Bound(iterable)) => { - let args = vec![item.clone(), iterable]; - return self.in_op_helper(&term.clone_with_value(Value::Expression( - Operation { operator: op, args }, - ))); - } - (VariableState::Unbound, VariableState::Unbound) => { - let constraint = op!(And, term.clone()); - self.constrain(&constraint)?; - } - (VariableState::Cycle(c), VariableState::Unbound) - | (VariableState::Unbound, VariableState::Cycle(c)) => { - let e = cycle_constraints(c); - self.constrain(&e.clone_with_new_constraint(term.clone()))?; - } - (VariableState::Cycle(c), VariableState::Cycle(d)) => { - let mut e = cycle_constraints(c); - e.merge_constraints(cycle_constraints(d)); - self.constrain(&e.clone_with_new_constraint(term.clone()))?; - } - (VariableState::Unbound, VariableState::Partial(e)) - | (VariableState::Partial(e), VariableState::Unbound) => { - self.constrain(&e.clone_with_new_constraint(term.clone()))?; - } - (VariableState::Cycle(c), VariableState::Partial(mut e)) - | (VariableState::Partial(mut e), VariableState::Cycle(c)) => { - e.merge_constraints(cycle_constraints(c)); - self.constrain(&e.clone_with_new_constraint(term.clone()))?; - } - (VariableState::Partial(mut e), VariableState::Partial(f)) => { - e.merge_constraints(f); - self.constrain(&e.clone_with_new_constraint(term.clone()))?; - } - } - } - - (_, Value::Variable(v)) => match self.variable_state(v) { - VariableState::Unbound => { - self.constrain(&op!(And, term.clone()))?; - } - VariableState::Bound(iterable) => { - let args = vec![item.clone(), iterable]; - return self.in_op_helper( - &term.clone_with_value(Value::Expression(Operation { operator: op, args })), - ); - } - VariableState::Cycle(c) => { - let e = cycle_constraints(c); - self.constrain(&e.clone_with_new_constraint(term.clone()))?; - } - VariableState::Partial(e) => { - self.constrain(&e.clone_with_new_constraint(term.clone()))?; - } - }, - - (_, Value::List(terms)) => { - // Unify item with each element of the list, skipping non-matching ground terms. - let item_is_ground = item.is_ground(); - self.choose( - terms - .iter() - .filter(|term| { - !item_is_ground || !term.is_ground() || term.value() == item.value() - }) - .map(|term| { - vec![Goal::Unify { - left: item.clone(), - right: term.clone(), - }] - }) - .collect::>(), - )?; - } - (_, Value::Dictionary(dict)) => { - // Unify item with each (k, v) pair of the dict, skipping non-matching ground terms. - let item_is_ground = item.is_ground(); - self.choose( - dict.fields - .iter() - .map(|(k, v)| { - iterable.clone_with_value(Value::List(vec![ - v.clone_with_value(Value::String(k.0.clone())), - v.clone(), - ])) - }) - .filter(|term| { - !item_is_ground || !term.is_ground() || term.value() == item.value() - }) - .map(|term| { - vec![Goal::Unify { - left: item.clone(), - right: term, - }] - }) - .collect::>(), - )?; - } - (_, Value::String(s)) => { - // Unify item with each element of the string - let item_is_ground = item.is_ground(); - self.choose( - s.chars() - .map(|c| c.to_string()) - .map(Value::String) - .filter(|c| !item_is_ground || c == item.value()) - .map(|c| { - vec![Goal::Unify { - left: item.clone(), - right: iterable.clone_with_value(c), - }] - }) - .collect::>(), - )?; - } - // Push an `ExternalLookup` goal for external instances - (_, Value::ExternalInstance(_)) => { - // Generate symbol for next result and bind to `false` (default) - let (call_id, next_term) = self.new_call_var("next_value", Value::Boolean(false)); - - // append unify goal to be evaluated after - // next result is fetched - self.append_goals(vec![ - Goal::NextExternal { - call_id, - iterable: self.deep_deref(&iterable), - }, - Goal::Unify { - left: item.clone(), - right: next_term, - }, - ])?; - } - _ => { - return Err(self.type_error( - &iterable, - format!( - "can only use `in` on an iterable value, this is {:?}", - iterable.value() - ), - )); - } - } - Ok(QueryEvent::None) - } - - #[allow(clippy::many_single_char_names)] - fn query_op_helper(&mut self, term: &Term, eval: F) -> PolarResult + fn query_op_helper( + &mut self, + term: &Term, + eval: F, + handle_left_var: bool, + ) -> PolarResult where - F: Fn(&Operation) -> Result, + F: Fn(&mut Self, &Term) -> PolarResult, { let operation = term.value().as_expression().unwrap(); let op = operation.operator; @@ -2090,13 +1918,14 @@ impl PolarVirtualMachine { panic!("invalid query"); } + // TODO(ap): ExternalInstance on one side. (Value::ExternalInstance(_), Value::ExternalInstance(_)) => { // Generate symbol for external result and bind to `false` (default). let (call_id, answer) = self.new_call_var("external_op_result", Value::Boolean(false)); // Unify the answer with the external result. - // TODO(ap): true??? + // TODO(ap): Why true? self.push_goal(Goal::Unify { left: answer, right: Term::new_temporary(Value::Boolean(true)), @@ -2154,7 +1983,7 @@ impl PolarVirtualMachine { } Ok(QueryEvent::None) } - (Value::Variable(l), _) => { + (Value::Variable(l), _) if handle_left_var => { // A variable on the left, ground on the right. match self.variable_state(l) { VariableState::Bound(x) => { @@ -2198,70 +2027,184 @@ impl PolarVirtualMachine { } Ok(QueryEvent::None) } - (_, _) => match eval(operation) { - Ok(goal) => { - self.push_goal(goal)?; - Ok(QueryEvent::None) - } - Err(error) => Err(self.set_error_context(term, error)), - }, + (_, _) => eval(self, term), } } /// Evaluate comparison operations. fn comparison_op_helper(&mut self, term: &Term) -> PolarResult { - self.query_op_helper(term, |Operation { operator: op, args }| { - assert_eq!(args.len(), 2); - let left = &args[0]; - let right = &args[1]; + let Operation { operator: op, args } = term.value().as_expression().unwrap(); - if !compare(*op, left, right) { - Ok(Goal::Backtrack) - } else { - Ok(Goal::Noop) - } - }) + assert_eq!(args.len(), 2); + let left = &args[0]; + let right = &args[1]; + + if !compare(*op, left, right) { + self.push_goal(Goal::Backtrack)?; + } + Ok(QueryEvent::None) } /// Evaluate arithmetic operations. fn arithmetic_op_helper(&mut self, term: &Term) -> PolarResult { - self.query_op_helper(term, |Operation { operator: op, args }| { - assert_eq!(args.len(), 3); - let left = &args[0]; - let right = &args[1]; - let result = &args[2]; - assert!(matches!(result.value(), Value::Variable(_))); - - match (left.value(), right.value()) { - (Value::Number(left), Value::Number(right)) => { - if let Some(answer) = match op { - Operator::Add => *left + *right, - Operator::Sub => *left - *right, - Operator::Mul => *left * *right, - Operator::Div => *left / *right, - Operator::Mod => (*left).modulo(*right), - Operator::Rem => *left % *right, - _ => { - return Err(error::RuntimeError::Unsupported { + let Operation { operator: op, args } = term.value().as_expression().unwrap(); + + assert_eq!(args.len(), 3); + let left = &args[0]; + let right = &args[1]; + let result = &args[2]; + assert!(matches!(result.value(), Value::Variable(_))); + + match (left.value(), right.value()) { + (Value::Number(left), Value::Number(right)) => { + if let Some(answer) = match op { + Operator::Add => *left + *right, + Operator::Sub => *left - *right, + Operator::Mul => *left * *right, + Operator::Div => *left / *right, + Operator::Mod => (*left).modulo(*right), + Operator::Rem => *left % *right, + _ => { + return Err(self.set_error_context( + &term, + error::RuntimeError::Unsupported { msg: format!("numeric operation {}", op.to_polar()), - }) - } - } { - Ok(Goal::Unify { - left: term.clone_with_value(Value::Number(answer)), - right: result.clone(), - }) - } else { - Err(error::RuntimeError::ArithmeticError { - msg: term.to_polar(), - }) + }, + )); } + } { + self.push_goal(Goal::Unify { + left: term.clone_with_value(Value::Number(answer)), + right: result.clone(), + })?; + Ok(QueryEvent::None) + } else { + Err(self.set_error_context( + &term, + error::RuntimeError::ArithmeticError { + msg: term.to_polar(), + }, + )) } - (_, _) => Err(error::RuntimeError::Unsupported { + } + (_, _) => Err(self.set_error_context( + &term, + error::RuntimeError::Unsupported { msg: format!("unsupported arithmetic operands: {}", term.to_polar()), - }), + }, + )), + } + } + + fn in_op_helper(&mut self, term: &Term) -> PolarResult { + let Operation { args, .. } = term.value().as_expression().unwrap(); + + assert_eq!(args.len(), 2); + let item = &args[0]; + let iterable = &args[1]; + + match (item.value(), iterable.value()) { + (_, Value::List(list)) if list.is_empty() => { + // Nothing is in an empty list. + self.backtrack()?; } - }) + (_, Value::String(s)) if s.is_empty() => { + // Nothing is in an empty string. + self.backtrack()?; + } + (_, Value::Dictionary(d)) if d.is_empty() => { + // Nothing is in an empty dict. + self.backtrack()?; + } + + (_, Value::List(terms)) => { + // Unify item with each element of the list, skipping non-matching ground terms. + let item_is_ground = item.is_ground(); + self.choose( + terms + .iter() + .filter(|term| { + !item_is_ground || !term.is_ground() || term.value() == item.value() + }) + .map(|term| { + vec![Goal::Unify { + left: item.clone(), + right: term.clone(), + }] + }) + .collect::>(), + )?; + } + (_, Value::Dictionary(dict)) => { + // Unify item with each (k, v) pair of the dict, skipping non-matching ground terms. + let item_is_ground = item.is_ground(); + self.choose( + dict.fields + .iter() + .map(|(k, v)| { + iterable.clone_with_value(Value::List(vec![ + v.clone_with_value(Value::String(k.0.clone())), + v.clone(), + ])) + }) + .filter(|term| { + !item_is_ground || !term.is_ground() || term.value() == item.value() + }) + .map(|term| { + vec![Goal::Unify { + left: item.clone(), + right: term, + }] + }) + .collect::>(), + )?; + } + (_, Value::String(s)) => { + // Unify item with each element of the string + let item_is_ground = item.is_ground(); + self.choose( + s.chars() + .map(|c| c.to_string()) + .map(Value::String) + .filter(|c| !item_is_ground || c == item.value()) + .map(|c| { + vec![Goal::Unify { + left: item.clone(), + right: iterable.clone_with_value(c), + }] + }) + .collect::>(), + )?; + } + // Push an `ExternalLookup` goal for external instances + (_, Value::ExternalInstance(_)) => { + // Generate symbol for next result and bind to `false` (default) + let (call_id, next_term) = self.new_call_var("next_value", Value::Boolean(false)); + + // append unify goal to be evaluated after + // next result is fetched + self.append_goals(vec![ + Goal::NextExternal { + call_id, + iterable: self.deep_deref(&iterable), + }, + Goal::Unify { + left: item.clone(), + right: next_term, + }, + ])?; + } + _ => { + return Err(self.type_error( + &iterable, + format!( + "can only use `in` on an iterable value, this is {:?}", + iterable.value() + ), + )); + } + } + Ok(QueryEvent::None) } /// Unify `left` and `right` terms. From 73b92cf8d4f99356254777250f4fdf36bb9a2585 Mon Sep 17 00:00:00 2001 From: David Hatch Date: Thu, 21 Jan 2021 14:50:21 -0500 Subject: [PATCH 03/18] Add add_constraint method. --- polar-core/src/vm.rs | 216 +++++++++++++++++++++++++------------------ 1 file changed, 126 insertions(+), 90 deletions(-) diff --git a/polar-core/src/vm.rs b/polar-core/src/vm.rs index ea9ef18275..a9c5502779 100644 --- a/polar-core/src/vm.rs +++ b/polar-core/src/vm.rs @@ -642,6 +642,86 @@ impl PolarVirtualMachine { Ok(()) } + /// Add a single constraint to the variables referenced in it. + fn add_constraint(&mut self, term: &Term) -> PolarResult<()> { + // Preconditions: At least one argument of operation is a variable & is unbound. + // Basically want to assert here that this is a binary operation. + let Operation { operator: op, args } = term.value().as_expression().unwrap(); + + assert_ne!( + op, + &Operator::And, + "Should be called with single constraint." + ); + assert_ne!(op, &Operator::Or, "Not or"); + assert_eq!(args.len(), 2); + + // Constrain only called in this function. + let (left, right) = (&args[0], &args[1]); + match (left.value(), right.value()) { + (Value::Variable(lv), Value::Variable(rv)) => { + match (self.variable_state(lv), self.variable_state(rv)) { + (VariableState::Unbound, VariableState::Unbound) => { + self.constrain(&op!(And, term.clone()))?; + } + (VariableState::Cycle(c), VariableState::Cycle(d)) => { + let mut e = cycle_constraints(c); + e.merge_constraints(cycle_constraints(d)); + self.constrain(&e.clone_with_new_constraint(term.clone()))?; + } + (VariableState::Partial(e), VariableState::Unbound) + | (VariableState::Unbound, VariableState::Partial(e)) => { + self.constrain(&e.clone_with_new_constraint(term.clone()))?; + } + (VariableState::Partial(mut e), VariableState::Partial(f)) => { + e.merge_constraints(f); + self.constrain(&e.clone_with_new_constraint(term.clone()))?; + } + (VariableState::Partial(mut e), VariableState::Cycle(c)) + | (VariableState::Cycle(c), VariableState::Partial(mut e)) => { + e.merge_constraints(cycle_constraints(c)); + self.constrain(&e.clone_with_new_constraint(term.clone()))?; + } + (VariableState::Cycle(c), VariableState::Unbound) + | (VariableState::Unbound, VariableState::Cycle(c)) => { + let e = cycle_constraints(c); + self.constrain(&e.clone_with_new_constraint(term.clone()))?; + } + (VariableState::Bound(val), _) => { + panic!( + "Variable passed to constrain must be unbound left bound to: {}.", + val.to_polar() + ); + } + (_, VariableState::Bound(val)) => { + panic!( + "Variable passed to constrain must be unbound right bound to: {}.", + val.to_polar() + ); + } + } + } + (Value::Variable(v), _) | (_, Value::Variable(v)) => match self.variable_state(v) { + VariableState::Unbound => { + self.constrain(&op!(And, term.clone()))?; + } + VariableState::Cycle(c) => { + let e = cycle_constraints(c); + self.constrain(&e.clone_with_new_constraint(term.clone()))?; + } + VariableState::Partial(e) => { + self.constrain(&e.clone_with_new_constraint(term.clone()))?; + } + VariableState::Bound(_) => { + panic!("Variable passed to constrain must be unbound."); + } + }, + (_, _) => panic!("Must call with at least one side as a variable"), // Cases for two variables based on variables states. + } + + Ok(()) + } + /// Augment the bindings stack with constants from a hash map. /// There must be no temporaries bound yet. pub fn bind_constants(&mut self, bindings: Bindings) { @@ -1217,9 +1297,7 @@ impl PolarVirtualMachine { match self.variable_state(v) { VariableState::Cycle(c) => cycle_constraints(c) .clone_with_new_constraint(unify.into_term()), - VariableState::Bound(b) => { - op!(Unify, left, b) - } + VariableState::Bound(b) => op!(Unify, left, b), VariableState::Partial(e) => { e.clone_with_new_constraint(unify.into_term()) } @@ -1893,6 +1971,13 @@ impl PolarVirtualMachine { /// Handle variables & constraints as arguments to various operations. /// Calls the `eval` method to handle ground terms. + /// + /// Arguments: + /// + /// - handle_unbound_left_var: If set to `false`, allow `eval` to handle + /// operations with an unbound left variable, instead of adding a constraint. + /// Some operations, like in emit new goals or choice points when the left + /// operand is a variable. #[allow(clippy::many_single_char_names)] fn query_op_helper( &mut self, @@ -1932,103 +2017,52 @@ impl PolarVirtualMachine { })?; // Emit an event for the external operation. - Ok(QueryEvent::ExternalOp { + return Ok(QueryEvent::ExternalOp { call_id, operator: op, args: vec![left.clone(), right.clone()], - }) + }); } + _ => {} + }; - (Value::Variable(l), Value::Variable(r)) => { - // Two variables. - match (self.variable_state(l), self.variable_state(r)) { - (VariableState::Bound(x), _) => { - args[0] = x; - self.push_goal(Goal::Query { - term: Operation { operator: op, args }.into_term(), - })?; - } - (_, VariableState::Bound(y)) => { - args[1] = y; - self.push_goal(Goal::Query { - term: Operation { operator: op, args }.into_term(), - })?; - } - (VariableState::Unbound, VariableState::Unbound) => { - self.constrain(&op!(And, term.clone()))?; - } - (VariableState::Cycle(c), VariableState::Cycle(d)) => { - let mut e = cycle_constraints(c); - e.merge_constraints(cycle_constraints(d)); - self.constrain(&e.clone_with_new_constraint(term.clone()))?; - } - (VariableState::Partial(e), VariableState::Unbound) - | (VariableState::Unbound, VariableState::Partial(e)) => { - self.constrain(&e.clone_with_new_constraint(term.clone()))?; - } - (VariableState::Partial(mut e), VariableState::Partial(f)) => { - e.merge_constraints(f); - self.constrain(&e.clone_with_new_constraint(term.clone()))?; - } - (VariableState::Partial(mut e), VariableState::Cycle(c)) - | (VariableState::Cycle(c), VariableState::Partial(mut e)) => { - e.merge_constraints(cycle_constraints(c)); - self.constrain(&e.clone_with_new_constraint(term.clone()))?; - } - (VariableState::Cycle(c), VariableState::Unbound) - | (VariableState::Unbound, VariableState::Cycle(c)) => { - let e = cycle_constraints(c); - self.constrain(&e.clone_with_new_constraint(term.clone()))?; - } - } - Ok(QueryEvent::None) - } - (Value::Variable(l), _) if handle_left_var => { - // A variable on the left, ground on the right. - match self.variable_state(l) { - VariableState::Bound(x) => { - args[0] = x; - self.push_goal(Goal::Query { - term: Operation { operator: op, args }.into_term(), - })?; - } - VariableState::Unbound => { - self.constrain(&op!(And, term.clone()))?; - } - VariableState::Cycle(c) => { - let e = cycle_constraints(c); - self.constrain(&e.clone_with_new_constraint(term.clone()))?; - } - VariableState::Partial(e) => { - self.constrain(&e.clone_with_new_constraint(term.clone()))?; - } + if let Value::Variable(r) = right.value() { + // A variable on the left, ground on the right. + match self.variable_state(r) { + VariableState::Bound(x) => { + args[1] = x; + self.push_goal(Goal::Query { + term: Operation { operator: op, args }.into_term(), + })?; + return Ok(QueryEvent::None); } - Ok(QueryEvent::None) + _ => {} } - (_, Value::Variable(r)) => { - // Ground on the left, a variable on the right. - match self.variable_state(r) { - VariableState::Bound(y) => { - args[1] = y; - self.push_goal(Goal::Query { - term: Operation { operator: op, args }.into_term(), - })?; - } - VariableState::Unbound => { - self.constrain(&op!(And, term.clone()))?; - } - VariableState::Cycle(d) => { - let f = cycle_constraints(d); - self.constrain(&f.clone_with_new_constraint(term.clone()))?; - } - VariableState::Partial(f) => { - self.constrain(&f.clone_with_new_constraint(term.clone()))?; - } + } + + if let Value::Variable(l) = left.value() { + // A variable on the left, ground on the right. + match self.variable_state(l) { + VariableState::Bound(x) => { + args[0] = x; + self.push_goal(Goal::Query { + term: Operation { operator: op, args }.into_term(), + })?; + return Ok(QueryEvent::None); } - Ok(QueryEvent::None) + _ => if !handle_left_var && !right.value().as_symbol().is_ok() { + return eval(self, term); + }, } - (_, _) => eval(self, term), } + + if left.value().as_symbol().is_ok() || right.value().as_symbol().is_ok() + { + self.add_constraint(term)?; + return Ok(QueryEvent::None); + } + + eval(self, term) } /// Evaluate comparison operations. @@ -2045,6 +2079,8 @@ impl PolarVirtualMachine { Ok(QueryEvent::None) } + // TODO(ap, dhatch): This does not strip the 3rd argument like Dot op (the unbound variable). + // So may be incorrect currently. /// Evaluate arithmetic operations. fn arithmetic_op_helper(&mut self, term: &Term) -> PolarResult { let Operation { operator: op, args } = term.value().as_expression().unwrap(); From a4344902ea056df2f6e50a80ae2fd25c1ba48955 Mon Sep 17 00:00:00 2001 From: David Hatch Date: Thu, 21 Jan 2021 15:01:19 -0500 Subject: [PATCH 04/18] Use add constraint in unify vars. --- polar-core/src/vm.rs | 49 ++++++++------------------------------------ 1 file changed, 9 insertions(+), 40 deletions(-) diff --git a/polar-core/src/vm.rs b/polar-core/src/vm.rs index a9c5502779..023f56a086 100644 --- a/polar-core/src/vm.rs +++ b/polar-core/src/vm.rs @@ -2487,48 +2487,17 @@ impl PolarVirtualMachine { } // Expressions. - (VariableState::Partial(e), VariableState::Bound(y)) => { - // Add a unification constraint. - let e = e.clone_with_new_constraint(op!(Unify, left.clone(), y).into_term()); - self.constrain(&e)?; + (VariableState::Partial(_), VariableState::Bound(right_value)) => { + // Add a unification constraint with the value of the bound right side. + self.add_constraint(&op!(Unify, left.clone(), right_value).into_term())?; } - (VariableState::Bound(x), VariableState::Partial(f)) => { - // Add a unification constraint. - let f = f.clone_with_new_constraint(op!(Unify, x, right.clone()).into_term()); - self.constrain(&f)?; + (VariableState::Bound(left_value), VariableState::Partial(_)) => { + // Add a unification constraint with the value of the bound left side. + self.add_constraint(&op!(Unify, left_value, right.clone()).into_term())?; } - (VariableState::Partial(mut e), VariableState::Partial(f)) => { - // Merge existing constraints and add a unification constraint. - e.merge_constraints(f); - let e = e - .clone_with_new_constraint(op!(Unify, left.clone(), right.clone()).into_term()); - self.constrain(&e)?; - } - (VariableState::Partial(e), VariableState::Unbound) => { - // Add a unification constraint. - let e = e - .clone_with_new_constraint(op!(Unify, left.clone(), right.clone()).into_term()); - self.constrain(&e)?; - } - (VariableState::Unbound, VariableState::Partial(f)) => { - // Add a unification constraint. - let f = f - .clone_with_new_constraint(op!(Unify, left.clone(), right.clone()).into_term()); - self.constrain(&f)?; - } - (VariableState::Partial(mut e), VariableState::Cycle(c)) => { - // Bind the entire cycle to the expression. - e.merge_constraints(cycle_constraints(c)); - let e = e - .clone_with_new_constraint(op!(Unify, left.clone(), right.clone()).into_term()); - self.constrain(&e)?; - } - (VariableState::Cycle(c), VariableState::Partial(mut f)) => { - // Bind the entire cycle to the expression. - f.merge_constraints(cycle_constraints(c)); - let f = f - .clone_with_new_constraint(op!(Unify, left.clone(), right.clone()).into_term()); - self.constrain(&f)?; + (VariableState::Partial(_), _) | (_, VariableState::Partial(_)) => { + // Add a unification constraint with both partials. + self.add_constraint(&op!(Unify, left.clone(), right.clone()).into_term())?; } } Ok(()) From 45d91d53a58bd990f70655ea4122c9e629d0f5d1 Mon Sep 17 00:00:00 2001 From: Alex Plotnick Date: Thu, 21 Jan 2021 13:57:18 -0700 Subject: [PATCH 05/18] Format, fix some comments, and the tests. --- languages/python/oso/tests/test_polar.py | 3 +-- languages/ruby/spec/oso/polar/polar_spec.rb | 3 +-- polar-core/src/vm.rs | 27 ++++++++++----------- 3 files changed, 15 insertions(+), 18 deletions(-) diff --git a/languages/python/oso/tests/test_polar.py b/languages/python/oso/tests/test_polar.py index 42e45c2800..abfeea2abc 100644 --- a/languages/python/oso/tests/test_polar.py +++ b/languages/python/oso/tests/test_polar.py @@ -426,8 +426,7 @@ def test_runtime_errors(polar, query): == """trace (most recent evaluation last): in query at line 1, column 1 foo(1,2) - in rule foo at line 2, column 17 - a in b + 1 in 2 Type error: can only use `in` on an iterable value, this is Number(Integer(2)) at line 1, column 7""" ) diff --git a/languages/ruby/spec/oso/polar/polar_spec.rb b/languages/ruby/spec/oso/polar/polar_spec.rb index d4fcda6cfe..e18e6a208c 100644 --- a/languages/ruby/spec/oso/polar/polar_spec.rb +++ b/languages/ruby/spec/oso/polar/polar_spec.rb @@ -693,8 +693,7 @@ def groups trace (most recent evaluation last): in query at line 1, column 1 foo(1,2) - in rule foo at line 1, column 13 - a in b + 1 in 2 Type error: can only use `in` on an iterable value, this is Number(Integer(2)) at line 1, column 7 TRACE expect(e.message).to eq(error) diff --git a/polar-core/src/vm.rs b/polar-core/src/vm.rs index 023f56a086..93a58ed85d 100644 --- a/polar-core/src/vm.rs +++ b/polar-core/src/vm.rs @@ -654,9 +654,8 @@ impl PolarVirtualMachine { "Should be called with single constraint." ); assert_ne!(op, &Operator::Or, "Not or"); - assert_eq!(args.len(), 2); + assert!(args.len() >= 2); - // Constrain only called in this function. let (left, right) = (&args[0], &args[1]); match (left.value(), right.value()) { (Value::Variable(lv), Value::Variable(rv)) => { @@ -689,13 +688,13 @@ impl PolarVirtualMachine { } (VariableState::Bound(val), _) => { panic!( - "Variable passed to constrain must be unbound left bound to: {}.", + "Variable passed to constrain must be unbound, left bound to: {}.", val.to_polar() ); } (_, VariableState::Bound(val)) => { panic!( - "Variable passed to constrain must be unbound right bound to: {}.", + "Variable passed to constrain must be unbound, right bound to: {}.", val.to_polar() ); } @@ -716,7 +715,7 @@ impl PolarVirtualMachine { panic!("Variable passed to constrain must be unbound."); } }, - (_, _) => panic!("Must call with at least one side as a variable"), // Cases for two variables based on variables states. + (_, _) => panic!("Must call with at least one side as a variable."), } Ok(()) @@ -2005,12 +2004,11 @@ impl PolarVirtualMachine { // TODO(ap): ExternalInstance on one side. (Value::ExternalInstance(_), Value::ExternalInstance(_)) => { - // Generate symbol for external result and bind to `false` (default). + // Generate a symbol for the external result and bind to `false` (default). let (call_id, answer) = self.new_call_var("external_op_result", Value::Boolean(false)); - // Unify the answer with the external result. - // TODO(ap): Why true? + // Check that the external result is `true` when we return. self.push_goal(Goal::Unify { left: answer, right: Term::new_temporary(Value::Boolean(true)), @@ -2027,7 +2025,7 @@ impl PolarVirtualMachine { }; if let Value::Variable(r) = right.value() { - // A variable on the left, ground on the right. + // A variable on the right, ground on the left. match self.variable_state(r) { VariableState::Bound(x) => { args[1] = x; @@ -2050,14 +2048,15 @@ impl PolarVirtualMachine { })?; return Ok(QueryEvent::None); } - _ => if !handle_left_var && !right.value().as_symbol().is_ok() { - return eval(self, term); - }, + _ => { + if !handle_left_var && right.value().as_symbol().is_err() { + return eval(self, term); + } + } } } - if left.value().as_symbol().is_ok() || right.value().as_symbol().is_ok() - { + if left.value().as_symbol().is_ok() || right.value().as_symbol().is_ok() { self.add_constraint(term)?; return Ok(QueryEvent::None); } From 08ccd4507f4301c7c8fd7624a1491195f60279c6 Mon Sep 17 00:00:00 2001 From: Alex Plotnick Date: Thu, 21 Jan 2021 14:32:24 -0700 Subject: [PATCH 06/18] Refactor dot_op_helper to use query_op_helper. --- polar-core/src/vm.rs | 139 +++++++++++++++++++++++-------------------- 1 file changed, 75 insertions(+), 64 deletions(-) diff --git a/polar-core/src/vm.rs b/polar-core/src/vm.rs index 93a58ed85d..63affe2a67 100644 --- a/polar-core/src/vm.rs +++ b/polar-core/src/vm.rs @@ -1731,7 +1731,9 @@ impl PolarVirtualMachine { let left = args.pop().unwrap(); self.push_goal(Goal::Unify { left, right })? } - Operator::Dot => self.dot_op_helper(args)?, + Operator::Dot => { + return self.query_op_helper(term, Self::dot_op_helper, false, false); + } Operator::Lt | Operator::Gt @@ -1739,7 +1741,7 @@ impl PolarVirtualMachine { | Operator::Geq | Operator::Eq | Operator::Neq => { - return self.query_op_helper(term, Self::comparison_op_helper, true); + return self.query_op_helper(term, Self::comparison_op_helper, true, true); } Operator::Add @@ -1748,11 +1750,11 @@ impl PolarVirtualMachine { | Operator::Div | Operator::Mod | Operator::Rem => { - return self.query_op_helper(term, Self::arithmetic_op_helper, true); + return self.query_op_helper(term, Self::arithmetic_op_helper, true, true); } Operator::In => { - return self.query_op_helper(term, Self::in_op_helper, false); + return self.query_op_helper(term, Self::in_op_helper, false, true); } Operator::Debug => { @@ -1892,7 +1894,11 @@ impl PolarVirtualMachine { } /// Push appropriate goals for lookups on dictionaries and instances. - fn dot_op_helper(&mut self, mut args: Vec) -> PolarResult<()> { + fn dot_op_helper(&mut self, term: &Term) -> PolarResult { + let Operation { operator: op, args } = term.value().as_expression().unwrap(); + assert_eq!(*op, Operator::Dot, "expected a dot operation"); + + let mut args = args.clone(); assert_eq!(args.len(), 3); let object = &args[0]; let field = &args[1]; @@ -1933,9 +1939,7 @@ impl PolarVirtualMachine { } Value::Variable(v) => { let constraints = match self.variable_state(v) { - VariableState::Bound(x) => { - return self.dot_op_helper(vec![x, field.clone(), value.clone()]); - } + VariableState::Bound(_) => panic!("should have already dereferenced {}", v), _ if matches!(field.value(), Value::Call(_)) => { return Err(self.set_error_context( object, @@ -1948,13 +1952,13 @@ impl PolarVirtualMachine { VariableState::Partial(expr) => expr, VariableState::Cycle(c) => cycle_constraints(c), }; - let dot_op = - object.clone_with_value(value!(op!(Dot, object.clone(), field.clone()))); - let constraints = constraints - .clone_with_new_constraint(op!(Unify, value.clone(), dot_op).into_term()); - self.constrain(&constraints)?; + // Translate `.(object, field, value)` → `value = .(object, field)`. + let dot2 = object.clone_with_value(value!(op!(Dot, object.clone(), field.clone()))); + self.constrain( + &constraints + .clone_with_new_constraint(op!(Unify, value.clone(), dot2).into_term()), + )?; } - Value::RestVariable(_) => unreachable!("invalid syntax"), _ => { return Err(self.type_error( &object, @@ -1965,7 +1969,7 @@ impl PolarVirtualMachine { )) } } - Ok(()) + Ok(QueryEvent::None) } /// Handle variables & constraints as arguments to various operations. @@ -1975,25 +1979,27 @@ impl PolarVirtualMachine { /// /// - handle_unbound_left_var: If set to `false`, allow `eval` to handle /// operations with an unbound left variable, instead of adding a constraint. - /// Some operations, like in emit new goals or choice points when the left + /// Some operations, like `In`, emit new goals or choice points when the left /// operand is a variable. + /// - handle_unbound_right_var: Same as above but for the RHS. `Dot` uses this. #[allow(clippy::many_single_char_names)] fn query_op_helper( &mut self, term: &Term, eval: F, - handle_left_var: bool, + handle_unbound_left_var: bool, + handle_unbound_right_var: bool, ) -> PolarResult where F: Fn(&mut Self, &Term) -> PolarResult, { - let operation = term.value().as_expression().unwrap(); - let op = operation.operator; + let Operation { operator: op, args } = term.value().as_expression().unwrap(); - let mut args = operation.args.clone(); + let mut args = args.clone(); assert!(args.len() >= 2); let left = &args[0]; let right = &args[1]; + match (left.value(), right.value()) { (Value::Expression(_), _) | (_, Value::Expression(_)) @@ -2001,58 +2007,40 @@ impl PolarVirtualMachine { | (_, Value::RestVariable(_)) => { panic!("invalid query"); } - - // TODO(ap): ExternalInstance on one side. - (Value::ExternalInstance(_), Value::ExternalInstance(_)) => { - // Generate a symbol for the external result and bind to `false` (default). - let (call_id, answer) = - self.new_call_var("external_op_result", Value::Boolean(false)); - - // Check that the external result is `true` when we return. - self.push_goal(Goal::Unify { - left: answer, - right: Term::new_temporary(Value::Boolean(true)), - })?; - - // Emit an event for the external operation. - return Ok(QueryEvent::ExternalOp { - call_id, - operator: op, - args: vec![left.clone(), right.clone()], - }); - } _ => {} }; if let Value::Variable(r) = right.value() { // A variable on the right, ground on the left. - match self.variable_state(r) { - VariableState::Bound(x) => { - args[1] = x; - self.push_goal(Goal::Query { - term: Operation { operator: op, args }.into_term(), - })?; - return Ok(QueryEvent::None); - } - _ => {} + if let VariableState::Bound(x) = self.variable_state(r) { + args[1] = x; + self.push_goal(Goal::Query { + term: Operation { + operator: *op, + args, + } + .into_term(), + })?; + return Ok(QueryEvent::None); + } else if !handle_unbound_right_var && left.value().as_symbol().is_err() { + return eval(self, term); } } if let Value::Variable(l) = left.value() { // A variable on the left, ground on the right. - match self.variable_state(l) { - VariableState::Bound(x) => { - args[0] = x; - self.push_goal(Goal::Query { - term: Operation { operator: op, args }.into_term(), - })?; - return Ok(QueryEvent::None); - } - _ => { - if !handle_left_var && right.value().as_symbol().is_err() { - return eval(self, term); + if let VariableState::Bound(x) = self.variable_state(l) { + args[0] = x; + self.push_goal(Goal::Query { + term: Operation { + operator: *op, + args, } - } + .into_term(), + })?; + return Ok(QueryEvent::None); + } else if !handle_unbound_left_var && right.value().as_symbol().is_err() { + return eval(self, term); } } @@ -2072,10 +2060,33 @@ impl PolarVirtualMachine { let left = &args[0]; let right = &args[1]; - if !compare(*op, left, right) { - self.push_goal(Goal::Backtrack)?; + match (left.value(), right.value()) { + // TODO(ap): ExternalInstance on one side. + (Value::ExternalInstance(_), Value::ExternalInstance(_)) => { + // Generate a symbol for the external result and bind to `false` (default). + let (call_id, answer) = + self.new_call_var("external_op_result", Value::Boolean(false)); + + // Check that the external result is `true` when we return. + self.push_goal(Goal::Unify { + left: answer, + right: Term::new_temporary(Value::Boolean(true)), + })?; + + // Emit an event for the external operation. + return Ok(QueryEvent::ExternalOp { + call_id, + operator: *op, + args: vec![left.clone(), right.clone()], + }); + } + _ => { + if !compare(*op, left, right) { + self.push_goal(Goal::Backtrack)?; + } + Ok(QueryEvent::None) + } } - Ok(QueryEvent::None) } // TODO(ap, dhatch): This does not strip the 3rd argument like Dot op (the unbound variable). From 9e771bc45d14c1d4e72fd018bb87b004539b32ea Mon Sep 17 00:00:00 2001 From: Alex Plotnick Date: Thu, 21 Jan 2021 16:42:00 -0700 Subject: [PATCH 07/18] Bump flask-oso dependencies. --- languages/python/flask-oso/requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/languages/python/flask-oso/requirements.txt b/languages/python/flask-oso/requirements.txt index cf2b15f5ad..b03db40556 100644 --- a/languages/python/flask-oso/requirements.txt +++ b/languages/python/flask-oso/requirements.txt @@ -1,2 +1,2 @@ -oso~=0.10.0 +oso>=0.10.0 flask>=0.12.0 From 304dfe4935995fa8b8c19e34949d6804bcab25ea Mon Sep 17 00:00:00 2001 From: Alex Plotnick Date: Thu, 21 Jan 2021 16:41:50 -0700 Subject: [PATCH 08/18] Fix some (but not all) JS tests. --- languages/js/src/Polar.test.ts | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/languages/js/src/Polar.test.ts b/languages/js/src/Polar.test.ts index 5fff939346..3b87ab7441 100644 --- a/languages/js/src/Polar.test.ts +++ b/languages/js/src/Polar.test.ts @@ -88,9 +88,8 @@ describe('#registerClass', () => { new Foo(\"A\").a() = x in query at line 1, column 1 new Foo(\"A\").a() = x - in query at line 1, column 1 - new Foo(\"A\").a() -Application error: Foo { a: 'A' }.a is not a function at line 1, column 1` + Foo(\"A\").a() = _value_4 +Application error: Foo { a: 'A' }.a is not a function` ); expect(qvar(p, 'x in new Foo("A").b', 'x', true)).rejects.toThrow( 'function is not iterable' @@ -692,8 +691,7 @@ describe('errors', () => { `trace (most recent evaluation last): in query at line 1, column 1 foo(1,2) - in rule foo at line 1, column 13 - a in b + 1 in 2 Type error: can only use \`in\` on an iterable value, this is Number(Integer(2)) at line 1, column 7` ); }); @@ -705,9 +703,8 @@ Type error: can only use \`in\` on an iterable value, this is Number(Integer(2)) `trace (most recent evaluation last): in query at line 1, column 1 undefined.foo - in query at line 1, column 1 - undefined.foo -Application error: Cannot read property 'foo' of undefined at line 1, column 1` + undefined.foo = _value_1 +Application error: Cannot read property 'foo' of undefined` ); }); }); From d2d46df99dfd9b2b39eb59482ddf78fa35cada8f Mon Sep 17 00:00:00 2001 From: Alex Plotnick Date: Fri, 22 Jan 2021 11:59:07 -0700 Subject: [PATCH 09/18] Actually fix JS tests. This reverts commit 304dfe4935995fa8b8c19e34949d6804bcab25ea, and instead fixes the underlying problem, which was that we were losing source information in sub-queries. --- languages/js/src/Polar.test.ts | 13 ++++++++----- polar-core/src/vm.rs | 10 ++++------ 2 files changed, 12 insertions(+), 11 deletions(-) diff --git a/languages/js/src/Polar.test.ts b/languages/js/src/Polar.test.ts index 3b87ab7441..5fff939346 100644 --- a/languages/js/src/Polar.test.ts +++ b/languages/js/src/Polar.test.ts @@ -88,8 +88,9 @@ describe('#registerClass', () => { new Foo(\"A\").a() = x in query at line 1, column 1 new Foo(\"A\").a() = x - Foo(\"A\").a() = _value_4 -Application error: Foo { a: 'A' }.a is not a function` + in query at line 1, column 1 + new Foo(\"A\").a() +Application error: Foo { a: 'A' }.a is not a function at line 1, column 1` ); expect(qvar(p, 'x in new Foo("A").b', 'x', true)).rejects.toThrow( 'function is not iterable' @@ -691,7 +692,8 @@ describe('errors', () => { `trace (most recent evaluation last): in query at line 1, column 1 foo(1,2) - 1 in 2 + in rule foo at line 1, column 13 + a in b Type error: can only use \`in\` on an iterable value, this is Number(Integer(2)) at line 1, column 7` ); }); @@ -703,8 +705,9 @@ Type error: can only use \`in\` on an iterable value, this is Number(Integer(2)) `trace (most recent evaluation last): in query at line 1, column 1 undefined.foo - undefined.foo = _value_1 -Application error: Cannot read property 'foo' of undefined` + in query at line 1, column 1 + undefined.foo +Application error: Cannot read property 'foo' of undefined at line 1, column 1` ); }); }); diff --git a/polar-core/src/vm.rs b/polar-core/src/vm.rs index 63affe2a67..9d5f674970 100644 --- a/polar-core/src/vm.rs +++ b/polar-core/src/vm.rs @@ -2015,11 +2015,10 @@ impl PolarVirtualMachine { if let VariableState::Bound(x) = self.variable_state(r) { args[1] = x; self.push_goal(Goal::Query { - term: Operation { + term: term.clone_with_value(Value::Expression(Operation { operator: *op, args, - } - .into_term(), + })), })?; return Ok(QueryEvent::None); } else if !handle_unbound_right_var && left.value().as_symbol().is_err() { @@ -2032,11 +2031,10 @@ impl PolarVirtualMachine { if let VariableState::Bound(x) = self.variable_state(l) { args[0] = x; self.push_goal(Goal::Query { - term: Operation { + term: term.clone_with_value(Value::Expression(Operation { operator: *op, args, - } - .into_term(), + })), })?; return Ok(QueryEvent::None); } else if !handle_unbound_left_var && right.value().as_symbol().is_err() { From 839830bf2bbfee32709325372463f81c9a622c7f Mon Sep 17 00:00:00 2001 From: Alex Plotnick Date: Fri, 22 Jan 2021 12:05:12 -0700 Subject: [PATCH 10/18] =?UTF-8?q?Fix=20more=20tests,=20appease=20?= =?UTF-8?q?=F0=9F=93=8E.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- languages/python/oso/tests/test_polar.py | 3 ++- languages/ruby/spec/oso/polar/polar_spec.rb | 3 ++- polar-core/src/vm.rs | 4 ++-- 3 files changed, 6 insertions(+), 4 deletions(-) diff --git a/languages/python/oso/tests/test_polar.py b/languages/python/oso/tests/test_polar.py index abfeea2abc..42e45c2800 100644 --- a/languages/python/oso/tests/test_polar.py +++ b/languages/python/oso/tests/test_polar.py @@ -426,7 +426,8 @@ def test_runtime_errors(polar, query): == """trace (most recent evaluation last): in query at line 1, column 1 foo(1,2) - 1 in 2 + in rule foo at line 2, column 17 + a in b Type error: can only use `in` on an iterable value, this is Number(Integer(2)) at line 1, column 7""" ) diff --git a/languages/ruby/spec/oso/polar/polar_spec.rb b/languages/ruby/spec/oso/polar/polar_spec.rb index e18e6a208c..d4fcda6cfe 100644 --- a/languages/ruby/spec/oso/polar/polar_spec.rb +++ b/languages/ruby/spec/oso/polar/polar_spec.rb @@ -693,7 +693,8 @@ def groups trace (most recent evaluation last): in query at line 1, column 1 foo(1,2) - 1 in 2 + in rule foo at line 1, column 13 + a in b Type error: can only use `in` on an iterable value, this is Number(Integer(2)) at line 1, column 7 TRACE expect(e.message).to eq(error) diff --git a/polar-core/src/vm.rs b/polar-core/src/vm.rs index 9d5f674970..7aad5d3b89 100644 --- a/polar-core/src/vm.rs +++ b/polar-core/src/vm.rs @@ -2072,11 +2072,11 @@ impl PolarVirtualMachine { })?; // Emit an event for the external operation. - return Ok(QueryEvent::ExternalOp { + Ok(QueryEvent::ExternalOp { call_id, operator: *op, args: vec![left.clone(), right.clone()], - }); + }) } _ => { if !compare(*op, left, right) { From f0ced341f29e97141a56126ed9930dce4064c3e9 Mon Sep 17 00:00:00 2001 From: Alex Plotnick Date: Fri, 22 Jan 2021 13:35:03 -0700 Subject: [PATCH 11/18] Mypy 0.800 compatibility See https://github.com/python/mypy/issues/9940 --- languages/python/oso/setup.cfg | 2 ++ 1 file changed, 2 insertions(+) diff --git a/languages/python/oso/setup.cfg b/languages/python/oso/setup.cfg index b74c2a70a0..9e99bc315f 100644 --- a/languages/python/oso/setup.cfg +++ b/languages/python/oso/setup.cfg @@ -3,6 +3,8 @@ xfail_strict=true filterwarnings = ignore::DeprecationWarning +[mypy] + [mypy-cffi.*] ignore_missing_imports = True From 4a57c3944447cbf69b712b8784cb3ad45e8618aa Mon Sep 17 00:00:00 2001 From: Alex Plotnick Date: Sat, 23 Jan 2021 11:08:50 -0700 Subject: [PATCH 12/18] Clean up comments & error messages in add_constraint. --- polar-core/src/vm.rs | 47 +++++++++++++++++++++++++------------------- 1 file changed, 27 insertions(+), 20 deletions(-) diff --git a/polar-core/src/vm.rs b/polar-core/src/vm.rs index 7aad5d3b89..6f495fae0b 100644 --- a/polar-core/src/vm.rs +++ b/polar-core/src/vm.rs @@ -642,24 +642,22 @@ impl PolarVirtualMachine { Ok(()) } - /// Add a single constraint to the variables referenced in it. + /// Add a single constraint operation to the variables referenced in it. + /// Precondition: Operation is either binary or ternary (binary + result var), + /// and at least one of the first two arguments is an unbound variable. + #[allow(clippy::many_single_char_names)] fn add_constraint(&mut self, term: &Term) -> PolarResult<()> { - // Preconditions: At least one argument of operation is a variable & is unbound. - // Basically want to assert here that this is a binary operation. let Operation { operator: op, args } = term.value().as_expression().unwrap(); - - assert_ne!( - op, - &Operator::And, - "Should be called with single constraint." + assert!( + !matches!(*op, Operator::And | Operator::Or), + "Expected a bare constraint." ); - assert_ne!(op, &Operator::Or, "Not or"); assert!(args.len() >= 2); let (left, right) = (&args[0], &args[1]); match (left.value(), right.value()) { - (Value::Variable(lv), Value::Variable(rv)) => { - match (self.variable_state(lv), self.variable_state(rv)) { + (Value::Variable(l), Value::Variable(r)) => { + match (self.variable_state(l), self.variable_state(r)) { (VariableState::Unbound, VariableState::Unbound) => { self.constrain(&op!(And, term.clone()))?; } @@ -686,16 +684,20 @@ impl PolarVirtualMachine { let e = cycle_constraints(c); self.constrain(&e.clone_with_new_constraint(term.clone()))?; } - (VariableState::Bound(val), _) => { + (VariableState::Bound(x), _) => { panic!( - "Variable passed to constrain must be unbound, left bound to: {}.", - val.to_polar() + "Variable {} unexpectedly bound to {} in constraint {}.", + left.to_polar(), + x.to_polar(), + term.to_polar(), ); } - (_, VariableState::Bound(val)) => { + (_, VariableState::Bound(x)) => { panic!( - "Variable passed to constrain must be unbound, right bound to: {}.", - val.to_polar() + "Variable {} unexpectedly bound to {} in constraint {}.", + right.to_polar(), + x.to_polar(), + term.to_polar(), ); } } @@ -711,11 +713,16 @@ impl PolarVirtualMachine { VariableState::Partial(e) => { self.constrain(&e.clone_with_new_constraint(term.clone()))?; } - VariableState::Bound(_) => { - panic!("Variable passed to constrain must be unbound."); + VariableState::Bound(x) => { + panic!( + "Variable {} unexpectedly bound to {} in constraint {}.", + v.0, + x.to_polar(), + term.to_polar() + ); } }, - (_, _) => panic!("Must call with at least one side as a variable."), + (_, _) => panic!("At least one side of a constraint expression must be a variable."), } Ok(()) From 5e46fb33bac8b1225b3c4508951cf15d180088b0 Mon Sep 17 00:00:00 2001 From: Alex Plotnick Date: Sat, 23 Jan 2021 11:14:57 -0700 Subject: [PATCH 13/18] Move `dot_op_helper` below `query_op_helper`. --- polar-core/src/vm.rs | 158 +++++++++++++++++++++---------------------- 1 file changed, 79 insertions(+), 79 deletions(-) diff --git a/polar-core/src/vm.rs b/polar-core/src/vm.rs index 6f495fae0b..11e998a818 100644 --- a/polar-core/src/vm.rs +++ b/polar-core/src/vm.rs @@ -1900,85 +1900,6 @@ impl PolarVirtualMachine { } } - /// Push appropriate goals for lookups on dictionaries and instances. - fn dot_op_helper(&mut self, term: &Term) -> PolarResult { - let Operation { operator: op, args } = term.value().as_expression().unwrap(); - assert_eq!(*op, Operator::Dot, "expected a dot operation"); - - let mut args = args.clone(); - assert_eq!(args.len(), 3); - let object = &args[0]; - let field = &args[1]; - let value = &args[2]; - - match object.value() { - // Push a `Lookup` goal for simple field lookups on dictionaries. - Value::Dictionary(dict) if matches!(field.value(), Value::String(_) | Value::Variable(_)) => { - self.push_goal(Goal::Lookup { - dict: dict.clone(), - field: field.clone(), - value: args.remove(2), - })? - } - // Push an `ExternalLookup` goal for external instances and built-ins. - Value::Dictionary(_) - | Value::ExternalInstance(_) - | Value::List(_) - | Value::Number(_) - | Value::String(_) => { - let value = value - .value() - .as_symbol() - .map_err(|mut e| { - e.add_stack_trace(self); - e - }) - .expect("bad lookup value"); - let call_id = self.new_call_id(value); - self.append_goals(vec![ - Goal::LookupExternal { - call_id, - field: field.clone(), - instance: object.clone(), - }, - Goal::CheckError, - ])?; - } - Value::Variable(v) => { - let constraints = match self.variable_state(v) { - VariableState::Bound(_) => panic!("should have already dereferenced {}", v), - _ if matches!(field.value(), Value::Call(_)) => { - return Err(self.set_error_context( - object, - error::RuntimeError::Unsupported { - msg: format!("cannot call method on unbound variable {}", v), - }, - )); - } - VariableState::Unbound => op!(And), - VariableState::Partial(expr) => expr, - VariableState::Cycle(c) => cycle_constraints(c), - }; - // Translate `.(object, field, value)` → `value = .(object, field)`. - let dot2 = object.clone_with_value(value!(op!(Dot, object.clone(), field.clone()))); - self.constrain( - &constraints - .clone_with_new_constraint(op!(Unify, value.clone(), dot2).into_term()), - )?; - } - _ => { - return Err(self.type_error( - &object, - format!( - "can only perform lookups on dicts and instances, this is {}", - object.to_polar() - ), - )) - } - } - Ok(QueryEvent::None) - } - /// Handle variables & constraints as arguments to various operations. /// Calls the `eval` method to handle ground terms. /// @@ -2147,6 +2068,85 @@ impl PolarVirtualMachine { } } + /// Push appropriate goals for lookups on dictionaries and instances. + fn dot_op_helper(&mut self, term: &Term) -> PolarResult { + let Operation { operator: op, args } = term.value().as_expression().unwrap(); + assert_eq!(*op, Operator::Dot, "expected a dot operation"); + + let mut args = args.clone(); + assert_eq!(args.len(), 3); + let object = &args[0]; + let field = &args[1]; + let value = &args[2]; + + match object.value() { + // Push a `Lookup` goal for simple field lookups on dictionaries. + Value::Dictionary(dict) if matches!(field.value(), Value::String(_) | Value::Variable(_)) => { + self.push_goal(Goal::Lookup { + dict: dict.clone(), + field: field.clone(), + value: args.remove(2), + })? + } + // Push an `ExternalLookup` goal for external instances and built-ins. + Value::Dictionary(_) + | Value::ExternalInstance(_) + | Value::List(_) + | Value::Number(_) + | Value::String(_) => { + let value = value + .value() + .as_symbol() + .map_err(|mut e| { + e.add_stack_trace(self); + e + }) + .expect("bad lookup value"); + let call_id = self.new_call_id(value); + self.append_goals(vec![ + Goal::LookupExternal { + call_id, + field: field.clone(), + instance: object.clone(), + }, + Goal::CheckError, + ])?; + } + Value::Variable(v) => { + let constraints = match self.variable_state(v) { + VariableState::Bound(_) => panic!("should have already dereferenced {}", v), + _ if matches!(field.value(), Value::Call(_)) => { + return Err(self.set_error_context( + object, + error::RuntimeError::Unsupported { + msg: format!("cannot call method on unbound variable {}", v), + }, + )); + } + VariableState::Unbound => op!(And), + VariableState::Partial(expr) => expr, + VariableState::Cycle(c) => cycle_constraints(c), + }; + // Translate `.(object, field, value)` → `value = .(object, field)`. + let dot2 = object.clone_with_value(value!(op!(Dot, object.clone(), field.clone()))); + self.constrain( + &constraints + .clone_with_new_constraint(op!(Unify, value.clone(), dot2).into_term()), + )?; + } + _ => { + return Err(self.type_error( + &object, + format!( + "can only perform lookups on dicts and instances, this is {}", + object.to_polar() + ), + )) + } + } + Ok(QueryEvent::None) + } + fn in_op_helper(&mut self, term: &Term) -> PolarResult { let Operation { args, .. } = term.value().as_expression().unwrap(); From 7729e0e4d19d6d13a23b36d150580210729795c7 Mon Sep 17 00:00:00 2001 From: Alex Plotnick Date: Sat, 23 Jan 2021 11:16:34 -0700 Subject: [PATCH 14/18] Remove misleading comments. --- polar-core/src/vm.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/polar-core/src/vm.rs b/polar-core/src/vm.rs index 11e998a818..6f418dc3ad 100644 --- a/polar-core/src/vm.rs +++ b/polar-core/src/vm.rs @@ -1939,7 +1939,6 @@ impl PolarVirtualMachine { }; if let Value::Variable(r) = right.value() { - // A variable on the right, ground on the left. if let VariableState::Bound(x) = self.variable_state(r) { args[1] = x; self.push_goal(Goal::Query { @@ -1955,7 +1954,6 @@ impl PolarVirtualMachine { } if let Value::Variable(l) = left.value() { - // A variable on the left, ground on the right. if let VariableState::Bound(x) = self.variable_state(l) { args[0] = x; self.push_goal(Goal::Query { From a7f9f561c249d7bebab064ae55ba4c463ee07809 Mon Sep 17 00:00:00 2001 From: Alex Plotnick Date: Sat, 23 Jan 2021 11:24:51 -0700 Subject: [PATCH 15/18] Fix TODO based on review suggestion. --- polar-core/src/vm.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/polar-core/src/vm.rs b/polar-core/src/vm.rs index 6f418dc3ad..9f70f0d1be 100644 --- a/polar-core/src/vm.rs +++ b/polar-core/src/vm.rs @@ -1985,8 +1985,7 @@ impl PolarVirtualMachine { let right = &args[1]; match (left.value(), right.value()) { - // TODO(ap): ExternalInstance on one side. - (Value::ExternalInstance(_), Value::ExternalInstance(_)) => { + (Value::ExternalInstance(_), _) | (_, Value::ExternalInstance(_)) => { // Generate a symbol for the external result and bind to `false` (default). let (call_id, answer) = self.new_call_var("external_op_result", Value::Boolean(false)); From 2d0879b42b3aefe0ecf7f49a745b49dd39087cdd Mon Sep 17 00:00:00 2001 From: Alex Plotnick Date: Wed, 27 Jan 2021 08:52:58 -0700 Subject: [PATCH 16/18] Re-mute logs during rule selection. --- polar-core/src/vm.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/polar-core/src/vm.rs b/polar-core/src/vm.rs index 9f70f0d1be..5fb6dfff28 100644 --- a/polar-core/src/vm.rs +++ b/polar-core/src/vm.rs @@ -1658,7 +1658,7 @@ impl PolarVirtualMachine { let args = predicate.args.iter().map(|t| self.deep_deref(t)).collect(); let pre_filter = generic_rule.get_applicable_rules(&args); - // self.polar_log_mute = true; + self.polar_log_mute = true; // Filter rules by applicability. vec![ From a08baa8244ff352d8388f8f122f13c20474cf0f7 Mon Sep 17 00:00:00 2001 From: Alex Plotnick Date: Wed, 27 Jan 2021 08:55:09 -0700 Subject: [PATCH 17/18] Better TODO comment. --- polar-core/src/vm.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/polar-core/src/vm.rs b/polar-core/src/vm.rs index 5fb6dfff28..4ec649eaf3 100644 --- a/polar-core/src/vm.rs +++ b/polar-core/src/vm.rs @@ -2012,8 +2012,8 @@ impl PolarVirtualMachine { } } - // TODO(ap, dhatch): This does not strip the 3rd argument like Dot op (the unbound variable). - // So may be incorrect currently. + // TODO(ap, dhatch): Rewrite 3-arg arithmetic ops as 2-arg + unify, + // like we do for dots; e.g., `+(a, b, c)` → `c = +(a, b)`. /// Evaluate arithmetic operations. fn arithmetic_op_helper(&mut self, term: &Term) -> PolarResult { let Operation { operator: op, args } = term.value().as_expression().unwrap(); From 183575117db26e64b59cd2621fc457de12c44254 Mon Sep 17 00:00:00 2001 From: Alex Plotnick Date: Wed, 27 Jan 2021 08:56:29 -0700 Subject: [PATCH 18/18] Update flask-oso oso requirement. --- languages/python/flask-oso/requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/languages/python/flask-oso/requirements.txt b/languages/python/flask-oso/requirements.txt index b03db40556..bdd1297f2c 100644 --- a/languages/python/flask-oso/requirements.txt +++ b/languages/python/flask-oso/requirements.txt @@ -1,2 +1,2 @@ -oso>=0.10.0 +oso~=0.11.0a0 flask>=0.12.0