LibJS: Remove [[VarNames]] from GlobalEnvironment

This is a normative change in the ECMA-262 spec. See:
ed75310

Libraries/LibJS/AST.cpp

@@ -1693,17 +1693,16 @@ ThrowCompletionOr<void> Program::global_declaration_instantiation(VM& vm, Global
     TRY(for_each_lexically_declared_identifier([&](Identifier const& identifier) -> ThrowCompletionOr<void> {
         auto const& name = identifier.string();
 
-        // a. If env.HasVarDeclaration(name) is true, throw a SyntaxError exception.
+        // a. If HasLexicalDeclaration(env, name) is true, throw a SyntaxError exception.
-        if (global_environment.has_var_declaration(name))
-            return vm.throw_completion<SyntaxError>(ErrorType::TopLevelVariableAlreadyDeclared, name);
-
-        // b. If env.HasLexicalDeclaration(name) is true, throw a SyntaxError exception.
         if (global_environment.has_lexical_declaration(name))
             return vm.throw_completion<SyntaxError>(ErrorType::TopLevelVariableAlreadyDeclared, name);
 
-        // c. Let hasRestrictedGlobal be ? env.HasRestrictedGlobalProperty(name).
+        // b. Let hasRestrictedGlobal be ? HasRestrictedGlobalProperty(env, name).
         auto has_restricted_global = TRY(global_environment.has_restricted_global_property(name));
 
+        // c. NOTE: Global var and function bindings (except those that are introduced by non-strict direct eval) are
+        //    non-configurable and are therefore restricted global properties.
+
         // d. If hasRestrictedGlobal is true, throw a SyntaxError exception.
         if (has_restricted_global)
             return vm.throw_completion<SyntaxError>(ErrorType::RestrictedGlobalProperty, name);

Libraries/LibJS/Runtime/GlobalEnvironment.cpp

@@ -146,34 +146,15 @@ ThrowCompletionOr<bool> GlobalEnvironment::delete_binding(VM& vm, FlyString cons
 
     // 6. If existingProp is true, then
     if (existing_prop) {
-        // a. Let status be ? ObjRec.DeleteBinding(N).
+        // a. Return ? ObjRec.DeleteBinding(N).
-        bool status = TRY(m_object_record->delete_binding(vm, name));
+        return TRY(m_object_record->delete_binding(vm, name));
-
-        // b. If status is true, then
-        if (status) {
-            // i. Let varNames be envRec.[[VarNames]].
-            // ii. If N is an element of varNames, remove that element from the varNames.
-            m_var_names.remove_all_matching([&](auto& entry) { return entry == name; });
-        }
-
-        // c. Return status.
-        return status;
     }
 
     // 7. Return true.
     return true;
 }
 
-// 9.1.1.4.12 HasVarDeclaration ( N ), https://tc39.es/ecma262/#sec-hasvardeclaration
+// 9.1.1.4.12 HasLexicalDeclaration ( envRec, N ), https://tc39.es/ecma262/#sec-haslexicaldeclaration
-bool GlobalEnvironment::has_var_declaration(FlyString const& name) const
-{
-    // 1. Let varDeclaredNames be envRec.[[VarNames]].
-    // 2. If varDeclaredNames contains N, return true.
-    // 3. Return false.
-    return m_var_names.contains_slow(name);
-}
-
-// 9.1.1.4.13 HasLexicalDeclaration ( N ), https://tc39.es/ecma262/#sec-haslexicaldeclaration
 bool GlobalEnvironment::has_lexical_declaration(FlyString const& name) const
 {
     // 1. Let DclRec be envRec.[[DeclarativeRecord]].
@@ -181,7 +162,7 @@ bool GlobalEnvironment::has_lexical_declaration(FlyString const& name) const
     return MUST(m_declarative_record->has_binding(name));
 }
 
-// 9.1.1.4.14 HasRestrictedGlobalProperty ( N ), https://tc39.es/ecma262/#sec-hasrestrictedglobalproperty
+// 9.1.1.4.13 HasRestrictedGlobalProperty ( envRec, N ), https://tc39.es/ecma262/#sec-hasrestrictedglobalproperty
 ThrowCompletionOr<bool> GlobalEnvironment::has_restricted_global_property(FlyString const& name) const
 {
     // 1. Let ObjRec be envRec.[[ObjectRecord]].
@@ -203,7 +184,7 @@ ThrowCompletionOr<bool> GlobalEnvironment::has_restricted_global_property(FlyStr
     return true;
 }
 
-// 9.1.1.4.15 CanDeclareGlobalVar ( N ), https://tc39.es/ecma262/#sec-candeclareglobalvar
+// 9.1.1.4.14 CanDeclareGlobalVar ( envRec, N ), https://tc39.es/ecma262/#sec-candeclareglobalvar
 ThrowCompletionOr<bool> GlobalEnvironment::can_declare_global_var(FlyString const& name) const
 {
     // 1. Let ObjRec be envRec.[[ObjectRecord]].
@@ -221,7 +202,7 @@ ThrowCompletionOr<bool> GlobalEnvironment::can_declare_global_var(FlyString cons
     return global_object.is_extensible();
 }
 
-// 9.1.1.4.16 CanDeclareGlobalFunction ( N ), https://tc39.es/ecma262/#sec-candeclareglobalfunction
+// 9.1.1.4.15 CanDeclareGlobalFunction ( envRec, N ), https://tc39.es/ecma262/#sec-candeclareglobalfunction
 ThrowCompletionOr<bool> GlobalEnvironment::can_declare_global_function(FlyString const& name) const
 {
     // 1. Let ObjRec be envRec.[[ObjectRecord]].
@@ -247,7 +228,7 @@ ThrowCompletionOr<bool> GlobalEnvironment::can_declare_global_function(FlyString
     return false;
 }
 
-// 9.1.1.4.17 CreateGlobalVarBinding ( N, D ), https://tc39.es/ecma262/#sec-createglobalvarbinding
+// 9.1.1.4.16 CreateGlobalVarBinding ( envRec, N, D ), https://tc39.es/ecma262/#sec-createglobalvarbinding
 ThrowCompletionOr<void> GlobalEnvironment::create_global_var_binding(FlyString const& name, bool can_be_deleted)
 {
     auto& vm = this->vm();
@@ -271,18 +252,11 @@ ThrowCompletionOr<void> GlobalEnvironment::create_global_var_binding(FlyString c
         TRY(m_object_record->initialize_binding(vm, name, js_undefined(), Environment::InitializeBindingHint::Normal));
     }
 
-    // 6. Let varDeclaredNames be envRec.[[VarNames]].
+    // 6. Return UNUSED.
-    // 7. If varDeclaredNames does not contain N, then
-    if (!m_var_names.contains_slow(name)) {
-        // a. Append N to varDeclaredNames.
-        m_var_names.append(name);
-    }
-
-    // 8. Return unused.
     return {};
 }
 
-// 9.1.1.4.18 CreateGlobalFunctionBinding ( N, V, D ), https://tc39.es/ecma262/#sec-createglobalfunctionbinding
+// 9.1.1.4.17 CreateGlobalFunctionBinding ( envRec, N, V, D ), https://tc39.es/ecma262/#sec-createglobalfunctionbinding
 ThrowCompletionOr<void> GlobalEnvironment::create_global_function_binding(FlyString const& name, Value value, bool can_be_deleted)
 {
     // 1. Let ObjRec be envRec.[[ObjectRecord]].
@@ -311,14 +285,7 @@ ThrowCompletionOr<void> GlobalEnvironment::create_global_function_binding(FlyStr
     // 7. Perform ? Set(globalObject, N, V, false).
     TRY(global_object.set(name, value, Object::ShouldThrowExceptions::Yes));
 
-    // 8. Let varDeclaredNames be envRec.[[VarNames]].
+    // 8. Return UNUSED.
-    // 9. If varDeclaredNames does not contain N, then
-    if (!m_var_names.contains_slow(name)) {
-        // a. Append N to varDeclaredNames.
-        m_var_names.append(name);
-    }
-
-    // 10. Return unused.
     return {};
 }
 

Libraries/LibJS/Runtime/GlobalEnvironment.h

@@ -30,7 +30,6 @@ public:
     Object& global_this_value() { return *m_global_this_value; }
     DeclarativeEnvironment& declarative_record() { return *m_declarative_record; }
 
-    bool has_var_declaration(FlyString const& name) const;
     bool has_lexical_declaration(FlyString const& name) const;
     ThrowCompletionOr<bool> has_restricted_global_property(FlyString const& name) const;
     ThrowCompletionOr<bool> can_declare_global_var(FlyString const& name) const;
@@ -47,7 +46,6 @@ private:
     GC::Ptr<ObjectEnvironment> m_object_record;           // [[ObjectRecord]]
     GC::Ptr<Object> m_global_this_value;                  // [[GlobalThisValue]]
     GC::Ptr<DeclarativeEnvironment> m_declarative_record; // [[DeclarativeRecord]]
-    Vector<FlyString> m_var_names;                        // [[VarNames]]
 };
 
 template<>

Libraries/LibJS/Tests/eval-redeclaration.js

@@ -0,0 +1,6 @@
+eval("var foo;");
+evaluateSource("let foo = 1;");
+
+test("redeclaration of variable in global scope succeeded", () => {
+    expect(foo).toBe(1);
+});

Tests/LibJS/test-js.cpp

@@ -32,6 +32,20 @@ TESTJS_GLOBAL_FUNCTION(can_parse_source, canParseSource)
     return JS::Value(!parser.has_errors());
 }
 
+// Based on $262.evalScript
+TESTJS_GLOBAL_FUNCTION(evaluate_source, evaluateSource)
+{
+    auto& realm = *vm.current_realm();
+
+    auto source = TRY(vm.argument(0).to_string(vm));
+
+    auto script = JS::Script::parse(source, realm);
+    if (script.is_error())
+        return vm.throw_completion<JS::SyntaxError>(script.error().first().to_string());
+
+    return vm.bytecode_interpreter().run(script.value());
+}
+
 TESTJS_GLOBAL_FUNCTION(run_queued_promise_jobs, runQueuedPromiseJobs)
 {
     vm.run_queued_promise_jobs();