LibJS: Implement non-value-producing statements properly

For various statements the spec states:

    Return NormalCompletion(empty).

In those cases we have been returning undefined so far, which is
incorrect.

In other cases it states:

    Return Completion(UpdateEmpty(stmtCompletion, undefined)).

Which essentially means a statement is evaluated and its completion
value returned if non-empty, and undefined otherwise.

While not actually noticeable in normal scripts as the VM's "last value"
can't be accessed from JS code directly (with the exception of eval(),
see below), it provided an inconsistent experience in the REPL:

    > if (true) 42;
    42
    > if (true) { 42; }
    undefined

This also fixes the case where eval() would return undefined if the last
executed statement is not a value-producing one:

    eval("1;;;;;")
    eval("1;{}")
    eval("1;var a;")

As a consequence of the changes outlined above, these now all correctly
return 1.

See https://tc39.es/ecma262/#sec-block-runtime-semantics-evaluation,
"NOTE 2".

Fixes #3609.
This commit is contained in:
Linus Groh 2021-03-16 09:12:34 +01:00 committed by Andreas Kling
parent dadf2e8251
commit c499239137
4 changed files with 40 additions and 26 deletions

View file

@ -114,7 +114,7 @@ Value FunctionDeclaration::execute(Interpreter& interpreter, GlobalObject&) cons
interpreter.enter_node(*this);
ScopeGuard exit_node { [&] { interpreter.exit_node(*this); } };
return js_undefined();
return {};
}
Value FunctionExpression::execute(Interpreter& interpreter, GlobalObject& global_object) const
@ -315,14 +315,14 @@ Value WhileStatement::execute(Interpreter& interpreter, GlobalObject& global_obj
interpreter.enter_node(*this);
ScopeGuard exit_node { [&] { interpreter.exit_node(*this); } };
Value last_value = js_undefined();
auto last_value = js_undefined();
for (;;) {
auto test_result = m_test->execute(interpreter, global_object);
if (interpreter.exception())
return {};
if (!test_result.to_boolean())
break;
last_value = interpreter.execute_statement(global_object, *m_body);
last_value = interpreter.execute_statement(global_object, *m_body).value_or(last_value);
if (interpreter.exception())
return {};
if (interpreter.vm().should_unwind()) {
@ -345,11 +345,11 @@ Value DoWhileStatement::execute(Interpreter& interpreter, GlobalObject& global_o
interpreter.enter_node(*this);
ScopeGuard exit_node { [&] { interpreter.exit_node(*this); } };
Value last_value = js_undefined();
auto last_value = js_undefined();
for (;;) {
if (interpreter.exception())
return {};
last_value = interpreter.execute_statement(global_object, *m_body);
last_value = interpreter.execute_statement(global_object, *m_body).value_or(last_value);
if (interpreter.exception())
return {};
if (interpreter.vm().should_unwind()) {
@ -392,8 +392,7 @@ Value ForStatement::execute(Interpreter& interpreter, GlobalObject& global_objec
interpreter.exit_scope(*wrapper);
});
Value last_value = js_undefined();
auto last_value = js_undefined();
if (m_init) {
m_init->execute(interpreter, global_object);
if (interpreter.exception())
@ -407,7 +406,7 @@ Value ForStatement::execute(Interpreter& interpreter, GlobalObject& global_objec
return {};
if (!test_result.to_boolean())
break;
last_value = interpreter.execute_statement(global_object, *m_body);
last_value = interpreter.execute_statement(global_object, *m_body).value_or(last_value);
if (interpreter.exception())
return {};
if (interpreter.vm().should_unwind()) {
@ -428,7 +427,7 @@ Value ForStatement::execute(Interpreter& interpreter, GlobalObject& global_objec
}
} else {
while (true) {
last_value = interpreter.execute_statement(global_object, *m_body);
last_value = interpreter.execute_statement(global_object, *m_body).value_or(last_value);
if (interpreter.exception())
return {};
if (interpreter.vm().should_unwind()) {
@ -499,7 +498,7 @@ Value ForInStatement::execute(Interpreter& interpreter, GlobalObject& global_obj
interpreter.vm().set_variable(variable_name, property_name.value_and_attributes(object).value, global_object, has_declaration);
if (interpreter.exception())
return {};
last_value = interpreter.execute_statement(global_object, *m_body);
last_value = interpreter.execute_statement(global_object, *m_body).value_or(last_value);
if (interpreter.exception())
return {};
if (interpreter.vm().should_unwind()) {
@ -543,7 +542,7 @@ Value ForOfStatement::execute(Interpreter& interpreter, GlobalObject& global_obj
get_iterator_values(global_object, rhs_result, [&](Value value) {
interpreter.vm().set_variable(variable_name, value, global_object, has_declaration);
last_value = interpreter.execute_statement(global_object, *m_body);
last_value = interpreter.execute_statement(global_object, *m_body).value_or(last_value);
if (interpreter.exception())
return IterationDecision::Break;
if (interpreter.vm().should_unwind()) {
@ -897,7 +896,7 @@ Value ClassDeclaration::execute(Interpreter& interpreter, GlobalObject& global_o
interpreter.current_scope()->put_to_scope(m_class_expression->name(), { class_constructor, DeclarationKind::Let });
return js_undefined();
return {};
}
static void print_indent(int indent)
@ -1604,7 +1603,7 @@ Value VariableDeclaration::execute(Interpreter& interpreter, GlobalObject& globa
interpreter.vm().set_variable(variable_name, initalizer_result, global_object, true);
}
}
return js_undefined();
return {};
}
Value VariableDeclarator::execute(Interpreter& interpreter, GlobalObject&) const
@ -2150,7 +2149,7 @@ Value BreakStatement::execute(Interpreter& interpreter, GlobalObject&) const
ScopeGuard exit_node { [&] { interpreter.exit_node(*this); } };
interpreter.vm().unwind(ScopeType::Breakable, m_target_label);
return js_undefined();
return {};
}
Value ContinueStatement::execute(Interpreter& interpreter, GlobalObject&) const
@ -2159,7 +2158,7 @@ Value ContinueStatement::execute(Interpreter& interpreter, GlobalObject&) const
ScopeGuard exit_node { [&] { interpreter.exit_node(*this); } };
interpreter.vm().unwind(ScopeType::Continuable, m_target_label);
return js_undefined();
return {};
}
void SwitchStatement::dump(int indent) const
@ -2247,7 +2246,7 @@ Value DebuggerStatement::execute(Interpreter& interpreter, GlobalObject&) const
ScopeGuard exit_node { [&] { interpreter.exit_node(*this); } };
// Sorry, no JavaScript debugger available (yet)!
return js_undefined();
return {};
}
void ScopeNode::add_variables(NonnullRefPtrVector<VariableDeclaration> variables)

View file

@ -91,7 +91,7 @@ public:
: Statement(move(source_range))
{
}
Value execute(Interpreter&, GlobalObject&) const override { return js_undefined(); }
Value execute(Interpreter&, GlobalObject&) const override { return {}; }
};
class ErrorStatement final : public Statement {
@ -100,7 +100,7 @@ public:
: Statement(move(source_range))
{
}
Value execute(Interpreter&, GlobalObject&) const override { return js_undefined(); }
Value execute(Interpreter&, GlobalObject&) const override { return {}; }
};
class ExpressionStatement final : public Statement {
@ -202,7 +202,7 @@ public:
: Declaration(move(source_range))
{
}
Value execute(Interpreter&, GlobalObject&) const override { return js_undefined(); }
Value execute(Interpreter&, GlobalObject&) const override { return {}; }
};
class FunctionNode {
@ -286,7 +286,7 @@ public:
{
}
Value execute(Interpreter&, GlobalObject&) const override { return js_undefined(); }
Value execute(Interpreter&, GlobalObject&) const override { return {}; }
};
class ReturnStatement final : public Statement {

View file

@ -61,6 +61,8 @@ void Interpreter::run(GlobalObject& global_object, const Program& program)
VM::InterpreterExecutionScope scope(*this);
vm.set_last_value({}, {});
CallFrame global_call_frame;
global_call_frame.current_node = &program;
global_call_frame.this_value = &global_object;
@ -73,6 +75,9 @@ void Interpreter::run(GlobalObject& global_object, const Program& program)
VERIFY(!vm.exception());
program.execute(*this, global_object);
vm.pop_call_frame();
if (vm.last_value().is_empty())
vm.set_last_value({}, js_undefined());
}
GlobalObject& Interpreter::global_object()
@ -162,11 +167,10 @@ Value Interpreter::execute_statement(GlobalObject& global_object, const Statemen
auto& block = static_cast<const ScopeNode&>(statement);
enter_scope(block, scope_type, global_object);
if (block.children().is_empty())
vm().set_last_value({}, js_undefined());
for (auto& node : block.children()) {
vm().set_last_value({}, node.execute(*this, global_object));
auto value = node.execute(*this, global_object);
if (!value.is_empty())
vm().set_last_value({}, value);
if (vm().should_unwind()) {
if (!block.label().is_null() && vm().should_unwind_until(ScopeType::Breakable, block.label()))
vm().stop_unwind();
@ -174,14 +178,18 @@ Value Interpreter::execute_statement(GlobalObject& global_object, const Statemen
}
}
bool did_return = vm().unwind_until() == ScopeType::Function;
if (scope_type == ScopeType::Function) {
bool did_return = vm().unwind_until() == ScopeType::Function;
if (!did_return)
vm().set_last_value({}, js_undefined());
}
if (vm().unwind_until() == scope_type)
vm().unwind(ScopeType::None);
exit_scope(block);
return did_return ? vm().last_value() : js_undefined();
return vm().last_value();
}
LexicalEnvironment* Interpreter::current_environment()

View file

@ -9,6 +9,13 @@ test("basic eval() functionality", () => {
expect(foo(7)).toBe(12);
});
test("returns value of last value-producing statement", () => {
// See https://tc39.es/ecma262/#sec-block-runtime-semantics-evaluation
expect(eval("1;;;;;")).toBe(1);
expect(eval("1;{}")).toBe(1);
expect(eval("1;var a;")).toBe(1);
});
test("syntax error", () => {
expect(() => {
eval("{");