LibJS: Extract most of Interpreter's run() into execute_statement()

Interpreter::run() was so far being used both as the "public API entry
point" for running a JS::Program as well as internally to execute
JS::Statement|s of all kinds - this is now more distinctly separated.
A program as returned by the parser is still going through run(), which
is responsible for creating the initial global call frame, but all other
statements are executed via execute_statement() directly.

Fixes #3437, a regression introduced by adding ASSERT(!exception()) to
run() without considering the effects that would have on internal usage.

Author: linusg

Libraries/LibJS/AST.cpp

@@ -76,7 +76,7 @@ static String get_function_name(Interpreter& interpreter, Value value)
 
 Value ScopeNode::execute(Interpreter& interpreter, GlobalObject& global_object) const
 {
-    return interpreter.run(global_object, *this);
+    return interpreter.execute_statement(global_object, *this);
 }
 
 Value FunctionDeclaration::execute(Interpreter&, GlobalObject&) const
@@ -225,10 +225,10 @@ Value IfStatement::execute(Interpreter& interpreter, GlobalObject& global_object
         return {};
 
     if (predicate_result.to_boolean())
-        return interpreter.run(global_object, *m_consequent);
+        return interpreter.execute_statement(global_object, *m_consequent);
 
     if (m_alternate)
-        return interpreter.run(global_object, *m_alternate);
+        return interpreter.execute_statement(global_object, *m_alternate);
 
     return js_undefined();
 }
