LibJS: Partially revert 12b283f

This commit partially reverts "LibJS: Make accessing the current
function's arguments cheaper".
While the change passed all the currently passing test262 tests, it
seems to have _some_ flaw that silently breaks with some real-world
websites.
As the speedup with negligible at best, let's just revert it until we
can implement it more correctly.

Userland/Libraries/LibJS/AST.cpp

@@ -1012,28 +1012,10 @@ Reference Identifier::to_reference(Interpreter& interpreter, GlobalObject&) cons
         VERIFY(environment);
         VERIFY(environment->is_declarative_environment());
         if (!environment->is_permanently_screwed_by_eval()) {
-            if (m_lexically_bound_function_argument.has_value()) {
-                return Reference {
-                    *environment,
-                    string(),
-                    *m_lexically_bound_function_argument,
-                    interpreter.vm().in_strict_mode(),
-                    m_cached_environment_coordinate,
-                    &interpreter.vm().running_execution_context(),
-                };
-            }
             return Reference { *environment, string(), interpreter.vm().in_strict_mode(), m_cached_environment_coordinate };
         }
         m_cached_environment_coordinate = {};
     }
-    if (m_lexically_bound_function_argument.has_value()) {
-        return Reference {
-            string(),
-            *m_lexically_bound_function_argument,
-            interpreter.vm().in_strict_mode(),
-            &interpreter.vm().running_execution_context(),
-        };
-    }
 
     auto reference = interpreter.vm().resolve_binding(string());
     if (reference.environment_coordinate().has_value())

Userland/Libraries/LibJS/Parser.cpp

@@ -1534,30 +1534,6 @@ NonnullRefPtr<Expression> Parser::parse_expression(int min_precedence, Associati
             if (identifier_instance->string() == "arguments"sv)
                 m_state.current_scope_pusher->set_contains_access_to_arguments_object();
         }
-
-        if (function_scope && has_not_been_declared_as_variable) {
-
-            auto& parameters = function_scope->function_parameters();
-            Optional<size_t> argument_index;
-            size_t index = 0;
-            for (auto& parameter : parameters) {
-                auto current_index = index++;
-                if (parameter.is_rest)
-                    break;
-                if (auto name_ptr = parameter.binding.get_pointer<FlyString>()) {
-                    // Need VM assistance for this, so let's just pretend it's not there.
-                    if (parameter.default_value)
-                        break;
-                    if (identifier_instance->string() == *name_ptr) {
-                        argument_index = current_index;
-                        if (m_state.strict_mode)
-                            break;
-                    }
-                }
-            }
-            if (argument_index.has_value())
-                m_state.current_scope_pusher->associate_identifier_with_argument_index(identifier_instance, *argument_index);
-        }
     }
 
     while (match(TokenType::TemplateLiteralStart)) {

Userland/Libraries/LibJS/Runtime/Reference.cpp

@@ -48,31 +48,6 @@ void Reference::put_value(GlobalObject& global_object, Value value)
     }
 
     VERIFY(m_base_type == BaseType::Environment);
-
-    // Note: Optimisation, not from the spec.
-    if (m_function_argument_index.has_value()) {
-        // Note: Modifying this binding requires us to sync with the environment.
-        if (!m_base_environment) {
-            auto real_reference = global_object.vm().resolve_binding(m_name.as_string(), m_referenced_function_context->lexical_environment);
-            m_base_environment = real_reference.m_base_environment;
-        }
-
-        if (!global_object.vm().execution_context_stack().is_empty() && m_referenced_function_context == &global_object.vm().running_execution_context()) {
-            auto& arguments = m_referenced_function_context->arguments;
-            auto index = m_function_argument_index.value();
-            if (arguments.size() > index) {
-                arguments[index] = value;
-            } else {
-                arguments.ensure_capacity(index + 1);
-                for (size_t i = arguments.size(); i < index; ++i)
-                    arguments.append(js_undefined());
-                arguments.append(value);
-            }
-            m_base_environment->set_mutable_binding(global_object, name().as_string(), value, is_strict());
-            return;
-        }
-    }
-
     VERIFY(m_base_environment);
     if (m_environment_coordinate.has_value())
         static_cast<DeclarativeEnvironment*>(m_base_environment)->set_mutable_binding_direct(global_object, m_environment_coordinate->index, value, m_strict);
@@ -106,16 +81,6 @@ Value Reference::get_value(GlobalObject& global_object) const
 
     VERIFY(m_base_type == BaseType::Environment);
 
-    // Note: Optimisation, not from the spec.
-    if (m_function_argument_index.has_value()) {
-        if (!global_object.vm().execution_context_stack().is_empty() && m_referenced_function_context == &global_object.vm().running_execution_context())
-            return global_object.vm().argument(m_function_argument_index.value());
-        if (!m_base_environment) {
-            auto real_reference = global_object.vm().resolve_binding(m_name.as_string(), m_referenced_function_context->lexical_environment);
-            m_base_environment = real_reference.m_base_environment;
-        }
-    }
-
     VERIFY(m_base_environment);
     if (m_environment_coordinate.has_value())
         return static_cast<DeclarativeEnvironment*>(m_base_environment)->get_binding_value_direct(global_object, m_environment_coordinate->index, m_strict);
@@ -177,12 +142,6 @@ bool Reference::delete_(GlobalObject& global_object)
 
     VERIFY(m_base_type == BaseType::Environment);
 
-    // Note: Optimisation, not from the spec.
-    if (m_function_argument_index.has_value()) {
-        // This is a direct reference to a function argument.
-        return false;
-    }
-
     //    c. Return ? base.DeleteBinding(ref.[[ReferencedName]]).
     return m_base_environment->delete_binding(global_object, m_name.as_string());
 }

Userland/Libraries/LibJS/Runtime/Reference.h

@@ -55,28 +55,6 @@ public:
     {
     }
 
-    Reference(FlyString referenced_name, size_t function_argument_index, bool strict = false, ExecutionContext* function_context = nullptr)
-        : m_base_type(BaseType::Environment)
-        , m_base_environment(nullptr)
-        , m_name(move(referenced_name))
-        , m_strict(strict)
-        , m_environment_coordinate({})
-        , m_function_argument_index(function_argument_index)
-        , m_referenced_function_context(function_context)
-    {
-    }
-
-    Reference(Environment& base, FlyString referenced_name, size_t function_argument_index, bool strict = false, Optional<EnvironmentCoordinate> environment_coordinate = {}, ExecutionContext* function_context = nullptr)
-        : m_base_type(BaseType::Environment)
-        , m_base_environment(&base)
-        , m_name(move(referenced_name))
-        , m_strict(strict)
-        , m_environment_coordinate(move(environment_coordinate))
-        , m_function_argument_index(function_argument_index)
-        , m_referenced_function_context(function_context)
-    {
-    }
-
     Value base() const
     {
         VERIFY(m_base_type == BaseType::Value);
@@ -157,8 +135,6 @@ private:
     Value m_this_value;
     bool m_strict { false };
     Optional<EnvironmentCoordinate> m_environment_coordinate;
-    Optional<size_t> m_function_argument_index;
-    ExecutionContext* m_referenced_function_context { nullptr };
 };
 
 }