diff --git a/Userland/Libraries/LibJS/Bytecode/ASTCodegen.cpp b/Userland/Libraries/LibJS/Bytecode/ASTCodegen.cpp index 0e7646704a..ca8e7e227b 100644 --- a/Userland/Libraries/LibJS/Bytecode/ASTCodegen.cpp +++ b/Userland/Libraries/LibJS/Bytecode/ASTCodegen.cpp @@ -2168,17 +2168,11 @@ Bytecode::CodeGenerationErrorOr BreakStatement::generate_bytecode(Bytecode // We need to execute the finally block, but tell it to resume // execution at the designated block if (m_target_label.is_null()) { - generator.perform_needed_unwinds(true); - generator.emit().set_targets( - generator.nearest_breakable_scope(), - {}); + generator.generate_break(); return {}; } - auto target_to_jump_to = generator.perform_needed_unwinds_for_labelled_break_and_return_target_block(m_target_label); - generator.emit().set_targets( - target_to_jump_to, - {}); + generator.generate_break(m_target_label); return {}; } @@ -2192,6 +2186,7 @@ Bytecode::CodeGenerationErrorOr TryStatement::generate_bytecode(Bytecode:: Bytecode::BasicBlock* next_block { nullptr }; if (m_finalizer) { + // FIXME: See notes in Op.h->ScheduleJump auto& finalizer_block = generator.make_block(); generator.switch_to_basic_block(finalizer_block); generator.emit(); diff --git a/Userland/Libraries/LibJS/Bytecode/Generator.cpp b/Userland/Libraries/LibJS/Bytecode/Generator.cpp index 8df4ff676c..771454a056 100644 --- a/Userland/Libraries/LibJS/Bytecode/Generator.cpp +++ b/Userland/Libraries/LibJS/Bytecode/Generator.cpp @@ -262,23 +262,62 @@ CodeGenerationErrorOr Generator::emit_delete_reference(JS::ASTNode const& return {}; } -Label Generator::perform_needed_unwinds_for_labelled_break_and_return_target_block(DeprecatedFlyString const& break_label) +void Generator::generate_break() +{ + bool last_was_finally = false; + // FIXME: Reduce code duplication + for (size_t i = m_boundaries.size(); i > 0; --i) { + auto boundary = m_boundaries[i - 1]; + using enum BlockBoundaryType; + switch (boundary) { + case Break: + emit().set_targets(nearest_breakable_scope(), {}); + return; + case Unwind: + if (!last_was_finally) + emit(); + last_was_finally = false; + break; + case LeaveLexicalEnvironment: + emit(Bytecode::Op::EnvironmentMode::Lexical); + break; + case LeaveVariableEnvironment: + emit(Bytecode::Op::EnvironmentMode::Var); + break; + case Continue: + break; + case ReturnToFinally: { + auto& block = make_block(DeprecatedString::formatted("{}.break", current_block().name())); + emit(Label { block }); + switch_to_basic_block(block); + last_was_finally = true; + break; + }; + } + } + VERIFY_NOT_REACHED(); +} + +void Generator::generate_break(DeprecatedFlyString const& break_label) { size_t current_boundary = m_boundaries.size(); - for (auto& breakable_scope : m_breakable_scopes.in_reverse()) { + bool last_was_finally = false; + for (auto const& breakable_scope : m_breakable_scopes.in_reverse()) { for (; current_boundary > 0; --current_boundary) { auto boundary = m_boundaries[current_boundary - 1]; - // FIXME: Handle ReturnToFinally in a graceful manner - // We need to execute the finally block, but tell it to resume - // execution at the designated label if (boundary == BlockBoundaryType::Unwind) { - emit(); + if (!last_was_finally) + emit(); + last_was_finally = false; } else if (boundary == BlockBoundaryType::LeaveLexicalEnvironment) { emit(Bytecode::Op::EnvironmentMode::Lexical); } else if (boundary == BlockBoundaryType::LeaveVariableEnvironment) { emit(Bytecode::Op::EnvironmentMode::Var); } else if (boundary == BlockBoundaryType::ReturnToFinally) { - // FIXME: We need to enter the `finally`, while still scheduling the break to happen + auto& block = make_block(DeprecatedString::formatted("{}.break", current_block().name())); + emit(Label { block }); + switch_to_basic_block(block); + last_was_finally = true; } else if (boundary == BlockBoundaryType::Break) { // Make sure we don't process this boundary twice if the current breakable scope doesn't contain the target label. --current_boundary; @@ -286,8 +325,10 @@ Label Generator::perform_needed_unwinds_for_labelled_break_and_return_target_blo } } - if (breakable_scope.language_label_set.contains_slow(break_label)) - return breakable_scope.bytecode_target; + if (breakable_scope.language_label_set.contains_slow(break_label)) { + emit().set_targets(breakable_scope.bytecode_target, {}); + return; + } } // We must have a breakable scope available that contains the label, as this should be enforced by the parser. diff --git a/Userland/Libraries/LibJS/Bytecode/Generator.h b/Userland/Libraries/LibJS/Bytecode/Generator.h index 3f966bcb8d..459db7da96 100644 --- a/Userland/Libraries/LibJS/Bytecode/Generator.h +++ b/Userland/Libraries/LibJS/Bytecode/Generator.h @@ -213,7 +213,9 @@ public: } } - Label perform_needed_unwinds_for_labelled_break_and_return_target_block(DeprecatedFlyString const& break_label); + void generate_break(); + void generate_break(DeprecatedFlyString const& break_label); + Label perform_needed_unwinds_for_labelled_continue_and_return_target_block(DeprecatedFlyString const& continue_label); void start_boundary(BlockBoundaryType type) { m_boundaries.append(type); } diff --git a/Userland/Libraries/LibJS/Bytecode/Instruction.h b/Userland/Libraries/LibJS/Bytecode/Instruction.h index 72255f2412..56b1171022 100644 --- a/Userland/Libraries/LibJS/Bytecode/Instruction.h +++ b/Userland/Libraries/LibJS/Bytecode/Instruction.h @@ -78,6 +78,7 @@ O(ResolveThisBinding) \ O(Return) \ O(RightShift) \ + O(ScheduleJump) \ O(SetVariable) \ O(Store) \ O(StrictlyEquals) \ diff --git a/Userland/Libraries/LibJS/Bytecode/Interpreter.cpp b/Userland/Libraries/LibJS/Bytecode/Interpreter.cpp index a95c980d97..33f43ac8ca 100644 --- a/Userland/Libraries/LibJS/Bytecode/Interpreter.cpp +++ b/Userland/Libraries/LibJS/Bytecode/Interpreter.cpp @@ -44,6 +44,7 @@ Interpreter::ValueAndFrame Interpreter::run_and_return_frame(Executable const& e dbgln_if(JS_BYTECODE_DEBUG, "Bytecode::Interpreter will run unit {:p}", &executable); TemporaryChange restore_executable { m_current_executable, &executable }; + TemporaryChange restore_saved_jump { m_scheduled_jump, static_cast(nullptr) }; VERIFY(m_saved_exception.is_null()); bool pushed_execution_context = false; @@ -217,7 +218,14 @@ ThrowCompletionOr Interpreter::continue_pending_unwind(Label const& resume return {}; } - jump(resume_label); + if (m_scheduled_jump) { + // FIXME: If we `break` or `continue` in the finally, we need to clear + // this field + jump(Label { *m_scheduled_jump }); + m_scheduled_jump = nullptr; + } else { + jump(resume_label); + } return {}; } diff --git a/Userland/Libraries/LibJS/Bytecode/Interpreter.h b/Userland/Libraries/LibJS/Bytecode/Interpreter.h index 0652cf260b..63068c090d 100644 --- a/Userland/Libraries/LibJS/Bytecode/Interpreter.h +++ b/Userland/Libraries/LibJS/Bytecode/Interpreter.h @@ -59,6 +59,12 @@ public: { m_pending_jump = &label.block(); } + void schedule_jump(Label const& label) + { + m_scheduled_jump = &label.block(); + VERIFY(unwind_contexts().last().finalizer); + jump(Label { *unwind_contexts().last().finalizer }); + } void do_return(Value return_value) { m_return_value = return_value; } void enter_unwind_context(Optional