LibJS: More properly implement scoping rules in bytecode codegen

Now we emit CreateVariable and SetVariable with the appropriate
initialization/environment modes, much closer to the spec.
This makes a whole lot of things like let/const variables, function
and variable hoisting and some other things work :^)

Userland/Libraries/LibJS/Bytecode/Generator.h

@@ -24,6 +24,11 @@ namespace JS::Bytecode {
 
 class Generator {
 public:
+ enum class SurroundingScopeKind {
+ Global,
+ Function,
+ Block,
+ };
  static CodeGenerationErrorOr<NonnullOwnPtr<Executable>> generate(ASTNode const&, FunctionKind = FunctionKind::Normal);
 
  Register allocate_register();
@@ -117,6 +122,55 @@ class Generator {
  bool is_in_generator_function() const { return m_enclosing_function_kind == FunctionKind::Generator; }
  bool is_in_async_function() const { return m_enclosing_function_kind == FunctionKind::Async; }
 
+ enum class BindingMode {
+ Lexical,
+ Var,
+ Global,
+ };
+ struct LexicalScope {
+ SurroundingScopeKind kind;
+ BindingMode mode;
+ HashTable<IdentifierTableIndex> known_bindings;
+ };
+
+ void register_binding(IdentifierTableIndex identifier, BindingMode mode = BindingMode::Lexical)
+ {
+ m_variable_scopes.last_matching([&](auto& x) { return x.mode == BindingMode::Global || x.mode == mode; })->known_bindings.set(identifier);
+ }
+ bool has_binding(IdentifierTableIndex identifier, Optional<BindingMode> const& specific_binding_mode = {})
+ {
+ for (auto index = m_variable_scopes.size(); index > 0; --index) {
+ auto& scope = m_variable_scopes[index - 1];
+
+ if (scope.mode != BindingMode::Global && specific_binding_mode.value_or(scope.mode) != scope.mode)
+ continue;
+
+ if (scope.known_bindings.contains(identifier))
+ return true;
+ }
+ return false;
+ }
+ void begin_variable_scope(BindingMode mode = BindingMode::Lexical, SurroundingScopeKind kind = SurroundingScopeKind::Block)
+ {
+ m_variable_scopes.append({ kind, mode, {} });
+ if (mode != BindingMode::Global) {
+ emit<Bytecode::Op::CreateEnvironment>(
+ mode == BindingMode::Lexical
+ ? Bytecode::Op::EnvironmentMode::Lexical
+ : Bytecode::Op::EnvironmentMode::Var);
+ }
+ }
+ void end_variable_scope()
+ {
+ auto mode = m_variable_scopes.take_last().mode;
+ if (mode != BindingMode::Global && !m_current_basic_block->is_terminated()) {
+ emit<Bytecode::Op::LeaveEnvironment>(
+ mode == BindingMode::Lexical
+ ? Bytecode::Op::EnvironmentMode::Lexical
+ : Bytecode::Op::EnvironmentMode::Var);
+ }
+ }
+
 private:
  Generator();
  ~Generator();
@@ -134,6 +188,7 @@ class Generator {
  FunctionKind m_enclosing_function_kind { FunctionKind::Normal };
  Vector<Label> m_continuable_scopes;
  Vector<Label> m_breakable_scopes;
+ Vector<LexicalScope> m_variable_scopes;
 };
 
 }

Userland/Libraries/LibJS/Bytecode/Instruction.h

@@ -20,6 +20,8 @@
  O(ConcatString) \
  O(ContinuePendingUnwind) \
  O(CopyObjectExcludingProperties) \
+ O(CreateEnvironment) \
+ O(CreateVariable) \
  O(Decrement) \
  O(Div) \
  O(EnterUnwindContext) \
@@ -42,6 +44,7 @@
  O(JumpConditional) \
  O(JumpNullish) \
  O(JumpUndefined) \
+ O(LeaveEnvironment) \
  O(LeaveUnwindContext) \
  O(LeftShift) \
  O(LessThan) \

Userland/Libraries/LibJS/Bytecode/Interpreter.cpp

@@ -67,7 +67,7 @@ Interpreter::ValueAndFrame Interpreter::run_and_return_frame(Executable const& e
  if (!m_manually_entered_frames.is_empty() && m_manually_entered_frames.last()) {
  m_register_windows.append(make<RegisterWindow>(m_register_windows.last()));
  } else {
- m_register_windows.append(make<RegisterWindow>());
+ m_register_windows.append(make<RegisterWindow>(MarkedVector<Value>(vm().heap()), MarkedVector<Environment*>(vm().heap()), MarkedVector<Environment*>(vm().heap())));
  }
 
  registers().resize(executable.number_of_registers);
@@ -150,7 +150,7 @@ Interpreter::ValueAndFrame Interpreter::run_and_return_frame(Executable const& e
 
  // NOTE: The return value from a called function is put into $0 in the caller context.
  if (!m_register_windows.is_empty())
- m_register_windows.last()[0] = return_value;
+ m_register_windows.last().registers[0] = return_value;
 
  // At this point we may have already run any queued promise jobs via on_call_stack_emptied,
  // in which case this is a no-op.

Userland/Libraries/LibJS/Bytecode/Interpreter.h

@@ -18,7 +18,11 @@
 
 namespace JS::Bytecode {
 
-using RegisterWindow = Vector<Value>;
+struct RegisterWindow {
+ MarkedVector<Value> registers;
+ MarkedVector<Environment*> saved_lexical_environments;
+ MarkedVector<Environment*> saved_variable_environments;
+};
 
 class Interpreter {
 public:
@@ -48,6 +52,9 @@ class Interpreter {
  Value& reg(Register const& r) { return registers()[r.index()]; }
  [[nodiscard]] RegisterWindow snapshot_frame() const { return m_register_windows.last(); }
 
+ auto& saved_lexical_environment_stack() { return m_register_windows.last().saved_lexical_environments; }
+ auto& saved_variable_environment_stack() { return m_register_windows.last().saved_variable_environments; }
+
  void enter_frame(RegisterWindow const& frame)
  {
  m_manually_entered_frames.append(true);
@@ -82,7 +89,7 @@ class Interpreter {
  VM::InterpreterExecutionScope ast_interpreter_scope();
 
 private:
- RegisterWindow& registers() { return m_register_windows.last(); }
+ MarkedVector<Value>& registers() { return m_register_windows.last().registers; }
 
  static AK::Array<OwnPtr<PassManager>, static_cast<UnderlyingType<Interpreter::OptimizationLevel>>(Interpreter::OptimizationLevel::__Count)> s_optimization_pipelines;
 

Userland/Libraries/LibJS/Bytecode/Op.cpp

@@ -260,12 +260,63 @@ ThrowCompletionOr<void> GetVariable::execute_impl(Bytecode::Interpreter& interpr
  return {};
 }
 
-ThrowCompletionOr<void> SetVariable::execute_impl(Bytecode::Interpreter& interpreter) const
+ThrowCompletionOr<void> CreateEnvironment::execute_impl(Bytecode::Interpreter& interpreter) const
+{
+ auto make_and_swap_envs = [&](auto*& old_environment) {
+ Environment* environment = new_declarative_environment(*old_environment);
+ swap(old_environment, environment);
+ return environment;
+ };
+ if (m_mode == EnvironmentMode::Lexical)
+ interpreter.saved_lexical_environment_stack().append(make_and_swap_envs(interpreter.vm().running_execution_context().lexical_environment));
+ else if (m_mode == EnvironmentMode::Var)
+ interpreter.saved_variable_environment_stack().append(make_and_swap_envs(interpreter.vm().running_execution_context().variable_environment));
+ return {};
+}
+
+ThrowCompletionOr<void> CreateVariable::execute_impl(Bytecode::Interpreter& interpreter) const
 {
  auto& vm = interpreter.vm();
- auto reference = TRY(vm.resolve_binding(interpreter.current_executable().get_identifier(m_identifier)));
+ auto const& name = interpreter.current_executable().get_identifier(m_identifier);
 
- TRY(reference.put_value(interpreter.global_object(), interpreter.accumulator()));
+ if (m_mode == EnvironmentMode::Lexical) {
+ // Note: This is papering over an issue where "FunctionDeclarationInstantiation" creates these bindings for us.
+ // Instead of crashing in there, we'll just raise an exception here.
+ if (TRY(vm.lexical_environment()->has_binding(name)))
+ return vm.throw_completion<InternalError>(interpreter.global_object(), String::formatted("Lexical environment already has binding '{}'", name));
+
+ if (m_is_immutable)
+ vm.lexical_environment()->create_immutable_binding(interpreter.global_object(), name, vm.in_strict_mode());
+ else
+ vm.lexical_environment()->create_mutable_binding(interpreter.global_object(), name, vm.in_strict_mode());
+ } else {
+ if (m_is_immutable)
+ vm.variable_environment()->create_immutable_binding(interpreter.global_object(), name, vm.in_strict_mode());
+ else
+ vm.variable_environment()->create_mutable_binding(interpreter.global_object(), name, vm.in_strict_mode());
+ }
+ return {};
+}
+
+ThrowCompletionOr<void> SetVariable::execute_impl(Bytecode::Interpreter& interpreter) const
+{
+ auto& vm = interpreter.vm();
+ auto const& name = interpreter.current_executable().get_identifier(m_identifier);
+ auto environment = m_mode == EnvironmentMode::Lexical ? vm.running_execution_context().lexical_environment : vm.running_execution_context().variable_environment;
+ auto reference = TRY(vm.resolve_binding(name, environment));
+ switch (m_initialization_mode) {
+ case InitializationMode::Initialize:
+ TRY(reference.initialize_referenced_binding(interpreter.global_object(), interpreter.accumulator()));
+ break;
+ case InitializationMode::Set:
+ TRY(reference.put_value(interpreter.global_object(), interpreter.accumulator()));
+ break;
+ case InitializationMode::InitializeOrSet:
+ VERIFY(reference.is_environment_reference());
+ VERIFY(reference.base_environment().is_declarative_environment());
+ TRY(static_cast<DeclarativeEnvironment&>(reference.base_environment()).initialize_or_set_mutable_binding(interpreter.global_object(), name, interpreter.accumulator()));
+ break;
+ }
  return {};
 }
 
@@ -440,6 +491,15 @@ void FinishUnwind::replace_references_impl(BasicBlock const& from, BasicBlock co
  m_next_target = Label { to };
 }
 
+ThrowCompletionOr<void> LeaveEnvironment::execute_impl(Bytecode::Interpreter& interpreter) const
+{
+ if (m_mode == EnvironmentMode::Lexical)
+ interpreter.vm().running_execution_context().lexical_environment = interpreter.saved_lexical_environment_stack().take_last();
+ if (m_mode == EnvironmentMode::Var)
+ interpreter.vm().running_execution_context().variable_environment = interpreter.saved_variable_environment_stack().take_last();
+ return {};
+}
+
 ThrowCompletionOr<void> LeaveUnwindContext::execute_impl(Bytecode::Interpreter& interpreter) const
 {
  interpreter.leave_unwind_context();
@@ -628,9 +688,27 @@ String GetVariable::to_string_impl(Bytecode::Executable const& executable) const
  return String::formatted("GetVariable {} ({})", m_identifier, executable.identifier_table->get(m_identifier));
 }
 
+String CreateEnvironment::to_string_impl(Bytecode::Executable const&) const
+{
+ auto mode_string = m_mode == EnvironmentMode::Lexical
+ ? "Lexical"
+ : "Variable";
+ return String::formatted("CreateEnvironment mode:{}", mode_string);
+}
+
+String CreateVariable::to_string_impl(Bytecode::Executable const& executable) const
+{
+ auto mode_string = m_mode == EnvironmentMode::Lexical ? "Lexical" : "Variable";
+ return String::formatted("CreateVariable env:{} immutable:{} {} ({})", mode_string, m_is_immutable, m_identifier, executable.identifier_table->get(m_identifier));
+}
+
 String SetVariable::to_string_impl(Bytecode::Executable const& executable) const
 {
- return String::formatted("SetVariable {} ({})", m_identifier, executable.identifier_table->get(m_identifier));
+ auto initialization_mode_name = m_initialization_mode == InitializationMode ::Initialize ? "Initialize"
+ : m_initialization_mode == InitializationMode::Set ? "Set"
+ : "InitializeOrSet";
+ auto mode_string = m_mode == EnvironmentMode::Lexical ? "Lexical" : "Variable";
+ return String::formatted("SetVariable env:{} init:{} {} ({})", mode_string, initialization_mode_name, m_identifier, executable.identifier_table->get(m_identifier));
 }
 
 String PutById::to_string_impl(Bytecode::Executable const& executable) const
@@ -729,6 +807,14 @@ String FinishUnwind::to_string_impl(const Bytecode::Executable&) const
  return String::formatted("FinishUnwind next:{}", m_next_target);
 }
 
+String LeaveEnvironment::to_string_impl(Bytecode::Executable const&) const
+{
+ auto mode_string = m_mode == EnvironmentMode::Lexical
+ ? "Lexical"
+ : "Variable";
+ return String::formatted("LeaveEnvironment env:{}", mode_string);
+}
+
 String LeaveUnwindContext::to_string_impl(Bytecode::Executable const&) const
 {
  return "LeaveUnwindContext";

Userland/Libraries/LibJS/Bytecode/Op.h

@@ -282,11 +282,59 @@ class ConcatString final : public Instruction {
  Register m_lhs;
 };
 
+enum class EnvironmentMode {
+ Lexical,
+ Var,
+};
+
+class CreateEnvironment final : public Instruction {
+public:
+ explicit CreateEnvironment(EnvironmentMode mode)
+ : Instruction(Type::CreateEnvironment)
+ , m_mode(mode)
+ {
+ }
+
+ ThrowCompletionOr<void> execute_impl(Bytecode::Interpreter&) const;
+ String to_string_impl(Bytecode::Executable const&) const;
+ void replace_references_impl(BasicBlock const&, BasicBlock const&) { }
+
+private:
+ EnvironmentMode m_mode { EnvironmentMode::Lexical };
+};
+
+class CreateVariable final : public Instruction {
+public:
+ explicit CreateVariable(IdentifierTableIndex identifier, EnvironmentMode mode, bool is_immutable)
+ : Instruction(Type::CreateVariable)
+ , m_identifier(identifier)
+ , m_mode(mode)
+ , m_is_immutable(is_immutable)
+ {
+ }
+
+ ThrowCompletionOr<void> execute_impl(Bytecode::Interpreter&) const;
+ String to_string_impl(Bytecode::Executable const&) const;
+ void replace_references_impl(BasicBlock const&, BasicBlock const&) { }
+
+private:
+ IdentifierTableIndex m_identifier;
+ EnvironmentMode m_mode;
+ bool m_is_immutable { false };
+};
+
 class SetVariable final : public Instruction {
 public:
- explicit SetVariable(IdentifierTableIndex identifier)
+ enum class InitializationMode {
+ Initialize,
+ Set,
+ InitializeOrSet,
+ };
+ explicit SetVariable(IdentifierTableIndex identifier, InitializationMode initialization_mode = InitializationMode::Set, EnvironmentMode mode = EnvironmentMode::Lexical)
  : Instruction(Type::SetVariable)
  , m_identifier(identifier)
+ , m_mode(mode)
+ , m_initialization_mode(initialization_mode)
  {
  }
 
@@ -296,6 +344,8 @@ class SetVariable final : public Instruction {
 
 private:
  IdentifierTableIndex m_identifier;
+ EnvironmentMode m_mode;
+ InitializationMode m_initialization_mode { InitializationMode::Set };
 };
 
 class GetVariable final : public Instruction {
@@ -599,6 +649,22 @@ class EnterUnwindContext final : public Instruction {
  Optional<Label> m_finalizer_target;
 };
 
+class LeaveEnvironment final : public Instruction {
+public:
+ LeaveEnvironment(EnvironmentMode mode)
+ : Instruction(Type::LeaveEnvironment)
+ , m_mode(mode)
+ {
+ }
+
+ ThrowCompletionOr<void> execute_impl(Bytecode::Interpreter&) const;
+ String to_string_impl(Bytecode::Executable const&) const;
+ void replace_references_impl(BasicBlock const&, BasicBlock const&) { }
+
+private:
+ EnvironmentMode m_mode { EnvironmentMode::Lexical };
+};
+
 class LeaveUnwindContext final : public Instruction {
 public:
  LeaveUnwindContext()

Userland/Libraries/LibJS/Runtime/DeclarativeEnvironment.cpp

@@ -196,15 +196,21 @@ ThrowCompletionOr<bool> DeclarativeEnvironment::delete_binding(GlobalObject&, Fl
  return true;
 }
 
-void DeclarativeEnvironment::initialize_or_set_mutable_binding(Badge<ScopeNode>, GlobalObject& global_object, FlyString const& name, Value value)
+ThrowCompletionOr<void> DeclarativeEnvironment::initialize_or_set_mutable_binding(GlobalObject& global_object, FlyString const& name, Value value)
 {
  auto it = m_names.find(name);
  VERIFY(it != m_names.end());
  auto& binding = m_bindings[it->value];
  if (!binding.initialized)
- MUST(initialize_binding(global_object, name, value));
+ TRY(initialize_binding(global_object, name, value));
  else
- MUST(set_mutable_binding(global_object, name, value, false));
+ TRY(set_mutable_binding(global_object, name, value, false));
+ return {};
+}
+
+void DeclarativeEnvironment::initialize_or_set_mutable_binding(Badge<ScopeNode>, GlobalObject& global_object, FlyString const& name, Value value)
+{
+ MUST(initialize_or_set_mutable_binding(global_object, name, value));
 }
 
 Vector<String> DeclarativeEnvironment::bindings() const

Userland/Libraries/LibJS/Runtime/DeclarativeEnvironment.h

@@ -31,6 +31,7 @@ class DeclarativeEnvironment : public Environment {
  virtual ThrowCompletionOr<bool> delete_binding(GlobalObject&, FlyString const& name) override;
 
  void initialize_or_set_mutable_binding(Badge<ScopeNode>, GlobalObject& global_object, FlyString const& name, Value value);
+ ThrowCompletionOr<void> initialize_or_set_mutable_binding(GlobalObject& global_object, FlyString const& name, Value value);
 
  // This is not a method defined in the spec! Do not use this in any LibJS (or other spec related) code.
  [[nodiscard]] Vector<String> bindings() const;