From ffe304e88b40ff88e9d5c6ea78eaeae660bf759b Mon Sep 17 00:00:00 2001
From: Andreas Kling <kling@serenityos.org>
Date: Thu, 16 Nov 2023 12:21:20 +0100
Subject: [PATCH] LibJS: Don't create `arguments` object due to `o.arguments`
 access

When deciding whether we need to create a full-blown `arguments` object,
we look at various things, starting as early as in the parser.

Until now, if the parser saw the identifier `arguments`, we'd decide
that it's enough of a clue that we should create the `arguments` object
since somebody is obviously accessing it.

However, that missed the case where someone is just accessing a property
named `arguments` on some object. In such cases (`o.arguments`), we now
hold off on creating an `arguments` object.

~11% speed-up on Octane/typescript.js :^)
---
 Userland/Libraries/LibJS/Parser.cpp | 15 +++++++++++----
 Userland/Libraries/LibJS/Parser.h   |  1 +
 2 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/Userland/Libraries/LibJS/Parser.cpp b/Userland/Libraries/LibJS/Parser.cpp
index b787ae1f87..3fbe0657ca 100644
--- a/Userland/Libraries/LibJS/Parser.cpp
+++ b/Userland/Libraries/LibJS/Parser.cpp
@@ -4252,6 +4252,7 @@ Token Parser::consume()
     if (old_token.is_identifier_name() && (m_state.current_token.type() == TokenType::Slash || m_state.current_token.type() == TokenType::SlashEquals)) {
         m_state.current_token = m_state.lexer.force_slash_as_regex();
     }
+    m_state.previous_token_was_period = old_token.type() == TokenType::Period;
     return old_token;
 }
 
@@ -4259,11 +4260,17 @@ Token Parser::consume_and_allow_division()
 {
     auto old_token = m_state.current_token;
     m_state.current_token = m_state.lexer.next();
-    // NOTE: This is the bare minimum needed to decide whether we might need an arguments object
-    // in a function expression or declaration. ("might" because the AST implements some further
-    // conditions from the spec that rule out the need for allocating one)
-    if (old_token.type() == TokenType::Identifier && old_token.value().is_one_of("arguments"sv, "eval"sv))
+
+    // NOTE: This is the bare minimum needed to decide whether we might need an `arguments` object
+    //       in a function expression or declaration. ("Might" because ESFO implements some further
+    //       conditions from the spec that rule out the need for allocating one.)
+    //       Basically any freestanding use of `arguments` in a function body. This is not perfect
+    //       but avoids a lot of unnecessary arguments objects. We check if the previous token was
+    //       a period to avoid creating `arguments` due to an unrelated property access (`o.arguments`)
+    if (old_token.type() == TokenType::Identifier && old_token.value().is_one_of("arguments"sv, "eval"sv) && !m_state.previous_token_was_period)
         m_state.function_might_need_arguments_object = true;
+
+    m_state.previous_token_was_period = old_token.type() == TokenType::Period;
     return old_token;
 }
 
diff --git a/Userland/Libraries/LibJS/Parser.h b/Userland/Libraries/LibJS/Parser.h
index 0a08894ffd..06c0ce47ac 100644
--- a/Userland/Libraries/LibJS/Parser.h
+++ b/Userland/Libraries/LibJS/Parser.h
@@ -305,6 +305,7 @@ private:
     struct ParserState {
         Lexer lexer;
         Token current_token;
+        bool previous_token_was_period { false };
         Vector<ParserError> errors;
         ScopePusher* current_scope_pusher { nullptr };