From ababcc5725dcae0980cb1d76f9178cbc805ba9f7 Mon Sep 17 00:00:00 2001 From: Linus Groh Date: Sun, 26 Sep 2021 16:28:50 +0200 Subject: [PATCH] LibJS: Defer execution of switch default clause until after case clauses When we encounter a default clause in a switch statement, we should not execute it immediately, instead we need to wait until all case clauses have been executed as a matching case clause can break from the switch/case. The code is nowhere close to the spec, so instead of fixing it properly I just made it slightly worse, but correct. Needs a complete refactor at some point. --- Userland/Libraries/LibJS/AST.cpp | 44 ++++++++++++++----- .../LibJS/Tests/switch-default-before-case.js | 9 ++++ 2 files changed, 41 insertions(+), 12 deletions(-) create mode 100644 Userland/Libraries/LibJS/Tests/switch-default-before-case.js diff --git a/Userland/Libraries/LibJS/AST.cpp b/Userland/Libraries/LibJS/AST.cpp index e974c232a1..90ca7d5c65 100644 --- a/Userland/Libraries/LibJS/AST.cpp +++ b/Userland/Libraries/LibJS/AST.cpp @@ -2409,31 +2409,25 @@ Value ThrowStatement::execute(Interpreter& interpreter, GlobalObject& global_obj Value SwitchStatement::execute(Interpreter& interpreter, GlobalObject& global_object) const { + // FIXME: This needs a massive refactoring, ideally once we start using continue, break, and return completions. + // Instead of having an optional test expression, SwitchCase should be split into CaseClause and DefaultClause. + // https://tc39.es/ecma262/#sec-switch-statement + InterpreterNodeScope node_scope { interpreter, *this }; auto discriminant_result = m_discriminant->execute(interpreter, global_object); if (interpreter.exception()) return {}; - bool falling_through = false; auto last_value = js_undefined(); - for (auto& switch_case : m_cases) { - if (!falling_through && switch_case.test()) { - auto test_result = switch_case.test()->execute(interpreter, global_object); - if (interpreter.exception()) - return {}; - if (!is_strictly_equal(discriminant_result, test_result)) - continue; - } - falling_through = true; - + auto execute_switch_case = [&](auto const& switch_case) -> Optional { for (auto& statement : switch_case.consequent()) { auto value = statement.execute(interpreter, global_object); if (!value.is_empty()) last_value = value; if (interpreter.exception()) - return {}; + return Value {}; if (interpreter.vm().should_unwind()) { if (interpreter.vm().should_unwind_until(ScopeType::Continuable, m_label)) { // No stop_unwind(), the outer loop will handle that - we just need to break out of the switch/case. @@ -2446,7 +2440,33 @@ Value SwitchStatement::execute(Interpreter& interpreter, GlobalObject& global_ob } } } + return {}; + }; + + bool falling_through = false; + SwitchCase const* default_switch_case = nullptr; + for (auto& switch_case : m_cases) { + if (!switch_case.test()) { + default_switch_case = &switch_case; + falling_through = true; + continue; + } + if (!falling_through) { + auto test_result = switch_case.test()->execute(interpreter, global_object); + if (interpreter.exception()) + return {}; + if (!is_strictly_equal(discriminant_result, test_result)) + continue; + } + falling_through = true; + if (auto result = execute_switch_case(switch_case); result.has_value()) + return *result; } + if (default_switch_case) { + if (auto result = execute_switch_case(*default_switch_case); result.has_value()) + return *result; + } + return last_value; } diff --git a/Userland/Libraries/LibJS/Tests/switch-default-before-case.js b/Userland/Libraries/LibJS/Tests/switch-default-before-case.js new file mode 100644 index 0000000000..409e84de8b --- /dev/null +++ b/Userland/Libraries/LibJS/Tests/switch-default-before-case.js @@ -0,0 +1,9 @@ +test("default clause before matching case clause", () => { + switch (1 + 2) { + default: + expect().fail(); + break; + case 3: + return; + } +});