[Merged by Bors] - Refactor the environment for runtime performance

boa/benches/full.rs

@@ -47,7 +47,7 @@ macro_rules! full_benchmarks {
                     static CODE: &str = include_str!(concat!("bench_scripts/", stringify!($name), ".js"));
                     let mut context = Context::default();
                     let statement_list = context.parse(CODE).expect("parsing failed");
-                    let code_block = context.compile(&statement_list);
+                    let code_block = context.compile(&statement_list).unwrap();
                     c.bench_function(concat!($id, " (Execution)"), move |b| {
                         b.iter(|| {
                             context.execute(black_box(code_block.clone())).unwrap()

boa/src/builtins/function/arguments.rs

@@ -1,14 +1,15 @@
 use crate::{
     builtins::Array,
-    environment::lexical_environment::Environment,
+    environments::DeclarativeEnvironment,
     gc::{Finalize, Trace},
     object::{FunctionBuilder, JsObject, ObjectData},
-    property::PropertyDescriptor,
+    property::{PropertyDescriptor, PropertyKey},
     symbol::{self, WellKnownSymbols},
     syntax::ast::node::FormalParameter,
     Context, JsValue,
 };
-use rustc_hash::FxHashSet;
+use gc::Gc;
+use rustc_hash::FxHashMap;
 
 #[derive(Debug, Clone, Trace, Finalize)]
 pub struct MappedArguments(JsObject);
@@ -57,15 +58,15 @@ impl Arguments {
                 .configurable(true),
             context,
         )
-        .expect("DefinePropertyOrThrow must not fail per the spec");
+        .expect("Defining new own properties for a new ordinary object cannot fail");
 
         // 5. Let index be 0.
         // 6. Repeat, while index < len,
         for (index, value) in arguments_list.iter().cloned().enumerate() {
             // a. Let val be argumentsList[index].
             // b. Perform ! CreateDataPropertyOrThrow(obj, ! ToString(𝔽(index)), val).
             obj.create_data_property_or_throw(index, value, context)
-                .expect("CreateDataPropertyOrThrow must not fail per the spec");
+                .expect("Defining new own properties for a new ordinary object cannot fail");
 
             // c. Set index to index + 1.
         }
@@ -82,7 +83,7 @@ impl Arguments {
                 .configurable(true),
             context,
         )
-        .expect("DefinePropertyOrThrow must not fail per the spec");
+        .expect("Defining new own properties for a new ordinary object cannot fail");
 
         let throw_type_error = context.intrinsics().throw_type_error();
 
@@ -98,7 +99,7 @@ impl Arguments {
                 .configurable(false),
             context,
         )
-        .expect("DefinePropertyOrThrow must not fail per the spec");
+        .expect("Defining new own properties for a new ordinary object cannot fail");
 
         // 9. Return obj.
         obj
@@ -111,7 +112,7 @@ impl Arguments {
         func: &JsObject,
         formals: &[FormalParameter],
         arguments_list: &[JsValue],
-        env: &Environment,
+        env: &Gc<DeclarativeEnvironment>,
         context: &mut Context,
     ) -> JsObject {
         // 1. Assert: formals does not contain a rest parameter, any binding patterns, or any initializers.
@@ -142,7 +143,7 @@ impl Arguments {
             // a. Let val be argumentsList[index].
             // b. Perform ! CreateDataPropertyOrThrow(obj, ! ToString(𝔽(index)), val).
             obj.create_data_property_or_throw(index, val, context)
-                .expect("CreateDataPropertyOrThrow must not fail per the spec");
+                .expect("Defining new own properties for a new ordinary object cannot fail");
             // c. Set index to index + 1.
         }
 
@@ -157,90 +158,105 @@ impl Arguments {
                 .configurable(true),
             context,
         )
-        .expect("DefinePropertyOrThrow must not fail per the spec");
+        .expect("Defining new own properties for a new ordinary object cannot fail");
 
-        // 17. Let mappedNames be a new empty List.
-        // using a set to optimize `contains`
-        let mut mapped_names = FxHashSet::default();
+        // The section 17-19 differs from the spec, due to the way the runtime environments work.
+        //
+        // This section creates getters and setters for all mapped arguments.
+        // Getting and setting values on the `arguments` object will actually access the bindings in the environment:
+        // ```
+        // function f(a) {console.log(a); arguments[0] = 1; console.log(a)};
+        // f(0) // 0, 1
+        // ```
+        //
+        // The spec assumes, that identifiers are used at runtime to reference bindings in the environment.
+        // We use indices to access environment bindings at runtime.
+        // To map to function parameters to binding indices, we use the fact, that bindings in a
+        // function environment start with all of the arguments in order:
+        // `function f (a,b,c)`
+        // | binding index | `arguments` property key | identifier |
+        // | 0             | 0                        | a          |
+        // | 1             | 1                        | b          |
+        // | 2             | 2                        | c          |
+        //
+        // Notice that the binding index does not correspond to the argument index:
+        // `function f (a,a,b)` => binding indices 0 (a), 1 (b), 2 (c)
+        // | binding index | `arguments` property key | identifier |
+        // | -             | 0                        | -          |
+        // | 0             | 1                        | a          |
+        // | 1             | 2                        | b          |
+        // While the `arguments` object contains all arguments, they must not be all bound.
+        // In the case of duplicate parameter names, the last one is bound as the environment binding.
+        //
+        // The following logic implements the steps 17-19 adjusted for our environment structure.
 
-        // 12. Let parameterNames be the BoundNames of formals.
-        // 13. Let numberOfParameters be the number of elements in parameterNames.
-        // 18. Set index to numberOfParameters - 1.
-        // 19. Repeat, while index ≥ 0,
-        // a. Let name be parameterNames[index].
-
-        for (index, parameter_name_vec) in
-            formals.iter().map(FormalParameter::names).enumerate().rev()
-        {
-            for parameter_name in parameter_name_vec.iter().copied() {
-                // b. If name is not an element of mappedNames, then
-                if !mapped_names.contains(&parameter_name) {
-                    // i. Add name as an element of the list mappedNames.
-                    mapped_names.insert(parameter_name);
-                    // ii. If index < len, then
-                    if index < len {
-                        // 1. Let g be MakeArgGetter(name, env).
-                        // https://tc39.es/ecma262/#sec-makearggetter
-                        let g = {
-                            // 2. Let getter be ! CreateBuiltinFunction(getterClosure, 0, "", « »).
-                            // 3. NOTE: getter is never directly accessible to ECMAScript code.
-                            // 4. Return getter.
-                            FunctionBuilder::closure_with_captures(
-                                context,
-                                // 1. Let getterClosure be a new Abstract Closure with no parameters that captures
-                                // name and env and performs the following steps when called:
-                                |_, _, captures, context| {
-                                    captures.0.get_binding_value(captures.1, false, context)
-                                },
-                                (env.clone(), parameter_name),
-                            )
-                            .length(0)
-                            .name("")
-                            .build()
-                        };
-                        // 2. Let p be MakeArgSetter(name, env).
-                        // https://tc39.es/ecma262/#sec-makeargsetter
-                        let p = {
-                            // 2. Let setter be ! CreateBuiltinFunction(setterClosure, 1, "", « »).
-                            // 3. NOTE: setter is never directly accessible to ECMAScript code.
-                            // 4. Return setter.
-                            FunctionBuilder::closure_with_captures(
-                                context,
-                                // 1. Let setterClosure be a new Abstract Closure with parameters (value) that captures
-                                // name and env and performs the following steps when called:
-                                |_, args, captures, context| {
-                                    let value = args.get(0).cloned().unwrap_or_default();
-                                    // a. Return env.SetMutableBinding(name, value, false).
-                                    captures
-                                        .0
-                                        .set_mutable_binding(captures.1, value, false, context)
-                                        .map(|_| JsValue::Undefined)
-                                    // Ok(JsValue::Undefined)
-                                },
-                                (env.clone(), parameter_name),
-                            )
-                            .length(1)
-                            .name("")
-                            .build()
-                        };
-
-                        // 3. Perform map.[[DefineOwnProperty]](! ToString(𝔽(index)), PropertyDescriptor {
-                        // [[Set]]: p, [[Get]]: g, [[Enumerable]]: false, [[Configurable]]: true }).
-                        map.__define_own_property__(
-                            index.into(),
-                            PropertyDescriptor::builder()
-                                .set(p)
-                                .get(g)
-                                .enumerable(false)
-                                .configurable(true)
-                                .build(),
-                            context,
-                        )
-                        .expect("[[DefineOwnProperty]] must not fail per the spec");
-                    }
+        let mut bindings = FxHashMap::default();
+        let mut property_index = 0;
+        'outer: for formal in formals {
+            for name in formal.names() {
+                if property_index >= len {
+                    break 'outer;
                 }
+                let binding_index = bindings.len() + 1;
+                let entry = bindings
+                    .entry(name)
+                    .or_insert((binding_index, property_index));
+                entry.1 = property_index;
+                property_index += 1;
             }
         }
+        for (binding_index, property_index) in bindings.values() {
+            // 19.b.ii.1. Let g be MakeArgGetter(name, env).
+            // https://tc39.es/ecma262/#sec-makearggetter
+            let g = {
+                // 2. Let getter be ! CreateBuiltinFunction(getterClosure, 0, "", « »).
+                // 3. NOTE: getter is never directly accessible to ECMAScript code.
+                // 4. Return getter.
+                FunctionBuilder::closure_with_captures(
+                    context,
+                    // 1. Let getterClosure be a new Abstract Closure with no parameters that captures
+                    // name and env and performs the following steps when called:
+                    |_, _, captures, _| Ok(captures.0.get(captures.1)),
+                    (env.clone(), *binding_index),
+                )
+                .length(0)
+                .build()
+            };
+            // 19.b.ii.2. Let p be MakeArgSetter(name, env).
+            // https://tc39.es/ecma262/#sec-makeargsetter
+            let p = {
+                // 2. Let setter be ! CreateBuiltinFunction(setterClosure, 1, "", « »).
+                // 3. NOTE: setter is never directly accessible to ECMAScript code.
+                // 4. Return setter.
+                FunctionBuilder::closure_with_captures(
+                    context,
+                    // 1. Let setterClosure be a new Abstract Closure with parameters (value) that captures
+                    // name and env and performs the following steps when called:
+                    |_, args, captures, _| {
+                        let value = args.get(0).cloned().unwrap_or_default();
+                        captures.0.set(captures.1, value);
+                        Ok(JsValue::undefined())
+                    },
+                    (env.clone(), *binding_index),
+                )
+                .length(1)
+                .build()
+            };
+
+            // 19.b.ii.3. Perform map.[[DefineOwnProperty]](! ToString(𝔽(index)), PropertyDescriptor {
+            // [[Set]]: p, [[Get]]: g, [[Enumerable]]: false, [[Configurable]]: true }).
+            map.__define_own_property__(
+                PropertyKey::from(*property_index),
+                PropertyDescriptor::builder()
+                    .set(p)
+                    .get(g)
+                    .enumerable(false)
+                    .configurable(true)
+                    .build(),
+                context,
+            )
+            .expect("Defining new own properties for a new ordinary object cannot fail");
+        }
 
         // 20. Perform ! DefinePropertyOrThrow(obj, @@iterator, PropertyDescriptor {
         // [[Value]]: %Array.prototype.values%, [[Writable]]: true, [[Enumerable]]: false,
@@ -254,7 +270,7 @@ impl Arguments {
                 .configurable(true),
             context,
         )
-        .expect("DefinePropertyOrThrow must not fail per the spec");
+        .expect("Defining new own properties for a new ordinary object cannot fail");
 
         // 21. Perform ! DefinePropertyOrThrow(obj, "callee", PropertyDescriptor {
         // [[Value]]: func, [[Writable]]: true, [[Enumerable]]: false, [[Configurable]]: true }).
@@ -267,7 +283,7 @@ impl Arguments {
                 .configurable(true),
             context,
         )
-        .expect("DefinePropertyOrThrow must not fail per the spec");
+        .expect("Defining new own properties for a new ordinary object cannot fail");
 
         // 22. Return obj.
         obj

boa/src/builtins/function/mod.rs

@@ -11,11 +11,10 @@
 //! [spec]: https://tc39.es/ecma262/#sec-function-objects
 //! [mdn]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Function
 
-use super::JsArgs;
 use crate::{
-    builtins::BuiltIn,
+    builtins::{BuiltIn, JsArgs},
     context::StandardObjects,
-    environment::lexical_environment::Environment,
+    environments::DeclarativeEnvironmentStack,
     gc::{self, Finalize, Gc, Trace},
     object::{
         internal_methods::get_prototype_from_constructor, JsObject, NativeObject, Object,
@@ -177,7 +176,7 @@ pub enum Function {
     },
     VmOrdinary {
         code: Gc<crate::vm::CodeBlock>,
-        environment: Environment,
+        environments: DeclarativeEnvironmentStack,
     },
 }
 

boa/src/builtins/global_this/mod.rs

@@ -28,6 +28,6 @@ impl BuiltIn for GlobalThis {
     fn init(context: &mut Context) -> JsValue {
         let _timer = BoaProfiler::global().start_event(Self::NAME, "init");
 
-        context.global_object().into()
+        context.global_object().clone().into()
     }
 }

boa/src/builtins/mod.rs

@@ -101,11 +101,11 @@ fn init_builtin<B: BuiltIn>(context: &mut Context) {
         .value(value)
         .writable(B::ATTRIBUTE.writable())
         .enumerable(B::ATTRIBUTE.enumerable())
-        .configurable(B::ATTRIBUTE.configurable());
+        .configurable(B::ATTRIBUTE.configurable())
+        .build();
     context
-        .global_object()
-        .borrow_mut()
-        .insert(B::NAME, property);
+        .global_bindings_mut()
+        .insert(B::NAME.into(), property);
 }
 
 /// Initializes built-in objects and functions

boa/src/builtins/number/mod.rs

@@ -13,13 +13,12 @@
 //! [spec]: https://tc39.es/ecma262/#sec-number-object
 //! [mdn]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Number
 
-use super::string::is_trimmable_whitespace;
-use super::JsArgs;
-use crate::context::StandardObjects;
-use crate::object::JsObject;
 use crate::{
-    builtins::{function::make_builtin_fn, BuiltIn},
-    object::{internal_methods::get_prototype_from_constructor, ConstructorBuilder, ObjectData},
+    builtins::{string::is_trimmable_whitespace, BuiltIn, JsArgs},
+    context::StandardObjects,
+    object::{
+        internal_methods::get_prototype_from_constructor, ConstructorBuilder, JsObject, ObjectData,
+    },
     property::Attribute,
     value::{AbstractRelation, IntegerOrInfinity, JsValue},
     BoaProfiler, Context, JsResult,
@@ -39,12 +38,6 @@ const BUF_SIZE: usize = 2200;
 #[derive(Debug, Clone, Copy)]
 pub(crate) struct Number;
 
-/// Maximum number of arguments expected to the builtin parseInt() function.
-const PARSE_INT_MAX_ARG_COUNT: usize = 2;
-
-/// Maximum number of arguments expected to the builtin parseFloat() function.
-const PARSE_FLOAT_MAX_ARG_COUNT: usize = 1;
-
 impl BuiltIn for Number {
     const NAME: &'static str = "Number";
 
@@ -83,23 +76,10 @@ impl BuiltIn for Number {
         .static_method(Self::number_is_integer, "isInteger", 1)
         .build();
 
-        let global = context.global_object();
-        make_builtin_fn(
-            Self::parse_int,
-            "parseInt",
-            &global,
-            PARSE_INT_MAX_ARG_COUNT,
-            context,
-        );
-        make_builtin_fn(
-            Self::parse_float,
-            "parseFloat",
-            &global,
-            PARSE_FLOAT_MAX_ARG_COUNT,
-            context,
-        );
-        make_builtin_fn(Self::global_is_finite, "isFinite", &global, 1, context);
-        make_builtin_fn(Self::global_is_nan, "isNaN", &global, 1, context);
+        context.register_global_builtin_function("parseInt", 2, Self::parse_int);
+        context.register_global_builtin_function("parseFloat", 1, Self::parse_float);
+        context.register_global_builtin_function("isFinite", 1, Self::global_is_finite);
+        context.register_global_builtin_function("isNaN", 1, Self::global_is_nan);
 
         number_object.into()
     }