@@ -239,7 +239,7 @@ Value WhileStatement::execute(Interpreter& interpreter, GlobalObject& global_obj
     while (m_test->execute(interpreter, global_object).to_boolean()) {
         if (interpreter.exception())
             return {};
-        last_value = interpreter.run(global_object, *m_body);
+        last_value = interpreter.execute_statement(global_object, *m_body);
         if (interpreter.exception())
             return {};
     }
@@ -253,7 +253,7 @@ Value DoWhileStatement::execute(Interpreter& interpreter, GlobalObject& global_o
     do {
         if (interpreter.exception())
             return {};
-        last_value = interpreter.run(global_object, *m_body);
+        last_value = interpreter.execute_statement(global_object, *m_body);
         if (interpreter.exception())
             return {};
     } while (m_test->execute(interpreter, global_object).to_boolean());
@@ -293,7 +293,7 @@ Value ForStatement::execute(Interpreter& interpreter, GlobalObject& global_objec
                 return {};
             if (!test_result.to_boolean())
                 break;
-            last_value = interpreter.run(global_object, *m_body);
+            last_value = interpreter.execute_statement(global_object, *m_body);
             if (interpreter.exception())
                 return {};
             if (interpreter.should_unwind()) {
@@ -314,7 +314,7 @@ Value ForStatement::execute(Interpreter& interpreter, GlobalObject& global_objec
         }
     } else {
         while (true) {
-            last_value = interpreter.run(global_object, *m_body);
+            last_value = interpreter.execute_statement(global_object, *m_body);
             if (interpreter.exception())
                 return {};
             if (interpreter.should_unwind()) {
@@ -381,7 +381,7 @@ Value ForInStatement::execute(Interpreter& interpreter, GlobalObject& global_obj
             interpreter.set_variable(variable_name, property_name.value_and_attributes(object).value, global_object);
             if (interpreter.exception())
                 return {};
-            last_value = interpreter.run(global_object, *m_body);
+            last_value = interpreter.execute_statement(global_object, *m_body);
             if (interpreter.exception())
                 return {};
             if (interpreter.should_unwind()) {
@@ -421,7 +421,7 @@ Value ForOfStatement::execute(Interpreter& interpreter, GlobalObject& global_obj
 
     get_iterator_values(global_object, rhs_result, [&](Value value) {
         interpreter.set_variable(variable_name, value, global_object);
-        last_value = interpreter.run(global_object, *m_body);
+        last_value = interpreter.execute_statement(global_object, *m_body);
         if (interpreter.exception())
             return IterationDecision::Break;
         if (interpreter.should_unwind()) {
@@ -1777,12 +1777,12 @@ void ThrowStatement::dump(int indent) const
 
 Value TryStatement::execute(Interpreter& interpreter, GlobalObject& global_object) const
 {
-    interpreter.run(global_object, block(), {}, ScopeType::Try);
+    interpreter.execute_statement(global_object, m_block, {}, ScopeType::Try);
     if (auto* exception = interpreter.exception()) {
         if (m_handler) {
             interpreter.clear_exception();
             ArgumentVector arguments { { m_handler->parameter(), exception->value() } };
-            interpreter.run(global_object, m_handler->body(), move(arguments));
+            interpreter.execute_statement(global_object, m_handler->body(), move(arguments));
         }
     }
 

Libraries/LibJS/Interpreter.cpp

@@ -58,23 +58,26 @@ Interpreter::~Interpreter()
 {
 }
 
-Value Interpreter::run(GlobalObject& global_object, const Statement& statement, ArgumentVector arguments, ScopeType scope_type)
+Value Interpreter::run(GlobalObject& global_object, const Program& program)
 {
     ASSERT(!exception());
 
-    if (statement.is_program()) {
-        if (m_call_stack.is_empty()) {
-            CallFrame global_call_frame;
-            global_call_frame.this_value = &global_object;
-            global_call_frame.function_name = "(global execution context)";
-            global_call_frame.environment = heap().allocate<LexicalEnvironment>(global_object, LexicalEnvironment::EnvironmentRecordType::Global);
-            global_call_frame.environment->bind_this_value(&global_object);
-            if (exception())
-                return {};
-            m_call_stack.append(move(global_call_frame));
-        }
+    if (m_call_stack.is_empty()) {
+        CallFrame global_call_frame;
+        global_call_frame.this_value = &global_object;
+        global_call_frame.function_name = "(global execution context)";
+        global_call_frame.environment = heap().allocate<LexicalEnvironment>(global_object, LexicalEnvironment::EnvironmentRecordType::Global);
+        global_call_frame.environment->bind_this_value(&global_object);
+        if (exception())
+            return {};
+        m_call_stack.append(move(global_call_frame));
     }
 
+    return program.execute(*this, global_object);
+}
+
+Value Interpreter::execute_statement(GlobalObject& global_object, const Statement& statement, ArgumentVector arguments, ScopeType scope_type)
+{
     if (!statement.is_scope_node())
         return statement.execute(*this, global_object);
 

Libraries/LibJS/Interpreter.h

@@ -100,7 +100,9 @@ class Interpreter : public Weakable<Interpreter> {
 
     ~Interpreter();
 
-    Value run(GlobalObject&, const Statement&, ArgumentVector = {}, ScopeType = ScopeType::Block);
+    Value run(GlobalObject&, const Program&);
+
+    Value execute_statement(GlobalObject&, const Statement&, ArgumentVector = {}, ScopeType = ScopeType::Block);
 
     GlobalObject& global_object();
     const GlobalObject& global_object() const;

Libraries/LibJS/Runtime/ScriptFunction.cpp

@@ -130,7 +130,7 @@ Value ScriptFunction::call(Interpreter& interpreter)
         arguments.append({ parameter.name, value });
         interpreter.current_environment()->set(parameter.name, { value, DeclarationKind::Var });
     }
-    return interpreter.run(global_object(), m_body, arguments, ScopeType::Function);
+    return interpreter.execute_statement(global_object(), m_body, arguments, ScopeType::Function);
 }
 
 Value ScriptFunction::construct(Interpreter& interpreter, Function&)

Libraries/LibJS/Tests/exception-in-catch-block.js

@@ -0,0 +1,25 @@
+test("Issue #1992, exception thrown in catch {} block", () => {
+    var tryHasBeenExecuted = false;
+    var catchHasBeenExecuted = false;
+    var finallyHasBeenExecuted = false;
+    expect(() => {
+        try {
+            tryHasBeenExecuted = true;
+            foo();
+            // execution must not reach this step
+            expect().fail();
+        } catch (e) {
+            catchHasBeenExecuted = true;
+            bar();
+            // ...also not this step
+            expect().fail();
+        } finally {
+            finallyHasBeenExecuted = true;
+        }
+        // ...or this step
+        expect().fail();
+    }).toThrow(ReferenceError, "'bar' is not defined");
+    expect(tryHasBeenExecuted).toBeTrue();
+    expect(catchHasBeenExecuted).toBeTrue();
+    expect(finallyHasBeenExecuted).toBeTrue();
+});