From ffcc9ad748e3f54c8aab98e5bf3247abcd2a5e63 Mon Sep 17 00:00:00 2001 From: Ole Martin Ruud Date: Tue, 22 Oct 2019 18:56:21 +0200 Subject: [PATCH] Implement block scoped variable declarations (#173) * Implement block scoped variable declarations `const` and `let` are now scoped to the block, while `var` is scoped to the surronding function (or global). Another bigger change is that all tests have been changed to use `var` instead of `let` or `const`. This is because every time `forward` is called with some new Javascript to evaluate, we parse it as a block, hence variables can only persist across calls to `forward` if they are defined using `var`. I assume it is intentional to parse each call as a separate block, because a block is the only `ExprDef` which can contain multiple statements. Closes #39 * Prefer environment parent over environment_stack Instead of iterating over the `environment_stack` we rather use `get_outer_environment` as it will be a better fit when asyncronous functions are implemented. * Ensure variable from outer scope is assigned Variables that are defined outside a block should be changeable within the scope. Just because variable is undefined does not mean it is not initialized. --- .../declarative_environment_record.rs | 2 +- src/lib/environment/lexical_environment.rs | 190 ++++++++++++++---- src/lib/exec.rs | 78 ++++--- src/lib/js/array.rs | 36 ++-- src/lib/js/boolean.rs | 16 +- src/lib/js/function.rs | 2 +- src/lib/js/regexp.rs | 8 +- src/lib/js/string.rs | 48 ++--- 8 files changed, 259 insertions(+), 121 deletions(-) diff --git a/src/lib/environment/declarative_environment_record.rs b/src/lib/environment/declarative_environment_record.rs index b689bd6c466..ef14575b382 100644 --- a/src/lib/environment/declarative_environment_record.rs +++ b/src/lib/environment/declarative_environment_record.rs @@ -161,7 +161,7 @@ impl EnvironmentRecordTrait for DeclarativeEnvironmentRecord { } fn get_outer_environment(&self) -> Option { - None + self.outer_env.as_ref().cloned() } fn set_outer_environment(&mut self, env: Environment) { diff --git a/src/lib/environment/lexical_environment.rs b/src/lib/environment/lexical_environment.rs index e8e5fcaee5b..d5093db9462 100644 --- a/src/lib/environment/lexical_environment.rs +++ b/src/lib/environment/lexical_environment.rs @@ -32,6 +32,15 @@ pub enum EnvironmentType { Object, } +/// The scope of a given variable +#[derive(Debug, Clone, Copy)] +pub enum VariableScope { + /// The variable declaration is scoped to the current block (`let` and `const`) + Block, + /// The variable declaration is scoped to the current function (`var`) + Function, +} + #[derive(Debug)] pub struct LexicalEnvironment { environment_stack: VecDeque, @@ -90,6 +99,12 @@ impl LexicalEnvironment { self.environment_stack.pop_back(); } + pub fn environments(&self) -> impl Iterator { + std::iter::successors(Some(self.get_current_environment_ref().clone()), |env| { + env.borrow().get_outer_environment() + }) + } + pub fn get_global_object(&self) -> Option { self.environment_stack .get(0) @@ -98,25 +113,74 @@ impl LexicalEnvironment { .get_global_object() } - pub fn create_mutable_binding(&mut self, name: String, deletion: bool) { - self.get_current_environment() - .borrow_mut() - .create_mutable_binding(name, deletion) + pub fn create_mutable_binding(&mut self, name: String, deletion: bool, scope: VariableScope) { + match scope { + VariableScope::Block => self + .get_current_environment() + .borrow_mut() + .create_mutable_binding(name, deletion), + VariableScope::Function => { + // Find the first function or global environment (from the top of the stack) + let env = self + .environments() + .find(|env| match env.borrow().get_environment_type() { + EnvironmentType::Function | EnvironmentType::Global => true, + _ => false, + }) + .expect("No function or global environment"); + + env.borrow_mut().create_mutable_binding(name, deletion); + } + } } - pub fn create_immutable_binding(&mut self, name: String, deletion: bool) -> bool { - self.get_current_environment() - .borrow_mut() - .create_immutable_binding(name, deletion) + pub fn create_immutable_binding( + &mut self, + name: String, + deletion: bool, + scope: VariableScope, + ) -> bool { + match scope { + VariableScope::Block => self + .get_current_environment() + .borrow_mut() + .create_immutable_binding(name, deletion), + VariableScope::Function => { + // Find the first function or global environment (from the top of the stack) + let env = self + .environments() + .find(|env| match env.borrow().get_environment_type() { + EnvironmentType::Function | EnvironmentType::Global => true, + _ => false, + }) + .expect("No function or global environment"); + + #[allow(clippy::let_and_return)] + // FIXME need to assign result to a variable to avoid borrow checker error + // (borrowed value `env` does not live long enough) + let b = env.borrow_mut().create_immutable_binding(name, deletion); + b + } + } } pub fn set_mutable_binding(&mut self, name: &str, value: Value, strict: bool) { - let env = self.get_current_environment(); + // Find the first environment which has the given binding + let env = self + .environments() + .find(|env| env.borrow().has_binding(name)) + .expect("Binding does not exists"); // TODO graceful error handling + env.borrow_mut().set_mutable_binding(name, value, strict); } pub fn initialize_binding(&mut self, name: &str, value: Value) { - let env = self.get_current_environment(); + // Find the first environment which has the given binding + let env = self + .environments() + .find(|env| env.borrow().has_binding(name)) + .expect("Binding does not exists"); // TODO graceful error handling + env.borrow_mut().initialize_binding(name, value); } @@ -138,37 +202,16 @@ impl LexicalEnvironment { .expect("Could not get mutable reference to back object") } - pub fn get_binding_value(&mut self, name: &str) -> Value { - let env: Environment = self.get_current_environment().clone(); - let borrowed_env = env.borrow(); - let result = borrowed_env.has_binding(name); - if result { - return borrowed_env.get_binding_value(name, false); - } - - // Check outer scope - if borrowed_env.get_outer_environment().is_some() { - let mut outer: Option = borrowed_env.get_outer_environment(); - while outer.is_some() { - if outer - .as_ref() - .expect("Could not get outer as reference") - .borrow() - .has_binding(&name) - { - return outer - .expect("Outer was None") - .borrow() - .get_binding_value(name, false); - } - outer = outer - .expect("Outer was None") - .borrow() - .get_outer_environment(); - } - } + pub fn has_binding(&self, name: &str) -> bool { + self.environments() + .any(|env| env.borrow().has_binding(name)) + } - Gc::new(ValueData::Undefined) + pub fn get_binding_value(&mut self, name: &str) -> Value { + self.environments() + .find(|env| env.borrow().has_binding(name)) + .map(|env| env.borrow().get_binding_value(name, false)) + .unwrap_or_else(|| Gc::new(ValueData::Undefined)) } } @@ -236,3 +279,70 @@ pub fn new_global_environment(global: Value, this_value: Value) -> Environment { var_names: HashSet::new(), }))) } + +#[cfg(test)] +mod tests { + use crate::exec; + + #[test] + fn let_is_blockscoped() { + let scenario = r#" + { + let bar = "bar"; + } + bar == undefined; + "#; + + assert_eq!(&exec(scenario), "true"); + } + + #[test] + fn const_is_blockscoped() { + let scenario = r#" + { + const bar = "bar"; + } + bar == undefined; + "#; + + assert_eq!(&exec(scenario), "true"); + } + + #[test] + fn var_not_blockscoped() { + let scenario = r#" + { + var bar = "bar"; + } + bar == "bar"; + "#; + + assert_eq!(&exec(scenario), "true"); + } + + #[test] + fn set_outer_var_in_blockscope() { + let scenario = r#" + var bar; + { + bar = "foo"; + } + bar == "foo"; + "#; + + assert_eq!(&exec(scenario), "true"); + } + + #[test] + fn set_outer_let_in_blockscope() { + let scenario = r#" + let bar; + { + bar = "foo"; + } + bar == "foo"; + "#; + + assert_eq!(&exec(scenario), "true"); + } +} diff --git a/src/lib/exec.rs b/src/lib/exec.rs index a7ed43587d1..11ea7393fd1 100644 --- a/src/lib/exec.rs +++ b/src/lib/exec.rs @@ -1,5 +1,7 @@ use crate::{ - environment::lexical_environment::new_function_environment, + environment::lexical_environment::{ + new_declarative_environment, new_function_environment, VariableScope, + }, js::{ function::{create_unmapped_arguments_object, Function, RegularFunction}, object::{ObjectKind, INSTANCE_PROTOTYPE, PROTOTYPE}, @@ -71,6 +73,13 @@ impl Executor for Interpreter { ExprDef::Const(Const::String(ref str)) => Ok(to_value(str.to_owned())), ExprDef::Const(Const::Bool(val)) => Ok(to_value(val)), ExprDef::Block(ref es) => { + { + let env = &mut self.realm.environment; + env.push(new_declarative_environment(Some( + env.get_current_environment_ref().clone(), + ))); + } + let mut obj = to_value(None::<()>); for e in es.iter() { let val = self.run(e)?; @@ -84,6 +93,8 @@ impl Executor for Interpreter { obj = val; } } + + self.realm.environment.pop(); Ok(obj) } ExprDef::Local(ref name) => { @@ -215,9 +226,11 @@ impl Executor for Interpreter { Function::RegularFunc(RegularFunction::new(*expr.clone(), args.clone())); let val = Gc::new(ValueData::Function(Box::new(GcCell::new(function)))); if name.is_some() { - self.realm - .environment - .create_mutable_binding(name.clone().expect("No name was supplied"), false); + self.realm.environment.create_mutable_binding( + name.clone().expect("No name was supplied"), + false, + VariableScope::Function, + ); self.realm.environment.initialize_binding( name.as_ref().expect("Could not get name as reference"), val.clone(), @@ -355,7 +368,11 @@ impl Executor for Interpreter { for i in 0..data.args.len() { let name = data.args.get(i).expect("Could not get data argument"); let expr = v_args.get(i).expect("Could not get argument"); - env.create_mutable_binding(name.clone(), false); + env.create_mutable_binding( + name.clone(), + false, + VariableScope::Function, + ); env.initialize_binding(name, expr.to_owned()); } let result = self.run(&data.expr); @@ -380,16 +397,17 @@ impl Executor for Interpreter { let val = self.run(val_e)?; match ref_e.def { ExprDef::Local(ref name) => { - if *self.realm.environment.get_binding_value(&name) != ValueData::Undefined - { + if self.realm.environment.has_binding(name) { // Binding already exists self.realm .environment .set_mutable_binding(&name, val.clone(), true); } else { - self.realm - .environment - .create_mutable_binding(name.clone(), true); + self.realm.environment.create_mutable_binding( + name.clone(), + true, + VariableScope::Function, + ); self.realm.environment.initialize_binding(name, val.clone()); } } @@ -408,9 +426,11 @@ impl Executor for Interpreter { Some(v) => self.run(&v)?, None => Gc::new(ValueData::Undefined), }; - self.realm - .environment - .create_mutable_binding(name.clone(), false); + self.realm.environment.create_mutable_binding( + name.clone(), + false, + VariableScope::Function, + ); self.realm.environment.initialize_binding(&name, val); } Ok(Gc::new(ValueData::Undefined)) @@ -422,18 +442,22 @@ impl Executor for Interpreter { Some(v) => self.run(&v)?, None => Gc::new(ValueData::Undefined), }; - self.realm - .environment - .create_mutable_binding(name.clone(), false); + self.realm.environment.create_mutable_binding( + name.clone(), + false, + VariableScope::Block, + ); self.realm.environment.initialize_binding(&name, val); } Ok(Gc::new(ValueData::Undefined)) } ExprDef::ConstDecl(ref vars) => { for (name, value) in vars.iter() { - self.realm - .environment - .create_immutable_binding(name.clone(), false); + self.realm.environment.create_immutable_binding( + name.clone(), + false, + VariableScope::Block, + ); let val = self.run(&value)?; self.realm.environment.initialize_binding(&name, val); } @@ -485,9 +509,11 @@ impl Interpreter { for i in 0..data.args.len() { let name = data.args.get(i).expect("Could not get data argument"); let expr: &Value = arguments_list.get(i).expect("Could not get argument"); - self.realm - .environment - .create_mutable_binding(name.clone(), false); + self.realm.environment.create_mutable_binding( + name.clone(), + false, + VariableScope::Function, + ); self.realm .environment .initialize_binding(name, expr.clone()); @@ -495,9 +521,11 @@ impl Interpreter { // Add arguments object let arguments_obj = create_unmapped_arguments_object(arguments_list); - self.realm - .environment - .create_mutable_binding("arguments".to_string(), false); + self.realm.environment.create_mutable_binding( + "arguments".to_string(), + false, + VariableScope::Function, + ); self.realm .environment .initialize_binding("arguments", arguments_obj); diff --git a/src/lib/js/array.rs b/src/lib/js/array.rs index 51618b8f72e..07dbafe029f 100644 --- a/src/lib/js/array.rs +++ b/src/lib/js/array.rs @@ -528,8 +528,8 @@ mod tests { let realm = Realm::create(); let mut engine = Executor::new(realm); let init = r#" - let empty = new Array(); - let one = new Array(1); + var empty = new Array(); + var one = new Array(1); "#; forward(&mut engine, init); // Empty ++ Empty @@ -551,9 +551,9 @@ mod tests { let realm = Realm::create(); let mut engine = Executor::new(realm); let init = r#" - let empty = [ ]; - let one = ["a"]; - let many = ["a", "b", "c"]; + var empty = [ ]; + var one = ["a"]; + var many = ["a", "b", "c"]; "#; forward(&mut engine, init); // Empty @@ -573,9 +573,9 @@ mod tests { let mut engine = Executor::new(realm); // taken from https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/every let init = r#" - let empty = []; + var empty = []; - let array = [11, 23, 45]; + var array = [11, 23, 45]; function callback(element) { return element > 10; } @@ -583,13 +583,13 @@ mod tests { return element < 10; } - let appendArray = [1,2,3,4]; + var appendArray = [1,2,3,4]; function appendingCallback(elem,index,arr) { arr.push('new'); return elem !== "new"; } - let delArray = [1,2,3,4]; + var delArray = [1,2,3,4]; function deletingCallback(elem,index,arr) { arr.pop() return elem < 3; @@ -620,7 +620,7 @@ mod tests { function comp(a) { return a == "a"; } - let many = ["a", "b", "c"]; + var many = ["a", "b", "c"]; "#; forward(&mut engine, init); let found = forward(&mut engine, "many.find(comp)"); @@ -658,10 +658,10 @@ mod tests { let realm = Realm::create(); let mut engine = Executor::new(realm); let init = r#" - let empty = [ ]; - let one = ["a"]; - let many = ["a", "b", "c"]; - let duplicates = ["a", "b", "c", "a", "b"]; + var empty = [ ]; + var one = ["a"]; + var many = ["a", "b", "c"]; + var duplicates = ["a", "b", "c", "a", "b"]; "#; forward(&mut engine, init); @@ -722,10 +722,10 @@ mod tests { let realm = Realm::create(); let mut engine = Executor::new(realm); let init = r#" - let empty = [ ]; - let one = ["a"]; - let many = ["a", "b", "c"]; - let duplicates = ["a", "b", "c", "a", "b"]; + var empty = [ ]; + var one = ["a"]; + var many = ["a", "b", "c"]; + var duplicates = ["a", "b", "c", "a", "b"]; "#; forward(&mut engine, init); diff --git a/src/lib/js/boolean.rs b/src/lib/js/boolean.rs index 83a980bd090..8a645f7eec5 100644 --- a/src/lib/js/boolean.rs +++ b/src/lib/js/boolean.rs @@ -108,8 +108,8 @@ mod tests { let realm = Realm::create(); let mut engine = Executor::new(realm); let init = r#" - const one = new Boolean(1); - const zero = Boolean(0); + var one = new Boolean(1); + var zero = Boolean(0); "#; forward(&mut engine, init); let one = forward_val(&mut engine, "one").unwrap(); @@ -124,10 +124,10 @@ mod tests { let realm = Realm::create(); let mut engine = Executor::new(realm); let init = r#" - const trueVal = new Boolean(true); - const trueNum = new Boolean(1); - const trueString = new Boolean("true"); - const trueBool = new Boolean(trueVal); + var trueVal = new Boolean(true); + var trueNum = new Boolean(1); + var trueString = new Boolean("true"); + var trueBool = new Boolean(trueVal); "#; forward(&mut engine, init); @@ -154,8 +154,8 @@ mod tests { let realm = Realm::create(); let mut engine = Executor::new(realm); let init = r#" - const boolInstance = new Boolean(true); - const boolProto = Boolean.prototype; + var boolInstance = new Boolean(true); + var boolProto = Boolean.prototype; "#; forward(&mut engine, init); diff --git a/src/lib/js/function.rs b/src/lib/js/function.rs index b20014975f6..67fa1505eab 100644 --- a/src/lib/js/function.rs +++ b/src/lib/js/function.rs @@ -145,7 +145,7 @@ mod tests { function jason(a, b) { return arguments[0]; } - const val = jason(100, 6); + var val = jason(100, 6); "#; forward(&mut engine, init); diff --git a/src/lib/js/regexp.rs b/src/lib/js/regexp.rs index e2d9d0fa953..cf22b782bbf 100644 --- a/src/lib/js/regexp.rs +++ b/src/lib/js/regexp.rs @@ -365,9 +365,9 @@ mod tests { let realm = Realm::create(); let mut engine = Executor::new(realm); let init = r#" - let constructed = new RegExp("[0-9]+(\\.[0-9]+)?"); - let literal = /[0-9]+(\.[0-9]+)?/; - let ctor_literal = new RegExp(/[0-9]+(\.[0-9]+)?/); + var constructed = new RegExp("[0-9]+(\\.[0-9]+)?"); + var literal = /[0-9]+(\.[0-9]+)?/; + var ctor_literal = new RegExp(/[0-9]+(\.[0-9]+)?/); "#; forward(&mut engine, init); @@ -416,7 +416,7 @@ mod tests { let realm = Realm::create(); let mut engine = Executor::new(realm); let init = r#" - let regex = /[0-9]+(\.[0-9]+)?/g; + var regex = /[0-9]+(\.[0-9]+)?/g; "#; forward(&mut engine, init); diff --git a/src/lib/js/string.rs b/src/lib/js/string.rs index 797b6ecc3c7..b288a67341d 100644 --- a/src/lib/js/string.rs +++ b/src/lib/js/string.rs @@ -822,9 +822,9 @@ mod tests { let realm = Realm::create(); let mut engine = Executor::new(realm); let init = r#" - const hello = new String('Hello, '); - const world = new String('world! '); - const nice = new String('Have a nice day.'); + var hello = new String('Hello, '); + var world = new String('world! '); + var nice = new String('Have a nice day.'); "#; forward(&mut engine, init); let _a = forward(&mut engine, "hello.concat(world, nice)"); @@ -841,8 +841,8 @@ mod tests { let realm = Realm::create(); let mut engine = Executor::new(realm); let init = r#" - const hello = new String('Hello'); - const world = String('world'); + var hello = new String('Hello'); + var world = String('world'); "#; forward(&mut engine, init); let hello = forward_val(&mut engine, "hello").unwrap(); @@ -857,9 +857,9 @@ mod tests { let realm = Realm::create(); let mut engine = Executor::new(realm); let init = r#" - const empty = new String(''); - const en = new String('english'); - const zh = new String('中文'); + var empty = new String(''); + var en = new String('english'); + var zh = new String('中文'); "#; forward(&mut engine, init); @@ -885,13 +885,13 @@ mod tests { let realm = Realm::create(); let mut engine = Executor::new(realm); let init = r#" - const empty = new String(''); - const en = new String('english'); - const zh = new String('中文'); + var empty = new String(''); + var en = new String('english'); + var zh = new String('中文'); - const emptyLiteral = ''; - const enLiteral = 'english'; - const zhLiteral = '中文'; + var emptyLiteral = ''; + var enLiteral = 'english'; + var zhLiteral = '中文'; "#; forward(&mut engine, init); let pass = String::from("true"); @@ -909,13 +909,13 @@ mod tests { let realm = Realm::create(); let mut engine = Executor::new(realm); let init = r#" - const empty = new String(''); - const en = new String('english'); - const zh = new String('中文'); + var empty = new String(''); + var en = new String('english'); + var zh = new String('中文'); - const emptyLiteral = ''; - const enLiteral = 'english'; - const zhLiteral = '中文'; + var emptyLiteral = ''; + var enLiteral = 'english'; + var zhLiteral = '中文'; "#; forward(&mut engine, init); let pass = String::from("true"); @@ -952,7 +952,7 @@ mod tests { forward( &mut engine, - "const groupMatches = 'test1test2'.matchAll(/t(e)(st(\\d?))/g)", + "var groupMatches = 'test1test2'.matchAll(/t(e)(st(\\d?))/g)", ); assert_eq!( forward(&mut engine, "groupMatches.length"), @@ -984,9 +984,9 @@ mod tests { ); let init = r#" - const regexp = RegExp('foo[a-z]*','g'); - const str = 'table football, foosball'; - const matches = str.matchAll(regexp); + var regexp = RegExp('foo[a-z]*','g'); + var str = 'table football, foosball'; + var matches = str.matchAll(regexp); "#; forward(&mut engine, init); assert_eq!(