LibJS: Use Identifier to represent name of FunctionNode

Userland/Libraries/LibJS/AST.cpp

@@ -314,7 +314,7 @@ Value FunctionExpression::instantiate_ordinary_function_expression(VM& vm, Depre
         given_name = "";
     auto has_own_name = !name().is_empty();
 
-    auto const& used_name = has_own_name ? name() : given_name;
+    auto const used_name = has_own_name ? name() : given_name.view();
     auto environment = NonnullGCPtr { *vm.running_execution_context().lexical_environment };
     if (has_own_name) {
         VERIFY(environment);
@@ -4805,7 +4805,7 @@ ThrowCompletionOr<void> Program::global_declaration_instantiation(VM& vm, Global
         // b. For each FunctionDeclaration f that is directly contained in the StatementList of a Block, CaseClause, or DefaultClause Contained within script, do
         TRY(for_each_function_hoistable_with_annexB_extension([&](FunctionDeclaration& function_declaration) -> ThrowCompletionOr<void> {
             // i. Let F be StringValue of the BindingIdentifier of f.
-            auto& function_name = function_declaration.name();
+            auto function_name = function_declaration.name();
 
             // ii. If replacing the FunctionDeclaration f with a VariableStatement that has F as a BindingIdentifier would not produce any Early Errors for script, then
             // Note: This step is already performed during parsing and for_each_function_hoistable_with_annexB_extension so this always passes here.

Userland/Libraries/LibJS/AST.h

@@ -663,9 +663,41 @@ struct FunctionParameter {
     bool is_rest { false };
 };
 
+class Identifier final : public Expression {
+public:
+    explicit Identifier(SourceRange source_range, DeprecatedFlyString string)
+        : Expression(source_range)
+        , m_string(move(string))
+    {
+    }
+
+    DeprecatedFlyString const& string() const { return m_string; }
+
+    bool is_local() const { return m_local_variable_index.has_value(); }
+    size_t local_variable_index() const
+    {
+        VERIFY(m_local_variable_index.has_value());
+        return m_local_variable_index.value();
+    }
+    void set_local_variable_index(size_t index) { m_local_variable_index = index; }
+
+    virtual Completion execute(Interpreter&) const override;
+    virtual void dump(int indent) const override;
+    virtual ThrowCompletionOr<Reference> to_reference(Interpreter&) const override;
+    virtual Bytecode::CodeGenerationErrorOr<void> generate_bytecode(Bytecode::Generator&) const override;
+
+private:
+    virtual bool is_identifier() const override { return true; }
+
+    DeprecatedFlyString m_string;
+    mutable EnvironmentCoordinate m_cached_environment_coordinate;
+
+    Optional<size_t> m_local_variable_index;
+};
+
 class FunctionNode {
 public:
-    DeprecatedFlyString const& name() const { return m_name; }
+    StringView name() const { return m_name ? m_name->string().view() : ""sv; }
     DeprecatedString const& source_text() const { return m_source_text; }
     Statement const& body() const { return *m_body; }
     Vector<FunctionParameter> const& parameters() const { return m_parameters; };
@@ -698,7 +730,7 @@ protected:
     void dump(int indent, DeprecatedString const& class_name) const;
 
 private:
-    DeprecatedFlyString m_name;
+    RefPtr<Identifier const> m_name { nullptr };
     DeprecatedString m_source_text;
     NonnullRefPtr<Statement const> m_body;
     Vector<FunctionParameter> const m_parameters;
@@ -1259,38 +1291,6 @@ private:
     DeprecatedString m_flags;
 };
 
-class Identifier final : public Expression {
-public:
-    explicit Identifier(SourceRange source_range, DeprecatedFlyString string)
-        : Expression(source_range)
-        , m_string(move(string))
-    {
-    }
-
-    DeprecatedFlyString const& string() const { return m_string; }
-
-    bool is_local() const { return m_local_variable_index.has_value(); }
-    size_t local_variable_index() const
-    {
-        VERIFY(m_local_variable_index.has_value());
-        return m_local_variable_index.value();
-    }
-    void set_local_variable_index(size_t index) { m_local_variable_index = index; }
-
-    virtual Completion execute(Interpreter&) const override;
-    virtual void dump(int indent) const override;
-    virtual ThrowCompletionOr<Reference> to_reference(Interpreter&) const override;
-    virtual Bytecode::CodeGenerationErrorOr<void> generate_bytecode(Bytecode::Generator&) const override;
-
-private:
-    virtual bool is_identifier() const override { return true; }
-
-    DeprecatedFlyString m_string;
-    mutable EnvironmentCoordinate m_cached_environment_coordinate;
-
-    Optional<size_t> m_local_variable_index;
-};
-
 class PrivateIdentifier final : public Expression {
 public:
     explicit PrivateIdentifier(SourceRange source_range, DeprecatedFlyString string)

Userland/Libraries/LibJS/Runtime/AbstractOperations.cpp

@@ -845,7 +845,7 @@ ThrowCompletionOr<void> eval_declaration_instantiation(VM& vm, Program const& pr
         // b. For each FunctionDeclaration f that is directly contained in the StatementList of a Block, CaseClause, or DefaultClause Contained within body, do
         TRY(program.for_each_function_hoistable_with_annexB_extension([&](FunctionDeclaration& function_declaration) -> ThrowCompletionOr<void> {
             // i. Let F be StringValue of the BindingIdentifier of f.
-            auto& function_name = function_declaration.name();
+            auto function_name = function_declaration.name();
 
             // ii. If replacing the FunctionDeclaration f with a VariableStatement that has F as a BindingIdentifier would not produce any Early Errors for body, then
             // Note: This is checked during parsing and for_each_function_hoistable_with_annexB_extension so it always passes here.

Userland/Libraries/LibJS/Runtime/ECMAScriptFunctionObject.cpp

@@ -534,7 +534,7 @@ ThrowCompletionOr<void> ECMAScriptFunctionObject::function_declaration_instantia
         // NOTE: Due to the use of MUST with `create_mutable_binding` and `initialize_binding` below,
         //       an exception should not result from `for_each_function_hoistable_with_annexB_extension`.
         MUST(scope_body->for_each_function_hoistable_with_annexB_extension([&](FunctionDeclaration& function_declaration) {
-            auto& function_name = function_declaration.name();
+            auto function_name = function_declaration.name();
             if (parameter_names.contains(function_name))
                 return;
             // The spec says 'initializedBindings' here but that does not exist and it then adds it to 'instantiatedVarNames' so it probably means 'instantiatedVarNames'.