From 092d510feb35debb84bd463ca532285a86362ecd Mon Sep 17 00:00:00 2001 From: Jeremy Wall Date: Sun, 1 Sep 2019 10:21:05 -0500 Subject: [PATCH] DEV: A bunch of comparison and in operator fixes. --- integration_tests/comparisons_test.ucg | 5 +++ integration_tests/types_test.ucg | 4 +- src/build/compile_test.rs | 35 ++++++++------- src/build/format.rs | 5 +-- src/build/opcode/environment.rs | 7 +++ src/build/opcode/mod.rs | 37 +++++++++++++++- src/build/opcode/runtime.rs | 39 +++++++++-------- src/build/opcode/test.rs | 6 +++ src/build/opcode/translate.rs | 14 +++--- src/build/opcode/vm.rs | 60 +++++++++++++++++++++++++- 10 files changed, 160 insertions(+), 52 deletions(-) diff --git a/integration_tests/comparisons_test.ucg b/integration_tests/comparisons_test.ucg index f7456de..be10b59 100644 --- a/integration_tests/comparisons_test.ucg +++ b/integration_tests/comparisons_test.ucg @@ -185,6 +185,11 @@ assert { desc = "Null valued fields are still present in tuple", }; +assert { + ok = "f" in "foo", + desc = "You can check for substrings in strings", +}; + assert { ok = true && true == true, desc = "&&: truth", diff --git a/integration_tests/types_test.ucg b/integration_tests/types_test.ucg index 2fe650a..6317b62 100644 --- a/integration_tests/types_test.ucg +++ b/integration_tests/types_test.ucg @@ -64,7 +64,9 @@ assert t.not_ok{ let test_for_field = func(name, tpl) => (name) in tpl; +let name = "foo"; + assert t.ok{ - test = test_for_field("name", {name="foo"}), + test = (name) in {foo="foo"}, desc = "bareword collisions with field names still works for `in` operator", }; \ No newline at end of file diff --git a/src/build/compile_test.rs b/src/build/compile_test.rs index 1d65887..668e74f 100644 --- a/src/build/compile_test.rs +++ b/src/build/compile_test.rs @@ -17,9 +17,8 @@ use std::rc::Rc; use regex::Regex; -use super::assets::MemoryCache; +//TODO(jwall): use super::assets::MemoryCache; use super::FileBuilder; -use crate::convert::ConverterRegistry; fn assert_build(input: &str) { let i_paths = Vec::new(); @@ -28,10 +27,10 @@ fn assert_build(input: &str) { let mut b = FileBuilder::new("", &i_paths, out_buffer, err_buffer); b.enable_validate_mode(); b.eval_string(input).unwrap(); - // FIXME(jwall): What do we want to do with the assert collector? - //if !b.assert_collector.success { - // assert!(false, b.assert_collector.failures); - //} + let env = b.environment.borrow(); + if !env.assert_results.success { + assert!(false, env.assert_results.failures.clone()); + } } fn assert_build_failure(input: &str, expect: Vec) { @@ -44,18 +43,18 @@ fn assert_build_failure(input: &str, expect: Vec) { match err { Ok(_) => { for r in expect.iter() { - // FIXME(jwall): Assert collector... - //if !b.assert_collector.success { - // if let None = r.find(&b.assert_collector.failures) { - // assert!( - // false, - // "[{}] was not found in Assertion Failures:\n{}", - // r, b.assert_collector.failures - // ); - // } - //} else { - // assert!(false, "Building input Did not panic!"); - //} + let env = b.environment.borrow(); + if !env.assert_results.success { + if let None = r.find(&env.assert_results.failures) { + assert!( + false, + "[{}] was not found in Assertion Failures:\n{}", + r, env.assert_results.failures + ); + } + } else { + assert!(false, "Building input Did not panic!"); + } } } Err(ref err) => { diff --git a/src/build/format.rs b/src/build/format.rs index 3d49ee2..5ae95ce 100644 --- a/src/build/format.rs +++ b/src/build/format.rs @@ -94,11 +94,8 @@ impl ExpressionTemplate { } if c == '}' { brace_count -= 1; - // We ignore the closing brace - if brace_count == 0 { - continue; - } } + // We ignore the closing brace if brace_count == 0 { break; } diff --git a/src/build/opcode/environment.rs b/src/build/opcode/environment.rs index 5212023..2a34b8b 100644 --- a/src/build/opcode/environment.rs +++ b/src/build/opcode/environment.rs @@ -21,6 +21,7 @@ use super::cache; use super::pointer::OpPointer; use super::Error; use super::Value; +use crate::build::AssertCollector; use crate::convert::{ConverterRegistry, ImporterRegistry}; use crate::iter::OffsetStrIter; use crate::parse::parse; @@ -35,6 +36,7 @@ where pub op_cache: cache::Ops, pub converter_registry: ConverterRegistry, pub importer_registry: ImporterRegistry, + pub assert_results: AssertCollector, pub stdout: Stdout, pub stderr: Stderr, pub env_vars: BTreeMap, // Environment Variables @@ -50,6 +52,7 @@ impl Environment { val_cache: BTreeMap::new(), env_vars: vars, op_cache: cache::Ops::new(), + assert_results: AssertCollector::new(), converter_registry: ConverterRegistry::make_registry(), importer_registry: ImporterRegistry::make_registry(), stdout: out, @@ -86,4 +89,8 @@ impl Environment { &path, ) } + + pub fn record_assert_result(&mut self, desc: &str, ok: bool) { + self.assert_results.record_assert_result(desc, ok); + } } diff --git a/src/build/opcode/mod.rs b/src/build/opcode/mod.rs index 636a643..6b316d8 100644 --- a/src/build/opcode/mod.rs +++ b/src/build/opcode/mod.rs @@ -11,6 +11,7 @@ // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. // See the License for the specific language governing permissions and // limitations under the License. +use std::collections::BTreeSet; use std::convert::{TryFrom, TryInto}; use std::rc::Rc; @@ -128,7 +129,7 @@ pub struct Module { pkg_ptr: Option, } -#[derive(PartialEq, Clone)] +#[derive(Clone)] pub enum Value { // Binding names. S(String), @@ -219,6 +220,7 @@ pub enum Op { // Spacer operation, Does nothing. Index, // indexing operation SafeIndex, // Safe indexing operation. Does Null Coelescing + Exist, Noop, // Pending Computation InitThunk(i32), // Basically just used for module return expressions @@ -239,6 +241,39 @@ pub enum Op { use super::ir::Val; +impl PartialEq for Value { + fn eq(&self, other: &Value) -> bool { + match (self, other) { + (P(left), P(right)) => left == right, + (C(List(left)), C(List(right))) => left == right, + (C(Tuple(left)), C(Tuple(right))) => { + if left.len() != right.len() { + return false; + } + for (ref lk, ref lv) in left.iter() { + let mut found = false; + for (ref rk, ref rv) in right.iter() { + if lk == rk { + found = true; + if lv != rv { + return false; + } + } + } + if !found { + return false; + } + } + true + } + (F(left), F(right)) => left == right, + (M(left), M(right)) => left == right, + (T(_), T(_)) | (S(_), S(_)) => false, + (_, _) => false, + } + } +} + impl From> for Val { fn from(val: Rc) -> Val { val.as_ref().into() diff --git a/src/build/opcode/runtime.rs b/src/build/opcode/runtime.rs index 0ae3828..b0ac30d 100644 --- a/src/build/opcode/runtime.rs +++ b/src/build/opcode/runtime.rs @@ -25,12 +25,10 @@ use super::VM; use super::{Composite, Error, Hook, Primitive, Value}; use crate::ast::Position; use crate::build::ir::Val; -use crate::build::AssertCollector; use Composite::{List, Tuple}; use Primitive::{Bool, Empty, Int, Str}; pub struct Builtins { - assert_results: AssertCollector, working_dir: PathBuf, import_path: Vec, validate_mode: bool, @@ -38,14 +36,13 @@ pub struct Builtins { impl Builtins { pub fn new() -> Self { + // FIXME(jwall): This should probably be injected in. Self::with_working_dir(std::env::current_dir().unwrap()) } pub fn with_working_dir>(path: P) -> Self { Self { - assert_results: AssertCollector::new(), working_dir: path.into(), - // FIXME(jwall): This should probably be injected in. import_path: Vec::new(), validate_mode: false, } @@ -53,7 +50,6 @@ impl Builtins { pub fn clone(&self) -> Self { Self { - assert_results: AssertCollector::new(), working_dir: self.working_dir.clone(), import_path: self.import_path.clone(), validate_mode: self.validate_mode, @@ -79,7 +75,7 @@ impl Builtins { match h { Hook::Import => self.import(stack, env, pos), Hook::Include => self.include(stack, env, pos), - Hook::Assert => self.assert(stack), + Hook::Assert => self.assert(stack, env), Hook::Convert => self.convert(stack, env, pos), Hook::Out => self.out(path, stack, env, pos), Hook::Map => self.map(stack, env, pos), @@ -166,7 +162,8 @@ impl Builtins { stack.push((v, path_pos)); } None => { - let op_pointer = decorate_error!(path_pos => borrowed_env.get_ops_for_path(path))?; + let op_pointer = + decorate_error!(path_pos => borrowed_env.get_ops_for_path(path))?; let mut vm = VM::with_pointer(op_pointer, env.clone()); vm.run()?; let result = Rc::new(vm.symbols_to_tuple(true)); @@ -244,7 +241,15 @@ impl Builtins { Ok(()) } - fn assert(&mut self, stack: &mut Vec<(Rc, Position)>) -> Result<(), Error> { + fn assert( + &mut self, + stack: &mut Vec<(Rc, Position)>, + env: Rc>>, + ) -> Result<(), Error> + where + O: std::io::Write, + E: std::io::Write, + { let tuple = stack.pop(); if let Some((val, tpl_pos)) = tuple.clone() { if let &Value::C(Tuple(ref tuple)) = val.as_ref() { @@ -253,7 +258,7 @@ impl Builtins { // look for the ok field. let mut ok = None; for &(ref name, ref val) in tuple.iter() { - if name == "description" { + if name == "desc" { desc = Some(val.clone()); } if name == "ok" { @@ -264,7 +269,7 @@ impl Builtins { if let (&Value::P(Bool(ref b)), &Value::P(Str(ref desc))) = (ok.as_ref(), desc.as_ref()) { - self.assert_results.record_assert_result(desc, *b); + env.borrow_mut().record_assert_result(desc, *b); return Ok(()); } } @@ -273,7 +278,7 @@ impl Builtins { "TYPE FAIL - Expected tuple with ok and desc fields got {:?} at {}\n", tuple, tpl_pos ); - self.assert_results.record_assert_result(&msg, false); + env.borrow_mut().record_assert_result(&msg, false); } else { unreachable!(); } @@ -412,8 +417,8 @@ impl Builtins { &C(Tuple(ref _flds)) => { let mut new_fields = Vec::new(); for (ref name, ref val) in _flds { - stack.push((val.clone(), list_pos.clone())); stack.push((Rc::new(P(Str(name.clone()))), list_pos.clone())); + stack.push((val.clone(), list_pos.clone())); let (result, result_pos) = VM::fcall_impl(f, stack, env.clone())?; if let &C(List(ref fval)) = result.as_ref() { // we expect them to be a list of exactly 2 items. @@ -515,8 +520,8 @@ impl Builtins { &C(Tuple(ref _flds)) => { let mut new_fields = Vec::new(); for (ref name, ref val) in _flds { - stack.push((val.clone(), list_pos.clone())); stack.push((Rc::new(P(Str(name.clone()))), list_pos.clone())); + stack.push((val.clone(), list_pos.clone())); let (condition, _) = VM::fcall_impl(f, stack, env.clone())?; // Check for empty or boolean results and only push e back in // if they are non empty and true @@ -630,8 +635,8 @@ impl Builtins { &C(List(ref elems)) => { for e in elems.iter() { // push function arguments on the stack. - stack.push((e.clone(), list_pos.clone())); stack.push((acc.clone(), acc_pos.clone())); + stack.push((e.clone(), list_pos.clone())); // call function and push it's result on the stack. let (new_acc, new_acc_pos) = VM::fcall_impl(f, stack, env.clone())?; acc = new_acc; @@ -641,9 +646,9 @@ impl Builtins { &C(Tuple(ref _flds)) => { for (ref name, ref val) in _flds.iter() { // push function arguments on the stack. - stack.push((val.clone(), list_pos.clone())); - stack.push((Rc::new(P(Str(name.clone()))), list_pos.clone())); stack.push((acc.clone(), acc_pos.clone())); + stack.push((Rc::new(P(Str(name.clone()))), list_pos.clone())); + stack.push((val.clone(), list_pos.clone())); // call function and push it's result on the stack. let (new_acc, new_acc_pos) = VM::fcall_impl(f, stack, env.clone())?; acc = new_acc; @@ -653,8 +658,8 @@ impl Builtins { &P(Str(ref _s)) => { for c in _s.chars() { // push function arguments on the stack. - stack.push((Rc::new(P(Str(c.to_string()))), list_pos.clone())); stack.push((acc.clone(), acc_pos.clone())); + stack.push((Rc::new(P(Str(c.to_string()))), list_pos.clone())); // call function and push it's result on the stack. let (new_acc, new_acc_pos) = VM::fcall_impl(f, stack, env.clone())?; acc = new_acc; diff --git a/src/build/opcode/test.rs b/src/build/opcode/test.rs index 8bb902a..6a576f7 100644 --- a/src/build/opcode/test.rs +++ b/src/build/opcode/test.rs @@ -645,6 +645,7 @@ fn simple_binary_expr() { "true || true;" => P(Bool(true)), "foo in {foo = 1};" => P(Bool(true)), "bar in {foo = 1};" => P(Bool(false)), + "let bar = \"foo\"; (bar) in {foo = 1};" => P(Bool(true)), ) } @@ -715,6 +716,7 @@ fn simple_format_expressions() { "\"@ @ @\" % (1, 2, 3);" => P(Str("1 2 3".to_owned())), "\"@ \\\\@\" % (1);" => P(Str("1 @".to_owned())), "\"@{item.num}\" % {num=1};" => P(Str("1".to_owned())), + "\"@{item.num()}\" % {num=func() => 1};" => P(Str("1".to_owned())), ]; } @@ -789,6 +791,7 @@ fn simple_selects() { "select \"foo\", { foo = 1, bar = 2, };" => P(Int(1)), "select \"bar\", { foo = 1, bar = 2, };" => P(Int(2)), "select \"quux\", 3, { foo = 1, bar = 2, };" => P(Int(3)), + "select true, { true = 1, false = 2, };" => P(Int(1)), ]; } @@ -810,6 +813,7 @@ fn select_compound_expressions() { "let want = \"foo\"; select want, { foo = 1, bar = 2, } == 1;" => P(Bool(true)), "let want = \"foo\"; select want, 3, { foo = 1, bar = 2, } == 1;" => P(Bool(true)), "{ok = select \"foo\", { foo = 1, bar = 2, } == 2}.ok;" => P(Bool(false)), + "{ok = func() => true}.ok();" => P(Bool(true)), ]; } @@ -905,3 +909,5 @@ fn simple_reduces() { ])), ]; } + +// TODO(jwall): functions and modules comparable? diff --git a/src/build/opcode/translate.rs b/src/build/opcode/translate.rs index 2edaf59..ebdf636 100644 --- a/src/build/opcode/translate.rs +++ b/src/build/opcode/translate.rs @@ -188,8 +188,9 @@ impl AST { BinaryExprType::IN => { // Dot expressions expect the left side to be pushed first Self::translate_expr(*def.right, &mut ops, root); - // Symbols on the right side should be converted to strings to satisfy + // Symbols on the left side should be converted to strings to satisfy // the Index operation contract. + // FIXME(jwall): List checks. match *def.left { Expression::Simple(Value::Symbol(name)) => { Self::translate_expr( @@ -202,10 +203,7 @@ impl AST { Self::translate_expr(expr, &mut ops, root); } } - ops.push(Op::SafeIndex, def.pos.clone()); - ops.push(Op::Val(Primitive::Empty), def.pos.clone()); - ops.push(Op::Equal, def.pos.clone()); - ops.push(Op::Not, def.pos); + ops.push(Op::Exist, def.pos.clone()); } BinaryExprType::DOT => { // Dot expressions expect the left side to be pushed first @@ -244,6 +242,7 @@ impl AST { } _ => unreachable!(), } + ops.push(Op::Index, def.pos); ops.push(Op::FCall, func_pos); return; } @@ -268,10 +267,7 @@ impl AST { Expression::Fail(def) => { let msg_pos = def.message.pos().clone(); Self::translate_expr(*def.message, &mut ops, root); - ops.push( - Op::Val(Primitive::Str("UserDefined: ".to_owned())), - msg_pos, - ); + ops.push(Op::Val(Primitive::Str("UserDefined: ".to_owned())), msg_pos); ops.push(Op::Add, def.pos.clone()); ops.push(Op::Bang, def.pos); } diff --git a/src/build/opcode/vm.rs b/src/build/opcode/vm.rs index 2967662..796a6df 100644 --- a/src/build/opcode/vm.rs +++ b/src/build/opcode/vm.rs @@ -139,6 +139,7 @@ where Op::Element => self.op_element()?, Op::Index => self.op_index(false, pos)?, Op::SafeIndex => self.op_index(true, pos)?, + Op::Exist => self.op_exist(pos)?, Op::Cp => self.op_copy(pos)?, //FIXME(jwall): Should this take a user provided message? Op::Bang => self.op_bang()?, @@ -286,6 +287,15 @@ where // compare them. let matched = match (field_name.as_ref(), search.as_ref()) { (&S(ref fname), &P(Str(ref sname))) | (&S(ref fname), &S(ref sname)) => fname == sname, + (&S(ref fname), &P(Bool(b))) => { + if fname == "true" && b { + true + } else if fname == "false" && !b { + true + } else { + false + } + } _ => false, }; if !matched { @@ -377,6 +387,8 @@ where } let mut ops = self.ops.clone(); ops.jump(idx)?; + // Our arguments will be pulled off the stack in reverse order; + bindings.reverse(); self.push( Rc::new(F(Func { ptr: ops, // where the function starts. @@ -634,8 +646,8 @@ where let name = if let &S(ref s) | &P(Str(ref s)) = name_val.as_ref() { s } else { - //dbg!(name_val); - //dbg!(val); + dbg!(name_val); + dbg!(val); unreachable!(); }; // get composite tuple from stack @@ -715,6 +727,50 @@ where )); } + fn op_exist(&mut self, pos: Position) -> Result<(), Error> { + let (right, right_pos) = self.pop()?; + let (left, left_pos) = self.pop()?; + match left.as_ref() { + &C(Tuple(ref flds)) => { + if let &P(Str(ref name)) = right.as_ref() { + for (ref nm, _) in flds { + if nm == name { + self.push(Rc::new(P(Bool(true))), pos)?; + return Ok(()); + } + } + } else { + return Err(Error::new( + format!("Expected String or Symbol got: {}", right.type_name()), + right_pos, + )); + } + } + &C(List(ref elems)) => { + for e in elems { + if e == &right { + self.push(Rc::new(P(Bool(true))), pos)?; + return Ok(()); + } + } + } + &P(Str(ref s)) => { + if let &P(Str(ref part)) = right.as_ref() { + self.push(Rc::new(P(Bool(s.contains(part)))), pos)?; + return Ok(()); + } + } + _ => { + return Err(Error::new( + format!("Expected String, Tuple, or List got: {}", left.type_name()), + left_pos, + )); + } + }; + self.push(Rc::new(P(Bool(false))), pos)?; + Ok(()) + } + fn op_copy(&mut self, pos: Position) -> Result<(), Error> { // This value should always be a tuple let (override_val, _) = self.pop()?;