From 685ee7407e0b7a5424f86ae577fb000cfb73fa10 Mon Sep 17 00:00:00 2001 From: Jeremy Wall Date: Mon, 14 Jan 2019 18:23:39 -0600 Subject: [PATCH] FEATURE: make macros close over their environment. The benefits are great enough to allow this and the benefits of not allowing closures are not terribly useful. We do not get the same benefits for modules though so we don't add it to them. --- integration_tests/macros_test.ucg | 11 +++ src/ast/mod.rs | 101 ++------------------------ src/ast/test.rs | 115 ------------------------------ src/build/mod.rs | 48 ++++++++----- src/build/scope.rs | 2 +- src/build/test.rs | 1 + src/parse/mod.rs | 1 + 7 files changed, 48 insertions(+), 231 deletions(-) delete mode 100644 src/ast/test.rs diff --git a/integration_tests/macros_test.ucg b/integration_tests/macros_test.ucg index 2d41892..a5c1a62 100644 --- a/integration_tests/macros_test.ucg +++ b/integration_tests/macros_test.ucg @@ -64,4 +64,15 @@ let arg_tpl = { assert { ok = macro_tpl_arg(arg_tpl).result == arg_tpl.field, desc = "macro_tpl_arg(arg_tpl).result == arg_tpl.field", +}; + +let closed_over = "foo"; + +let closure = macro(arg1) => { + result = closed_over + arg1, +}; + +assert { + ok = closure("bar").result == "foobar", + desc = "we closed over closed_over and got @" % (closure("bar").result), }; \ No newline at end of file diff --git a/src/ast/mod.rs b/src/ast/mod.rs index 6ccd747..5eb2202 100644 --- a/src/ast/mod.rs +++ b/src/ast/mod.rs @@ -19,7 +19,6 @@ use std::cmp::Eq; use std::cmp::Ordering; use std::cmp::PartialEq; use std::cmp::PartialOrd; -use std::collections::HashSet; use std::convert::Into; use std::fmt; use std::hash::Hash; @@ -29,6 +28,7 @@ use std::rc::Rc; use abortable_parser; +use crate::build::scope::Scope; use crate::build::Val; pub mod walk; @@ -377,104 +377,12 @@ impl<'a> From<&'a PositionedItem> for PositionedItem { /// any values except what is defined in their arguments. #[derive(PartialEq, Debug, Clone)] pub struct MacroDef { + pub scope: Option, pub argdefs: Vec>, pub fields: FieldList, pub pos: Position, } -impl MacroDef { - fn symbol_is_in_args(&self, sym: &String) -> bool { - for arg in self.argdefs.iter() { - if &arg.val == sym { - return true; - } - } - return false; - } - - fn validate_value_symbols<'a>( - &self, - stack: &mut Vec<&'a Expression>, - val: &'a Value, - ) -> HashSet { - let mut bad_symbols = HashSet::new(); - if let &Value::Symbol(ref name) = val { - if name.val != "self" && !self.symbol_is_in_args(&name.val) { - bad_symbols.insert(name.val.clone()); - } - } else if let &Value::Tuple(ref tuple_node) = val { - let fields = &tuple_node.val; - for &(_, ref expr) in fields.iter() { - stack.push(expr); - } - } else if let &Value::List(ref def) = val { - for elem in def.elems.iter() { - stack.push(elem); - } - } - return bad_symbols; - } - - /// Performs typechecking of a ucg macro's arguments to ensure - /// that they are valid for the expressions in the macro. - pub fn validate_symbols(&self) -> Result<(), HashSet> { - let mut bad_symbols = HashSet::new(); - for &(_, ref expr) in self.fields.iter() { - let mut stack = Vec::new(); - stack.push(expr); - while stack.len() > 0 { - match stack.pop().unwrap() { - &Expression::Binary(ref bexpr) => { - stack.push(&bexpr.left); - if bexpr.kind != BinaryExprType::DOT { - stack.push(&bexpr.right); - } - } - &Expression::Grouped(ref expr) => { - stack.push(expr); - } - &Expression::Format(ref def) => { - let exprs = &def.args; - for arg_expr in exprs.iter() { - stack.push(arg_expr); - } - } - &Expression::Select(ref def) => { - stack.push(def.default.borrow()); - stack.push(def.val.borrow()); - for &(_, ref expr) in def.tuple.iter() { - stack.push(expr); - } - } - &Expression::Call(ref def) => { - for expr in def.arglist.iter() { - stack.push(expr); - } - } - &Expression::Simple(ref val) => { - let mut syms_set = self.validate_value_symbols(&mut stack, val); - bad_symbols.extend(syms_set.drain()); - } - &Expression::Macro(_) - | &Expression::Copy(_) - | &Expression::Module(_) - | &Expression::Range(_) - | &Expression::FuncOp(_) - | &Expression::Import(_) - | &Expression::Include(_) => { - // noop - continue; - } - } - } - } - if bad_symbols.len() > 0 { - return Err(bad_symbols); - } - return Ok(()); - } -} - /// Specifies the types of binary operations supported in /// UCG expression. #[derive(Debug, PartialEq, Clone)] @@ -604,6 +512,7 @@ impl FuncOpDef { // TODO(jwall): this should probably be moved to a Val::Module IR type. #[derive(Debug, PartialEq, Clone)] pub struct ModuleDef { + pub scope: Option, pub pos: Position, pub arg_set: FieldList, pub arg_tuple: Option>, @@ -613,6 +522,7 @@ pub struct ModuleDef { impl ModuleDef { pub fn new>(arg_set: FieldList, stmts: Vec, pos: P) -> Self { ModuleDef { + scope: None, pos: pos.into(), arg_set: arg_set, arg_tuple: None, @@ -782,6 +692,3 @@ pub enum Statement { // Identify an Expression for output. Output(Token, Expression), } - -#[cfg(test)] -pub mod test; diff --git a/src/ast/test.rs b/src/ast/test.rs deleted file mode 100644 index e16bee6..0000000 --- a/src/ast/test.rs +++ /dev/null @@ -1,115 +0,0 @@ -// Copyright 2017 Jeremy Wall -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// 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 super::*; - -#[test] -pub fn test_macro_validation_happy_path() { - let def = MacroDef { - argdefs: vec![value_node!("foo".to_string(), Position::new(1, 0, 0))], - fields: vec![( - make_tok!("f1", Position::new(1, 1, 0)), - Expression::Binary(BinaryOpDef { - kind: BinaryExprType::Add, - left: Box::new(Expression::Simple(Value::Symbol(value_node!( - "foo".to_string(), - Position::new(1, 1, 0) - )))), - right: Box::new(Expression::Simple(Value::Int(value_node!( - 1, - Position::new(1, 1, 0) - )))), - pos: Position::new(1, 0, 0), - }), - )], - pos: Position::new(1, 0, 0), - }; - assert!(def.validate_symbols().unwrap() == ()); -} - -#[test] -pub fn test_macro_validation_fail() { - let def = MacroDef { - argdefs: vec![value_node!("foo".to_string(), Position::new(1, 0, 0))], - fields: vec![( - make_tok!("f1", Position::new(1, 1, 0)), - Expression::Binary(BinaryOpDef { - kind: BinaryExprType::Add, - left: Box::new(Expression::Simple(Value::Symbol(value_node!( - "bar".to_string(), - Position::new(1, 1, 0) - )))), - right: Box::new(Expression::Simple(Value::Int(value_node!( - 1, - Position::new(1, 1, 0) - )))), - pos: Position::new(1, 0, 0), - }), - )], - pos: Position::new(1, 0, 0), - }; - let mut expected = HashSet::new(); - expected.insert("bar".to_string()); - assert_eq!(def.validate_symbols().err().unwrap(), expected); -} - -#[test] -pub fn test_macro_validation_selector_happy_path() { - let def = MacroDef { - argdefs: vec![value_node!("foo".to_string(), Position::new(1, 0, 0))], - fields: vec![( - make_tok!("f1", Position::new(1, 1, 0)), - Expression::Binary(BinaryOpDef { - kind: BinaryExprType::Add, - left: Box::new(Expression::Simple(Value::Symbol(PositionedItem::new( - "foo".to_string(), - Position::new(1, 1, 0), - )))), - right: Box::new(Expression::Simple(Value::Int(value_node!( - 1, - Position::new(1, 1, 0) - )))), - pos: Position::new(1, 0, 0), - }), - )], - pos: Position::new(1, 0, 0), - }; - assert!(def.validate_symbols().unwrap() == ()); -} - -#[test] -pub fn test_macro_validation_selector_fail() { - let def = MacroDef { - argdefs: vec![value_node!("foo".to_string(), Position::new(1, 0, 0))], - fields: vec![( - make_tok!("f1", Position::new(1, 1, 0)), - Expression::Binary(BinaryOpDef { - kind: BinaryExprType::Add, - left: Box::new(Expression::Simple(Value::Symbol(PositionedItem::new( - "bar".to_string(), - Position::new(1, 1, 0), - )))), - right: Box::new(Expression::Simple(Value::Int(value_node!( - 1, - Position::new(1, 1, 0) - )))), - pos: Position::new(1, 0, 0), - }), - )], - pos: Position::new(1, 0, 0), - }; - let mut expected = HashSet::new(); - expected.insert("bar".to_string()); - assert_eq!(def.validate_symbols(), Err(expected)); -} diff --git a/src/build/mod.rs b/src/build/mod.rs index 540707d..48aade6 100644 --- a/src/build/mod.rs +++ b/src/build/mod.rs @@ -72,12 +72,18 @@ impl MacroDef { // If the expressions reference Symbols not defined in the MacroDef that is also an error. // TODO(jwall): We should probably enforce that the Expression Symbols must be in argdefs rules // at Macro definition time not evaluation time. - let mut scope = HashMap::, Rc>::new(); + let mut build_output = HashMap::, Rc>::new(); for (i, arg) in args.drain(0..).enumerate() { - scope.entry(self.argdefs[i].clone()).or_insert(arg.clone()); + build_output + .entry(self.argdefs[i].clone()) + .or_insert(arg.clone()); } let mut b = parent_builder.clone_builder(root); - b.set_build_output(scope); + if let Some(ref scope) = self.scope { + b.scope = scope.spawn_child(); + } + // We clobber anything that used to be in the scope with the arguments. + b.merge_build_output(build_output, true); let mut result: Vec<(PositionedItem, Rc)> = Vec::new(); for &(ref key, ref expr) in self.fields.iter() { // We clone the expressions here because this macro may be consumed @@ -216,6 +222,16 @@ impl<'a> FileBuilder<'a> { self.scope.build_output = scope; } + pub fn merge_build_output(&mut self, scope: ValueMap, clobber: bool) { + for (name, value) in scope.iter() { + if !clobber && !self.scope.build_output.contains_key(name) { + self.scope.build_output.insert(name.clone(), value.clone()); + } else { + self.scope.build_output.insert(name.clone(), value.clone()); + } + } + } + pub fn set_strict(&mut self, to: bool) { self.strict = to; } @@ -1104,19 +1120,9 @@ impl<'a> FileBuilder<'a> { ))) } - fn eval_macro_def(&self, def: &MacroDef) -> Result, Box> { - match def.validate_symbols() { - Ok(()) => Ok(Rc::new(Val::Macro(def.clone()))), - Err(set) => Err(Box::new(error::BuildError::new( - format!( - "Macro has the following \ - undefined symbols: {:?}", - set - ), - error::ErrorType::NoSuchSymbol, - def.pos.clone(), - ))), - } + fn eval_macro_def(&self, def: &mut MacroDef, scope: &Scope) -> Result, Box> { + def.scope = Some(scope.spawn_child()); + Ok(Rc::new(Val::Macro(def.clone()))) } fn file_dir(&self) -> PathBuf { @@ -1625,8 +1631,14 @@ impl<'a> FileBuilder<'a> { &Expression::Grouped(ref expr) => self.eval_expr(expr, scope), &Expression::Format(ref def) => self.eval_format(def, scope), &Expression::Call(ref def) => self.eval_call(def, scope), - &Expression::Macro(ref def) => self.eval_macro_def(def), - &Expression::Module(ref def) => self.eval_module_def(def, scope), + &Expression::Macro(ref def) => { + let mut def_clone = def.clone(); + self.eval_macro_def(&mut def_clone, scope) + } + &Expression::Module(ref def) => { + let mut def_clone = def.clone(); + self.eval_module_def(&mut def_clone, scope) + } &Expression::Select(ref def) => self.eval_select(def, scope), &Expression::FuncOp(ref def) => self.eval_func_op(def, scope), &Expression::Include(ref def) => self.eval_include(def), diff --git a/src/build/scope.rs b/src/build/scope.rs index 6e124f6..aea6f53 100644 --- a/src/build/scope.rs +++ b/src/build/scope.rs @@ -33,7 +33,7 @@ pub type ValueMap = HashMap, Rc>; /// /// UCG Scopes do not descend up into their parent scopes so we do not maintain a stack /// for those. -#[derive(Clone)] +#[derive(Debug, PartialEq, Clone)] pub struct Scope { pub import_stack: Vec, pub env: Rc, diff --git a/src/build/test.rs b/src/build/test.rs index b7f41bd..d6eb9d7 100644 --- a/src/build/test.rs +++ b/src/build/test.rs @@ -248,6 +248,7 @@ fn test_macro_hermetic() { .build_output .entry(value_node!("tstmac".to_string(), Position::new(1, 0, 0))) .or_insert(Rc::new(Val::Macro(MacroDef { + scope: None, argdefs: vec![value_node!("arg2".to_string(), Position::new(1, 0, 0))], fields: vec![( make_tok!("foo", Position::new(1, 1, 1)), diff --git a/src/parse/mod.rs b/src/parse/mod.rs index e4b3079..2f6b677 100644 --- a/src/parse/mod.rs +++ b/src/parse/mod.rs @@ -361,6 +361,7 @@ fn tuple_to_macro<'a>( .collect(); match val { Value::Tuple(v) => Ok(Expression::Macro(MacroDef { + scope: None, argdefs: arglist, fields: v.val, pos: pos,