Set function names in object literal methods (#2460)

This Pull Request changes the following:

- Implement `SetFunctionName` opcode based on [`SetFunctionName`](https://tc39.es/ecma262/#sec-setfunctionname)

boa_ast/src/expression/literal/object.rs

@@ -3,6 +3,7 @@
 use crate::{
     block_to_string,
     expression::{operator::assign::AssignTarget, Expression, RESERVED_IDENTIFIERS_STRICT},
+    function::Function,
     join_nodes,
     pattern::{ObjectPattern, ObjectPatternElement},
     property::{MethodDefinition, PropertyDefinition, PropertyName},
@@ -211,6 +212,12 @@ impl ToIndentedString for ObjectLiteral {
                     format!("{indentation}{},\n", interner.resolve_expect(ident.sym()))
                 }
                 PropertyDefinition::Property(key, value) => {
+                    let value = if let Expression::Function(f) = value {
+                        Function::new(None, f.parameters().clone(), f.body().clone()).into()
+                    } else {
+                        value.clone()
+                    };
+
                     format!(
                         "{indentation}{}: {},\n",
                         key.to_interned_string(interner),

boa_ast/src/expression/mod.rs

@@ -195,6 +195,27 @@ impl Expression {
             Self::FormalParameterList(_) => unreachable!(),
         }
     }
+
+    /// Returns if the expression is a function definition according to the spec.
+    ///
+    /// More information:
+    ///  - [ECMAScript reference][spec]
+    ///
+    /// [spec]: https://tc39.es/ecma262/#sec-static-semantics-isfunctiondefinition
+    #[must_use]
+    #[inline]
+    pub const fn is_function_definition(&self) -> bool {
+        matches!(
+            self,
+            Self::ArrowFunction(_)
+                | Self::AsyncArrowFunction(_)
+                | Self::Function(_)
+                | Self::Generator(_)
+                | Self::AsyncGenerator(_)
+                | Self::AsyncFunction(_)
+                | Self::Class(_)
+        )
+    }
 }
 
 impl From<Expression> for Statement {

boa_ast/src/property.rs

@@ -356,3 +356,15 @@ pub enum ClassElementName {
     /// A private property.
     PrivateIdentifier(Sym),
 }
+
+impl ClassElementName {
+    /// Returns the property name if it exists.
+    #[must_use]
+    pub const fn literal(&self) -> Option<Sym> {
+        if let Self::PropertyName(name) = self {
+            name.literal()
+        } else {
+            None
+        }
+    }
+}

boa_engine/src/bytecompiler/mod.rs

@@ -1225,12 +1225,18 @@ impl<'b> ByteCompiler<'b> {
                             PropertyName::Computed(name_node) => {
                                 self.compile_expr(name_node, true)?;
                                 self.emit_opcode(Opcode::ToPropertyKey);
+                                if expr.is_function_definition() {
+                                    self.emit_opcode(Opcode::Dup);
+                                    self.compile_expr(expr, true)?;
+                                    self.emit_opcode(Opcode::SetFunctionName);
+                                    self.emit_u8(0);
+                                } else {
                                     self.compile_expr(expr, true)?;
+                                }
                                 self.emit_opcode(Opcode::DefineOwnPropertyByValue);
                             }
                         },
                         PropertyDefinition::MethodDefinition(name, kind) => match kind {
-                            // TODO: set function name for getter and setters
                             MethodDefinition::Get(expr) => match name {
                                 PropertyName::Literal(name) => {
                                     self.function(expr.into(), NodeKind::Expression, true)?;
@@ -1240,11 +1246,13 @@ impl<'b> ByteCompiler<'b> {
                                 PropertyName::Computed(name_node) => {
                                     self.compile_expr(name_node, true)?;
                                     self.emit_opcode(Opcode::ToPropertyKey);
+                                    self.emit_opcode(Opcode::Dup);
                                     self.function(expr.into(), NodeKind::Expression, true)?;
+                                    self.emit_opcode(Opcode::SetFunctionName);
+                                    self.emit_u8(1);
                                     self.emit_opcode(Opcode::SetPropertyGetterByValue);
                                 }
                             },
-                            // TODO: set function name for getter and setters
                             MethodDefinition::Set(expr) => match name {
                                 PropertyName::Literal(name) => {
                                     self.function(expr.into(), NodeKind::Expression, true)?;
@@ -1254,7 +1262,10 @@ impl<'b> ByteCompiler<'b> {
                                 PropertyName::Computed(name_node) => {
                                     self.compile_expr(name_node, true)?;
                                     self.emit_opcode(Opcode::ToPropertyKey);
+                                    self.emit_opcode(Opcode::Dup);
                                     self.function(expr.into(), NodeKind::Expression, true)?;
+                                    self.emit_opcode(Opcode::SetFunctionName);
+                                    self.emit_u8(2);
                                     self.emit_opcode(Opcode::SetPropertySetterByValue);
                                 }
                             },
@@ -1267,7 +1278,10 @@ impl<'b> ByteCompiler<'b> {
                                 PropertyName::Computed(name_node) => {
                                     self.compile_expr(name_node, true)?;
                                     self.emit_opcode(Opcode::ToPropertyKey);
+                                    self.emit_opcode(Opcode::Dup);
                                     self.function(expr.into(), NodeKind::Expression, true)?;
+                                    self.emit_opcode(Opcode::SetFunctionName);
+                                    self.emit_u8(0);
                                     self.emit_opcode(Opcode::DefineOwnPropertyByValue);
                                 }
                             },
@@ -1280,7 +1294,10 @@ impl<'b> ByteCompiler<'b> {
                                 PropertyName::Computed(name_node) => {
                                     self.compile_expr(name_node, true)?;
                                     self.emit_opcode(Opcode::ToPropertyKey);
+                                    self.emit_opcode(Opcode::Dup);
                                     self.function(expr.into(), NodeKind::Expression, true)?;
+                                    self.emit_opcode(Opcode::SetFunctionName);
+                                    self.emit_u8(0);
                                     self.emit_opcode(Opcode::DefineOwnPropertyByValue);
                                 }
                             },
@@ -1293,7 +1310,10 @@ impl<'b> ByteCompiler<'b> {
                                 PropertyName::Computed(name_node) => {
                                     self.compile_expr(name_node, true)?;
                                     self.emit_opcode(Opcode::ToPropertyKey);
+                                    self.emit_opcode(Opcode::Dup);
                                     self.function(expr.into(), NodeKind::Expression, true)?;
+                                    self.emit_opcode(Opcode::SetFunctionName);
+                                    self.emit_u8(0);
                                     self.emit_opcode(Opcode::DefineOwnPropertyByValue);
                                 }
                             },
@@ -1306,7 +1326,10 @@ impl<'b> ByteCompiler<'b> {
                                 PropertyName::Computed(name_node) => {
                                     self.compile_expr(name_node, true)?;
                                     self.emit_opcode(Opcode::ToPropertyKey);
+                                    self.emit_opcode(Opcode::Dup);
                                     self.function(expr.into(), NodeKind::Expression, true)?;
+                                    self.emit_opcode(Opcode::SetFunctionName);
+                                    self.emit_u8(0);
                                     self.emit_opcode(Opcode::DefineOwnPropertyByValue);
                                 }
                             },

boa_engine/src/vm/code_block.rs

@@ -177,6 +177,17 @@ impl CodeBlock {
         let opcode: Opcode = self.code[*pc].try_into().expect("invalid opcode");
         *pc += size_of::<Opcode>();
         match opcode {
+            Opcode::SetFunctionName => {
+                let operand = self.read::<u8>(*pc);
+                *pc += size_of::<u8>();
+                match operand {
+                    0 => "prefix: none",
+                    1 => "prefix: get",
+                    2 => "prefix: set",
+                    _ => unreachable!(),
+                }
+                .to_owned()
+            }
             Opcode::RotateLeft | Opcode::RotateRight => {
                 let result = self.read::<u8>(*pc).to_string();
                 *pc += size_of::<u8>();

boa_engine/src/vm/flowgraph/mod.rs

@@ -45,6 +45,21 @@ impl CodeBlock {
 
             pc += size_of::<Opcode>();
             match opcode {
+                Opcode::SetFunctionName => {
+                    let operand = self.read::<u8>(pc);
+                    pc += size_of::<u8>();
+                    let label = format!(
+                        "{opcode_str} {}",
+                        match operand {
+                            0 => "prefix: none",
+                            1 => "prefix: get",
+                            2 => "prefix: set",
+                            _ => unreachable!(),
+                        }
+                    );
+                    graph.add_node(previous_pc, NodeShape::None, label.into(), Color::None);
+                    graph.add_edge(previous_pc, pc, None, Color::None, EdgeStyle::Line);
+                }
                 Opcode::RotateLeft | Opcode::RotateRight => {
                     pc += size_of::<u8>();
                     graph.add_node(previous_pc, NodeShape::None, label.into(), Color::None);

boa_engine/src/vm/opcode/mod.rs

@@ -744,6 +744,22 @@ generate_impl! {
         /// Stack: object, value **=>** value
         SetPropertyByName,
 
+        /// Sets the name of a function object.
+        ///
+        /// This operation is corresponds to the `SetFunctionName` abstract operation in the [spec].
+        ///
+        ///  The prefix operand is mapped as follows:
+        /// * 0 -> no prefix
+        /// * 1 -> "get "
+        /// * 2 -> "set "
+        ///
+        ///  Operands: prefix: `u8`
+        ///
+        /// Stack: name, function **=>** function
+        ///
+        /// [spec]: https://tc39.es/ecma262/#sec-setfunctionname
+        SetFunctionName,
+
         /// Defines a own property of an object by name.
         ///
         /// Operands: name_index: `u32`

boa_engine/src/vm/opcode/set/property.rs

@@ -3,6 +3,7 @@ use crate::{
     vm::{opcode::Operation, ShouldExit},
     Context, JsResult, JsString,
 };
+use boa_macros::utf16;
 
 /// `SetPropertyByName` implements the Opcode Operation for `Opcode::SetPropertyByName`
 ///
@@ -220,3 +221,55 @@ impl Operation for SetPropertySetterByValue {
         Ok(ShouldExit::False)
     }
 }
+
+/// `SetFunctionName` implements the Opcode Operation for `Opcode::SetFunctionName`
+///
+/// Operation:
+///  - Sets the name of a function object.
+#[derive(Debug, Clone, Copy)]
+pub(crate) struct SetFunctionName;
+
+impl Operation for SetFunctionName {
+    const NAME: &'static str = "SetFunctionName";
+    const INSTRUCTION: &'static str = "INST - SetFunctionName";
+
+    fn execute(context: &mut Context) -> JsResult<ShouldExit> {
+        let prefix = context.vm.read::<u8>();
+
+        let function = context.vm.pop();
+        let name = context.vm.pop();
+
+        let function_object = function.as_object().expect("function is not an object");
+
+        let name = if let Some(symbol) = name.as_symbol() {
+            if let Some(name) = symbol.description() {
+                JsString::concat_array(&[utf16!("["), &name, utf16!("]")])
+            } else {
+                JsString::from("")
+            }
+        } else {
+            name.as_string().expect("name is not a string").clone()
+        };
+
+        let name = match prefix {
+            0 => name,
+            1 => JsString::concat(utf16!("get "), &name),
+            2 => JsString::concat(utf16!("set "), &name),
+            _ => unreachable!(),
+        };
+
+        let desc = PropertyDescriptor::builder()
+            .value(name)
+            .writable(false)
+            .enumerable(false)
+            .configurable(true)
+            .build();
+
+        function_object
+            .__define_own_property__("name".into(), desc, context)
+            .expect("msg");
+
+        context.vm.stack.push(function);
+        Ok(ShouldExit::False)
+    }
+}

boa_interner/src/sym.rs

@@ -107,6 +107,9 @@ impl Sym {
     /// Symbol for the `"__proto__"` string.
     pub const __PROTO__: Self = unsafe { Self::new_unchecked(29) };
 
+    /// Symbol for the `"name"` string.
+    pub const NAME: Self = unsafe { Self::new_unchecked(30) };
+
     /// Creates a new [`Sym`] from the provided `value`, or returns `None` if `index` is zero.
     #[inline]
     pub(super) fn new(value: usize) -> Option<Self> {
@@ -199,4 +202,5 @@ create_static_strings! {
     "of",
     "target",
     "__proto__",
+    "name",
 }

boa_parser/src/parser/expression/primary/function_expression/mod.rs

@@ -66,16 +66,11 @@ where
             | TokenKind::Keyword((
                 Keyword::Yield | Keyword::Await | Keyword::Async | Keyword::Of,
                 _,
-            )) => (
-                Some(BindingIdentifier::new(false, false).parse(cursor, interner)?),
-                true,
-            ),
-            _ => (self.name, false),
-        };
+            )) => {
+                let name = BindingIdentifier::new(false, false).parse(cursor, interner)?;
 
                 // Early Error: If BindingIdentifier is present and the source code matching BindingIdentifier is strict mode code,
                 // it is a Syntax Error if the StringValue of BindingIdentifier is "eval" or "arguments".
-        if let Some(name) = name {
                 if cursor.strict_mode() && [Sym::EVAL, Sym::ARGUMENTS].contains(&name.sym()) {
                     return Err(Error::lex(LexError::Syntax(
                         "Unexpected eval or arguments in strict mode".into(),
@@ -84,7 +79,11 @@ where
                             .map_or_else(|| Position::new(1, 1), |token| token.span().end()),
                     )));
                 }
+
+                (Some(name), true)
             }
+            _ => (self.name, false),
+        };
 
         let params_start_position = cursor
             .expect(Punctuator::OpenParen, "function expression", interner)?

boa_parser/src/parser/expression/primary/object_initializer/mod.rs

@@ -292,8 +292,14 @@ where
 
         //  PropertyName[?Yield, ?Await] : AssignmentExpression[+In, ?Yield, ?Await]
         if cursor.next_if(Punctuator::Colon, interner)?.is_some() {
-            let value = AssignmentExpression::new(None, true, self.allow_yield, self.allow_await)
+            let name = property_name
+                .literal()
+                .filter(|name| *name != Sym::__PROTO__)
+                .map(Into::into);
+
+            let value = AssignmentExpression::new(name, true, self.allow_yield, self.allow_await)
                 .parse(cursor, interner)?;
+
             return Ok(property::PropertyDefinition::Property(property_name, value));
         }
 
@@ -330,8 +336,16 @@ where
                     "get method definition",
                     interner,
                 )?;
+
+                let name = property_name.literal().map(|name| {
+                    let s = interner.resolve_expect(name).utf16();
+                    interner
+                        .get_or_intern([utf16!("get "), s].concat().as_slice())
+                        .into()
+                });
+
                 let method = MethodDefinition::Get(Function::new(
-                    None,
+                    name,
                     FormalParameterList::default(),
                     body,
                 ));
@@ -380,8 +394,9 @@ where
                     interner,
                 )?;
 
-                // Early Error: It is a Syntax Error if FunctionBodyContainsUseStrict of FunctionBody is true
+                // It is a Syntax Error if FunctionBodyContainsUseStrict of FunctionBody is true
                 // and IsSimpleParameterList of PropertySetParameterList is false.
+                // https://tc39.es/ecma262/#sec-method-definitions-static-semantics-early-errors
                 if body.strict() && !parameters.is_simple() {
                     return Err(Error::lex(LexError::Syntax(
                         "Illegal 'use strict' directive in function with non-simple parameter list"
@@ -390,9 +405,26 @@ where
                     )));
                 }
 
-                let method = MethodDefinition::Set(Function::new(None, parameters, body));
+                // It is a Syntax Error if any element of the BoundNames of PropertySetParameterList also
+                // occurs in the LexicallyDeclaredNames of FunctionBody.
+                // https://tc39.es/ecma262/#sec-method-definitions-static-semantics-early-errors
+                name_in_lexically_declared_names(
+                    &bound_names(&parameters),
+                    &top_level_lexically_declared_names(&body),
+                    params_start_position,
+                )?;
+
+                let name = property_name.literal().map(|name| {
+                    let s = interner.resolve_expect(name).utf16();
+                    interner
+                        .get_or_intern([utf16!("set "), s].concat().as_slice())
+                        .into()
+                });
+
+                let method = MethodDefinition::Set(Function::new(name, parameters, body));
 
                 // It is a Syntax Error if HasDirectSuper of MethodDefinition is true.
+                // https://tc39.es/ecma262/#sec-object-initializer-static-semantics-early-errors
                 if has_direct_super(&method) {
                     return Err(Error::general("invalid super usage", params_start_position));
                 }
@@ -457,7 +489,11 @@ where
                     params_start_position,
                 )?;
 
-                let method = MethodDefinition::Ordinary(Function::new(None, params, body));
+                let method = MethodDefinition::Ordinary(Function::new(
+                    property_name.literal().map(Into::into),
+                    params,
+                    body,
+                ));
 
                 // It is a Syntax Error if HasDirectSuper of MethodDefinition is true.
                 if has_direct_super(&method) {
@@ -715,7 +751,12 @@ where
             params_start_position,
         )?;
 
-        let method = MethodDefinition::Generator(Generator::new(None, params, body, false));
+        let method = MethodDefinition::Generator(Generator::new(
+            class_element_name.literal().map(Into::into),
+            params,
+            body,
+            false,
+        ));
 
         if contains(&method, ContainsSymbol::Super) {
             return Err(Error::lex(LexError::Syntax(
@@ -825,8 +866,12 @@ where
             params_start_position,
         )?;
 
-        let method =
-            MethodDefinition::AsyncGenerator(AsyncGenerator::new(None, params, body, false));
+        let method = MethodDefinition::AsyncGenerator(AsyncGenerator::new(
+            name.literal().map(Into::into),
+            params,
+            body,
+            false,
+        ));
 
         if contains(&method, ContainsSymbol::Super) {
             return Err(Error::lex(LexError::Syntax(
@@ -913,7 +958,12 @@ where
             params_start_position,
         )?;
 
-        let method = MethodDefinition::Async(AsyncFunction::new(None, params, body, false));
+        let method = MethodDefinition::Async(AsyncFunction::new(
+            class_element_name.literal().map(Into::into),
+            params,
+            body,
+            false,
+        ));
 
         if contains(&method, ContainsSymbol::Super) {
             return Err(Error::lex(LexError::Syntax(

boa_parser/src/parser/expression/primary/object_initializer/tests.rs

@@ -63,7 +63,7 @@ fn check_object_short_function() {
         PropertyDefinition::MethodDefinition(
             interner.get_or_intern_static("b", utf16!("b")).into(),
             MethodDefinition::Ordinary(Function::new(
-                None,
+                Some(interner.get_or_intern_static("b", utf16!("b")).into()),
                 FormalParameterList::default(),
                 StatementList::default(),
             )),
@@ -112,7 +112,11 @@ fn check_object_short_function_arguments() {
         ),
         PropertyDefinition::MethodDefinition(
             interner.get_or_intern_static("b", utf16!("b")).into(),
-            MethodDefinition::Ordinary(Function::new(None, parameters, StatementList::default())),
+            MethodDefinition::Ordinary(Function::new(
+                Some(interner.get_or_intern_static("b", utf16!("b")).into()),
+                parameters,
+                StatementList::default(),
+            )),
         ),
     ];
 
@@ -147,7 +151,11 @@ fn check_object_getter() {
         PropertyDefinition::MethodDefinition(
             interner.get_or_intern_static("b", utf16!("b")).into(),
             MethodDefinition::Get(Function::new(
-                None,
+                Some(
+                    interner
+                        .get_or_intern_static("get b", utf16!("get b"))
+                        .into(),
+                ),
                 FormalParameterList::default(),
                 StatementList::default(),
             )),
@@ -195,7 +203,15 @@ fn check_object_setter() {
         ),
         PropertyDefinition::MethodDefinition(
             interner.get_or_intern_static("b", utf16!("b")).into(),
-            MethodDefinition::Set(Function::new(None, params, StatementList::default())),
+            MethodDefinition::Set(Function::new(
+                Some(
+                    interner
+                        .get_or_intern_static("set b", utf16!("set b"))
+                        .into(),
+                ),
+                params,
+                StatementList::default(),
+            )),
         ),
     ];
 
@@ -225,7 +241,7 @@ fn check_object_short_function_get() {
     let object_properties = vec![PropertyDefinition::MethodDefinition(
         interner.get_or_intern_static("get", utf16!("get")).into(),
         MethodDefinition::Ordinary(Function::new(
-            None,
+            Some(interner.get_or_intern_static("get", utf16!("get")).into()),
             FormalParameterList::default(),
             StatementList::default(),
         )),
@@ -256,7 +272,7 @@ fn check_object_short_function_set() {
     let object_properties = vec![PropertyDefinition::MethodDefinition(
         interner.get_or_intern_static("set", utf16!("set")).into(),
         MethodDefinition::Ordinary(Function::new(
-            None,
+            Some(interner.get_or_intern_static("set", utf16!("set")).into()),
             FormalParameterList::default(),
             StatementList::default(),
         )),
@@ -404,7 +420,7 @@ fn check_async_method() {
     let object_properties = vec![PropertyDefinition::MethodDefinition(
         interner.get_or_intern_static("dive", utf16!("dive")).into(),
         MethodDefinition::Async(AsyncFunction::new(
-            None,
+            Some(interner.get_or_intern_static("dive", utf16!("dive")).into()),
             FormalParameterList::default(),
             StatementList::default(),
             false,
@@ -438,7 +454,11 @@ fn check_async_generator_method() {
             .get_or_intern_static("vroom", utf16!("vroom"))
             .into(),
         MethodDefinition::AsyncGenerator(AsyncGenerator::new(
-            None,
+            Some(
+                interner
+                    .get_or_intern_static("vroom", utf16!("vroom"))
+                    .into(),
+            ),
             FormalParameterList::default(),
             StatementList::default(),
             false,
@@ -492,7 +512,11 @@ fn check_async_ordinary_method() {
     let object_properties = vec![PropertyDefinition::MethodDefinition(
         PropertyName::Literal(interner.get_or_intern_static("async", utf16!("async"))),
         MethodDefinition::Ordinary(Function::new(
-            None,
+            Some(
+                interner
+                    .get_or_intern_static("async", utf16!("async"))
+                    .into(),
+            ),
             FormalParameterList::default(),
             StatementList::default(),
         )),