Allow class expressions without identifier (#2464)

This Pull Request changes the following: - Remove false early error when a class expression was missing a binding identifier. - Simplify/fix environment truncation on function returns. The new failed tests where false positives before that will be fixed in another PR.
boa-dev · Nov 29, 2022 · b9f8841 · b9f8841
1 parent b3ab499
commit b9f8841
Show file tree

Hide file tree

Showing 2 changed files with 20 additions and 52 deletions.
diff --git a/boa_engine/src/vm/code_block.rs b/boa_engine/src/vm/code_block.rs
@@ -738,6 +738,7 @@ impl JsObject {
                         .into());
                 }
 
+                let environments_len = environments.len();
                 std::mem::swap(&mut environments, &mut context.realm.environments);
 
                 let lexical_this_mode = code.this_mode == ThisMode::Lexical;
@@ -817,8 +818,6 @@ impl JsObject {
                 }
 
                 let param_count = code.params.as_ref().len();
-                let has_expressions = code.params.has_expressions();
-                let has_binding_identifier = code.has_binding_identifier;
 
                 context.vm.push_frame(CallFrame {
                     code,
@@ -840,20 +839,10 @@ impl JsObject {
                 });
 
                 let result = context.run();
-                let frame = context.vm.pop_frame().expect("must have frame");
-
-                context.realm.environments.pop();
-                if has_expressions
-                    && frame.pc > frame.code.function_environment_push_location as usize
-                {
-                    context.realm.environments.pop();
-                }
-
-                if has_binding_identifier {
-                    context.realm.environments.pop();
-                }
+                context.vm.pop_frame().expect("must have frame");
 
                 std::mem::swap(&mut environments, &mut context.realm.environments);
+                environments.truncate(environments_len);
 
                 let (result, _) = result?;
                 Ok(result)
@@ -869,6 +858,7 @@ impl JsObject {
                 let promise = promise_capability.promise().clone();
                 drop(object);
 
+                let environments_len = environments.len();
                 std::mem::swap(&mut environments, &mut context.realm.environments);
 
                 let lexical_this_mode = code.this_mode == ThisMode::Lexical;
@@ -948,8 +938,6 @@ impl JsObject {
                 }
 
                 let param_count = code.params.as_ref().len();
-                let has_expressions = code.params.has_expressions();
-                let has_binding_identifier = code.has_binding_identifier;
 
                 context.vm.push_frame(CallFrame {
                     code,
@@ -971,20 +959,11 @@ impl JsObject {
                 });
 
                 let _result = context.run();
-                let frame = context.vm.pop_frame().expect("must have frame");
-
-                context.realm.environments.pop();
-                if has_expressions
-                    && frame.pc > frame.code.function_environment_push_location as usize
-                {
-                    context.realm.environments.pop();
-                }
-
-                if has_binding_identifier {
-                    context.realm.environments.pop();
-                }
+                context.vm.pop_frame().expect("must have frame");
 
                 std::mem::swap(&mut environments, &mut context.realm.environments);
+                environments.truncate(environments_len);
+
                 Ok(promise.into())
             }
             Function::Generator {
@@ -1367,6 +1346,7 @@ impl JsObject {
                 let constructor_kind = *constructor_kind;
                 drop(object);
 
+                let environments_len = environments.len();
                 std::mem::swap(&mut environments, &mut context.realm.environments);
 
                 let this = if constructor_kind.is_base() {
@@ -1412,8 +1392,6 @@ impl JsObject {
                     false,
                 );
 
-                let has_expressions = code.params.has_expressions();
-
                 if let Some(binding) = code.arguments_binding {
                     let arguments_obj = if code.strict || !code.params.is_simple() {
                         Arguments::create_unmapped_arguments_object(args, context)
@@ -1478,17 +1456,18 @@ impl JsObject {
 
                 context.vm.pop_frame();
 
-                let mut environment = context.realm.environments.pop();
-                if has_expressions {
-                    environment = context.realm.environments.pop();
-                }
-
-                if has_binding_identifier {
-                    context.realm.environments.pop();
-                }
-
                 std::mem::swap(&mut environments, &mut context.realm.environments);
 
+                let environment = if has_binding_identifier {
+                    environments.truncate(environments_len + 2);
+                    let environment = environments.pop();
+                    environments.pop();
+                    environment
+                } else {
+                    environments.truncate(environments_len + 1);
+                    environments.pop()
+                };
+
                 let (result, _) = result?;
 
                 if let Some(result) = result.as_object() {

diff --git a/boa_parser/src/parser/expression/primary/class_expression/mod.rs b/boa_parser/src/parser/expression/primary/class_expression/mod.rs
@@ -4,10 +4,9 @@ use crate::{
         expression::BindingIdentifier, statement::ClassTail, AllowAwait, AllowYield, Cursor,
         OrAbrupt, ParseResult, TokenParser,
     },
-    Error,
 };
 use boa_ast::{expression::Identifier, function::Class, Keyword};
-use boa_interner::Interner;
+use boa_interner::{Interner, Sym};
 use boa_profiler::Profiler;
 use std::io::Read;
 
@@ -57,17 +56,7 @@ where
                 BindingIdentifier::new(self.allow_yield, self.allow_await)
                     .parse(cursor, interner)?
             }
-            _ => {
-                if let Some(name) = self.name {
-                    name
-                } else {
-                    return Err(Error::unexpected(
-                        token.to_string(interner),
-                        token.span(),
-                        "expected class identifier",
-                    ));
-                }
-            }
+            _ => self.name.unwrap_or_else(|| Sym::EMPTY_STRING.into()),
         };
         cursor.set_strict_mode(strict);