LibJS: Use a local variable for arguments object when possible

This allows us to skip allocating a function environment in cases where
it was previously impossible because the arguments object needed a
binding.

This change does not bring visible improvement in Kraken or Octane
benchmarks but seems useful to have anyway.

Userland/Libraries/LibJS/Bytecode/Generator.cpp

@@ -45,10 +45,15 @@ CodeGenerationErrorOr<void> Generator::emit_function_declaration_instantiation(E
     }
 
     if (function.m_arguments_object_needed) {
+        Optional<Operand> dst;
+        auto local_var_index = function.m_local_variables_names.find_first_index("arguments"sv);
+        if (local_var_index.has_value())
+            dst = local(local_var_index.value());
+
         if (function.m_strict || !function.has_simple_parameter_list()) {
-            emit<Op::CreateArguments>(Op::CreateArguments::Kind::Unmapped, function.m_strict);
+            emit<Op::CreateArguments>(dst, Op::CreateArguments::Kind::Unmapped, function.m_strict);
         } else {
-            emit<Op::CreateArguments>(Op::CreateArguments::Kind::Mapped, function.m_strict);
+            emit<Op::CreateArguments>(dst, Op::CreateArguments::Kind::Mapped, function.m_strict);
         }
     }
 

Userland/Libraries/LibJS/Bytecode/Interpreter.cpp

@@ -1373,6 +1373,11 @@ ThrowCompletionOr<void> CreateArguments::execute_impl(Bytecode::Interpreter& int
         arguments_object = create_unmapped_arguments_object(interpreter.vm(), passed_arguments);
     }
 
+    if (m_dst.has_value()) {
+        interpreter.set(*m_dst, arguments_object);
+        return {};
+    }
+
     if (m_is_immutable) {
         MUST(environment->create_immutable_binding(interpreter.vm(), interpreter.vm().names.arguments.as_string(), false));
     } else {
@@ -2110,9 +2115,14 @@ ByteString CreateRestParams::to_byte_string_impl(Bytecode::Executable const& exe
     return ByteString::formatted("CreateRestParams {}, rest_index:{}", format_operand("dst"sv, m_dst, executable), m_rest_index);
 }
 
-ByteString CreateArguments::to_byte_string_impl(Bytecode::Executable const&) const
+ByteString CreateArguments::to_byte_string_impl(Bytecode::Executable const& executable) const
 {
-    return ByteString::formatted("CreateArguments {} immutable:{}", m_kind == Kind::Mapped ? "mapped"sv : "unmapped"sv, m_is_immutable);
+    StringBuilder builder;
+    builder.appendff("CreateArguments");
+    if (m_dst.has_value())
+        builder.appendff(" {}", format_operand("dst"sv, *m_dst, executable));
+    builder.appendff(" {} immutable:{}", m_kind == Kind::Mapped ? "mapped"sv : "unmapped"sv, m_is_immutable);
+    return builder.to_byte_string();
 }
 
 ByteString EnterObjectEnvironment::to_byte_string_impl(Executable const& executable) const

Userland/Libraries/LibJS/Bytecode/Op.h

@@ -60,17 +60,24 @@ class CreateArguments final : public Instruction {
         Unmapped,
     };
 
-    CreateArguments(Kind kind, bool is_immutable)
+    CreateArguments(Optional<Operand> dst, Kind kind, bool is_immutable)
         : Instruction(Type::CreateArguments)
+        , m_dst(dst)
         , m_kind(kind)
         , m_is_immutable(is_immutable)
     {
     }
 
     ThrowCompletionOr<void> execute_impl(Bytecode::Interpreter&) const;
     ByteString to_byte_string_impl(Bytecode::Executable const&) const;
+    void visit_operands_impl(Function<void(Operand&)> visitor)
+    {
+        if (m_dst.has_value())
+            visitor(m_dst.value());
+    }
 
 private:
+    Optional<Operand> m_dst;
     Kind m_kind;
     bool m_is_immutable { false };
 };

Userland/Libraries/LibJS/Parser.cpp

@@ -295,17 +295,15 @@ class ScopePusher {
                 continue;
             }
 
-            if (identifier_group_name == "arguments"sv) {
-                // NOTE: arguments is a special variable that should not be treated as a candidate to become local
-                continue;
-            }
-
             bool scope_has_declaration = false;
             if (is_top_level() && m_var_names.contains(identifier_group_name))
                 scope_has_declaration = true;
             else if (m_lexical_names.contains(identifier_group_name) || m_function_names.contains(identifier_group_name))
                 scope_has_declaration = true;
 
+            if (m_type == ScopeType::Function && !m_is_arrow_function && identifier_group_name == "arguments"sv)
+                scope_has_declaration = true;
+
             bool hoistable_function_declaration = false;
             for (auto const& function_declaration : m_functions_to_hoist) {
                 if (function_declaration->name() == identifier_group_name)
@@ -448,6 +446,11 @@ class ScopePusher {
         }
     }
 
+    void set_is_arrow_function()
+    {
+        m_is_arrow_function = true;
+    }
+
 private:
     void throw_identifier_declared(DeprecatedFlyString const& name, NonnullRefPtr<Declaration const> const& declaration)
     {
@@ -489,6 +492,7 @@ class ScopePusher {
     bool m_contains_await_expression { false };
     bool m_screwed_by_eval_in_scope_chain { false };
     bool m_uses_this { false };
+    bool m_is_arrow_function { false };
 };
 
 class OperatorPrecedenceTable {
@@ -995,6 +999,7 @@ RefPtr<FunctionExpression const> Parser::try_parse_arrow_function_expression(boo
     bool uses_this = false;
     auto function_body_result = [&]() -> RefPtr<FunctionBody const> {
         ScopePusher function_scope = ScopePusher::function_scope(*this);
+        function_scope.set_is_arrow_function();
 
         if (expect_parens) {
             // We have parens around the function parameters and can re-use the same parsing

Userland/Libraries/LibJS/Runtime/ECMAScriptFunctionObject.cpp

@@ -328,7 +328,8 @@ ECMAScriptFunctionObject::ECMAScriptFunctionObject(DeprecatedFlyString name, Byt
         }));
     }
 
-    m_function_environment_needed = m_arguments_object_needed || m_function_environment_bindings_count > 0 || m_var_environment_bindings_count > 0 || m_lex_environment_bindings_count > 0 || uses_this == UsesThis::Yes || m_contains_direct_call_to_eval;
+    auto arguments_object_needs_binding = m_arguments_object_needed && !m_local_variables_names.contains_slow(vm().names.arguments.as_string());
+    m_function_environment_needed = arguments_object_needs_binding || m_function_environment_bindings_count > 0 || m_var_environment_bindings_count > 0 || m_lex_environment_bindings_count > 0 || uses_this == UsesThis::Yes || m_contains_direct_call_to_eval;
 }
 
 void ECMAScriptFunctionObject::initialize(Realm& realm)