From 9ea6ef4ed1450590ad02f6d501c7345eab779439 Mon Sep 17 00:00:00 2001 From: Linus Groh Date: Tue, 25 Aug 2020 12:52:32 +0200 Subject: [PATCH] LibJS: Make Interpreter::throw_exception() a void function The motivation for this change is twofold: - Returning a JS::Value is misleading as one would expect it to carry some meaningful information, like maybe the error object that's being created, but in fact it is always empty. Supposedly to serve as a shortcut for the common case of "throw and return empty value", but that's just leading us to my second point. - Inconsistent usage / coding style: as of this commit there are 114 uses of throw_exception() discarding its return value and 55 uses directly returning the call result (in LibJS, not counting LibWeb); with the first style often having a more explicit empty value (or nullptr in some cases) return anyway. One more line to always make the return value obvious is should be worth it. So now it's basically always these steps, which is already being used in the majority of cases (as outlined above): - Throw an exception. This mutates interpreter state by updating m_exception and unwinding, but doesn't return anything. - Let the caller explicitly return an empty value, nullptr or anything else itself. --- Applications/Spreadsheet/Spreadsheet.cpp | 14 ++-- Libraries/LibJS/AST.cpp | 42 ++++++----- Libraries/LibJS/Interpreter.cpp | 3 +- Libraries/LibJS/Interpreter.h | 8 +- .../LibJS/Runtime/ArrayIteratorPrototype.cpp | 8 +- Libraries/LibJS/Runtime/ArrayPrototype.cpp | 12 ++- Libraries/LibJS/Runtime/ErrorPrototype.cpp | 22 ++++-- Libraries/LibJS/Runtime/FunctionPrototype.cpp | 39 ++++++---- .../LibJS/Runtime/LexicalEnvironment.cpp | 7 +- Libraries/LibJS/Runtime/NumberPrototype.cpp | 9 ++- Libraries/LibJS/Runtime/ObjectConstructor.cpp | 38 ++++++---- Libraries/LibJS/Runtime/ProxyConstructor.cpp | 22 ++++-- Libraries/LibJS/Runtime/ProxyObject.cpp | 69 ++++++++++------- Libraries/LibJS/Runtime/ReflectObject.cpp | 6 +- Libraries/LibJS/Runtime/ScriptFunction.cpp | 6 +- .../LibJS/Runtime/StringIteratorPrototype.cpp | 6 +- Libraries/LibJS/Runtime/StringPrototype.cpp | 18 +++-- Libraries/LibJS/Runtime/Uint8ClampedArray.cpp | 6 +- Libraries/LibJS/Runtime/Value.cpp | 34 +++++---- Libraries/LibWeb/Bindings/WindowObject.cpp | 74 ++++++++++++------- .../CodeGenerators/WrapperGenerator.cpp | 9 ++- 21 files changed, 281 insertions(+), 171 deletions(-) diff --git a/Applications/Spreadsheet/Spreadsheet.cpp b/Applications/Spreadsheet/Spreadsheet.cpp index e255f3a6dc..e40cb983ca 100644 --- a/Applications/Spreadsheet/Spreadsheet.cpp +++ b/Applications/Spreadsheet/Spreadsheet.cpp @@ -88,13 +88,15 @@ public: static JS_DEFINE_NATIVE_FUNCTION(parse_cell_name) { - if (interpreter.argument_count() != 1) - return interpreter.throw_exception("Expected exactly one argument to parse_cell_name()"); - + if (interpreter.argument_count() != 1) { + interpreter.throw_exception("Expected exactly one argument to parse_cell_name()"); + return {}; + } auto name_value = interpreter.argument(0); - if (!name_value.is_string()) - return interpreter.throw_exception("Expected a String argument to parse_cell_name()"); - + if (!name_value.is_string()) { + interpreter.throw_exception("Expected a String argument to parse_cell_name()"); + return {}; + } auto position = Sheet::parse_cell_name(name_value.as_string().string()); if (!position.has_value()) return JS::js_undefined(); diff --git a/Libraries/LibJS/AST.cpp b/Libraries/LibJS/AST.cpp index 1d440fcf17..70e1992a79 100644 --- a/Libraries/LibJS/AST.cpp +++ b/Libraries/LibJS/AST.cpp @@ -147,10 +147,11 @@ Value CallExpression::execute(Interpreter& interpreter, GlobalObject& global_obj } else { expression_string = static_cast(*m_callee).to_string_approximation(); } - return interpreter.throw_exception(ErrorType::IsNotAEvaluatedFrom, callee.to_string_without_side_effects().characters(), call_type, expression_string.characters()); + interpreter.throw_exception(ErrorType::IsNotAEvaluatedFrom, callee.to_string_without_side_effects().characters(), call_type, expression_string.characters()); } else { - return interpreter.throw_exception(ErrorType::IsNotA, callee.to_string_without_side_effects().characters(), call_type); + interpreter.throw_exception(ErrorType::IsNotA, callee.to_string_without_side_effects().characters(), call_type); } + return {}; } auto& function = callee.as_function(); @@ -184,9 +185,10 @@ Value CallExpression::execute(Interpreter& interpreter, GlobalObject& global_obj } else if (m_callee->is_super_expression()) { auto* super_constructor = interpreter.current_environment()->current_function()->prototype(); // FIXME: Functions should track their constructor kind. - if (!super_constructor || !super_constructor->is_function()) - return interpreter.throw_exception(ErrorType::NotAConstructor, "Super constructor"); - + if (!super_constructor || !super_constructor->is_function()) { + interpreter.throw_exception(ErrorType::NotAConstructor, "Super constructor"); + return {}; + } result = interpreter.construct(static_cast(*super_constructor), function, move(arguments), global_object); if (interpreter.exception()) return {}; @@ -668,9 +670,10 @@ Value ClassExpression::execute(Interpreter& interpreter, GlobalObject& global_ob super_constructor = m_super_class->execute(interpreter, global_object); if (interpreter.exception()) return {}; - if (!super_constructor.is_function() && !super_constructor.is_null()) - return interpreter.throw_exception(ErrorType::ClassDoesNotExtendAConstructorOrNull, super_constructor.to_string_without_side_effects().characters()); - + if (!super_constructor.is_function() && !super_constructor.is_null()) { + interpreter.throw_exception(ErrorType::ClassDoesNotExtendAConstructorOrNull, super_constructor.to_string_without_side_effects().characters()); + return {}; + } class_constructor->set_constructor_kind(Function::ConstructorKind::Derived); Object* prototype = Object::create_empty(global_object); @@ -695,9 +698,10 @@ Value ClassExpression::execute(Interpreter& interpreter, GlobalObject& global_ob if (interpreter.exception()) return {}; - if (!class_prototype.is_object()) - return interpreter.throw_exception(ErrorType::NotAnObject, "Class prototype"); - + if (!class_prototype.is_object()) { + interpreter.throw_exception(ErrorType::NotAnObject, "Class prototype"); + return {}; + } for (const auto& method : m_methods) { auto method_value = method.execute(interpreter, global_object); if (interpreter.exception()) @@ -1137,8 +1141,10 @@ void ForOfStatement::dump(int indent) const Value Identifier::execute(Interpreter& interpreter, GlobalObject& global_object) const { auto value = interpreter.get_variable(string(), global_object); - if (value.is_empty()) - return interpreter.throw_exception(ErrorType::UnknownIdentifier, string().characters()); + if (value.is_empty()) { + interpreter.throw_exception(ErrorType::UnknownIdentifier, string().characters()); + return {}; + } return value; } @@ -1259,9 +1265,10 @@ Value AssignmentExpression::execute(Interpreter& interpreter, GlobalObject& glob if (interpreter.exception()) return {}; - if (reference.is_unresolvable()) - return interpreter.throw_exception(ErrorType::InvalidLeftHandAssignment); - + if (reference.is_unresolvable()) { + interpreter.throw_exception(ErrorType::InvalidLeftHandAssignment); + return {}; + } update_function_name(rhs_result, get_function_name(interpreter, reference.name().to_value(interpreter))); reference.put(interpreter, global_object, rhs_result); @@ -1797,7 +1804,8 @@ Value ThrowStatement::execute(Interpreter& interpreter, GlobalObject& global_obj auto value = m_argument->execute(interpreter, global_object); if (interpreter.exception()) return {}; - return interpreter.throw_exception(value); + interpreter.throw_exception(value); + return {}; } Value SwitchStatement::execute(Interpreter& interpreter, GlobalObject& global_object) const diff --git a/Libraries/LibJS/Interpreter.cpp b/Libraries/LibJS/Interpreter.cpp index a795b5126b..f301040dfe 100644 --- a/Libraries/LibJS/Interpreter.cpp +++ b/Libraries/LibJS/Interpreter.cpp @@ -324,7 +324,7 @@ Value Interpreter::construct(Function& function, Function& new_target, Optional< return this_value; } -Value Interpreter::throw_exception(Exception* exception) +void Interpreter::throw_exception(Exception* exception) { #ifdef INTERPRETER_DEBUG if (exception->value().is_object() && exception->value().as_object().is_error()) { @@ -341,7 +341,6 @@ Value Interpreter::throw_exception(Exception* exception) #endif m_exception = exception; unwind(ScopeType::Try); - return {}; } GlobalObject& Interpreter::global_object() diff --git a/Libraries/LibJS/Interpreter.h b/Libraries/LibJS/Interpreter.h index b18c490a79..cd5a1f33b4 100644 --- a/Libraries/LibJS/Interpreter.h +++ b/Libraries/LibJS/Interpreter.h @@ -176,19 +176,19 @@ public: void clear_exception() { m_exception = nullptr; } template - Value throw_exception(Args&&... args) + void throw_exception(Args&&... args) { return throw_exception(T::create(global_object(), forward(args)...)); } - Value throw_exception(Exception*); - Value throw_exception(Value value) + void throw_exception(Exception*); + void throw_exception(Value value) { return throw_exception(heap().allocate(global_object(), value)); } template - Value throw_exception(ErrorType type, Args&&... args) + void throw_exception(ErrorType type, Args&&... args) { return throw_exception(T::create(global_object(), String::format(type.message(), forward(args)...))); } diff --git a/Libraries/LibJS/Runtime/ArrayIteratorPrototype.cpp b/Libraries/LibJS/Runtime/ArrayIteratorPrototype.cpp index 1c7085fa89..fd10aec49c 100644 --- a/Libraries/LibJS/Runtime/ArrayIteratorPrototype.cpp +++ b/Libraries/LibJS/Runtime/ArrayIteratorPrototype.cpp @@ -53,11 +53,11 @@ ArrayIteratorPrototype::~ArrayIteratorPrototype() JS_DEFINE_NATIVE_FUNCTION(ArrayIteratorPrototype::next) { auto this_value = interpreter.this_value(global_object); - if (!this_value.is_object() || !this_value.as_object().is_array_iterator_object()) - return interpreter.throw_exception(ErrorType::NotAn, "Array Iterator"); - + if (!this_value.is_object() || !this_value.as_object().is_array_iterator_object()) { + interpreter.throw_exception(ErrorType::NotAn, "Array Iterator"); + return {}; + } auto& this_object = this_value.as_object(); - auto& iterator = static_cast(this_object); auto target_array = iterator.array(); if (target_array.is_undefined()) diff --git a/Libraries/LibJS/Runtime/ArrayPrototype.cpp b/Libraries/LibJS/Runtime/ArrayPrototype.cpp index c3bb9dac62..38a17ebdca 100644 --- a/Libraries/LibJS/Runtime/ArrayPrototype.cpp +++ b/Libraries/LibJS/Runtime/ArrayPrototype.cpp @@ -204,8 +204,10 @@ JS_DEFINE_NATIVE_FUNCTION(ArrayPrototype::push) return {}; auto argument_count = interpreter.argument_count(); auto new_length = length + argument_count; - if (new_length > MAX_ARRAY_LIKE_INDEX) - return interpreter.throw_exception(ErrorType::ArrayMaxSize); + if (new_length > MAX_ARRAY_LIKE_INDEX) { + interpreter.throw_exception(ErrorType::ArrayMaxSize); + return {}; + } for (size_t i = 0; i < argument_count; ++i) { this_object->put(length + i, interpreter.argument(i)); if (interpreter.exception()) @@ -744,8 +746,10 @@ JS_DEFINE_NATIVE_FUNCTION(ArrayPrototype::splice) size_t new_length = initial_length + insert_count - actual_delete_count; - if (new_length > MAX_ARRAY_LIKE_INDEX) - return interpreter.throw_exception(ErrorType::ArrayMaxSize); + if (new_length > MAX_ARRAY_LIKE_INDEX) { + interpreter.throw_exception(ErrorType::ArrayMaxSize); + return {}; + } auto removed_elements = Array::create(global_object); diff --git a/Libraries/LibJS/Runtime/ErrorPrototype.cpp b/Libraries/LibJS/Runtime/ErrorPrototype.cpp index 785c65f1ef..5f70f0d36d 100644 --- a/Libraries/LibJS/Runtime/ErrorPrototype.cpp +++ b/Libraries/LibJS/Runtime/ErrorPrototype.cpp @@ -58,8 +58,10 @@ JS_DEFINE_NATIVE_GETTER(ErrorPrototype::name_getter) auto* this_object = interpreter.this_value(global_object).to_object(interpreter, global_object); if (!this_object) return {}; - if (!this_object->is_error()) - return interpreter.throw_exception(ErrorType::NotAn, "Error"); + if (!this_object->is_error()) { + interpreter.throw_exception(ErrorType::NotAn, "Error"); + return {}; + } return js_string(interpreter, static_cast(this_object)->name()); } @@ -83,15 +85,19 @@ JS_DEFINE_NATIVE_GETTER(ErrorPrototype::message_getter) auto* this_object = interpreter.this_value(global_object).to_object(interpreter, global_object); if (!this_object) return {}; - if (!this_object->is_error()) - return interpreter.throw_exception(ErrorType::NotAn, "Error"); + if (!this_object->is_error()) { + interpreter.throw_exception(ErrorType::NotAn, "Error"); + return {}; + } return js_string(interpreter, static_cast(this_object)->message()); } JS_DEFINE_NATIVE_FUNCTION(ErrorPrototype::to_string) { - if (!interpreter.this_value(global_object).is_object()) - return interpreter.throw_exception(ErrorType::NotAnObject, interpreter.this_value(global_object).to_string_without_side_effects().characters()); + if (!interpreter.this_value(global_object).is_object()) { + interpreter.throw_exception(ErrorType::NotAnObject, interpreter.this_value(global_object).to_string_without_side_effects().characters()); + return {}; + } auto& this_object = interpreter.this_value(global_object).as_object(); String name = "Error"; @@ -123,10 +129,10 @@ JS_DEFINE_NATIVE_FUNCTION(ErrorPrototype::to_string) #define __JS_ENUMERATE(ClassName, snake_name, PrototypeName, ConstructorName) \ PrototypeName::PrototypeName(GlobalObject& global_object) \ - : Object(*global_object.error_prototype()) \ + : Object(*global_object.error_prototype()) \ { \ } \ - PrototypeName::~PrototypeName() { } + PrototypeName::~PrototypeName() {} JS_ENUMERATE_ERROR_SUBCLASSES #undef __JS_ENUMERATE diff --git a/Libraries/LibJS/Runtime/FunctionPrototype.cpp b/Libraries/LibJS/Runtime/FunctionPrototype.cpp index d3f7a1f935..f1bab379d5 100644 --- a/Libraries/LibJS/Runtime/FunctionPrototype.cpp +++ b/Libraries/LibJS/Runtime/FunctionPrototype.cpp @@ -65,15 +65,19 @@ JS_DEFINE_NATIVE_FUNCTION(FunctionPrototype::apply) auto* this_object = interpreter.this_value(global_object).to_object(interpreter, global_object); if (!this_object) return {}; - if (!this_object->is_function()) - return interpreter.throw_exception(ErrorType::NotA, "Function"); + if (!this_object->is_function()) { + interpreter.throw_exception(ErrorType::NotA, "Function"); + return {}; + } auto& function = static_cast(*this_object); auto this_arg = interpreter.argument(0); auto arg_array = interpreter.argument(1); if (arg_array.is_null() || arg_array.is_undefined()) return interpreter.call(function, this_arg); - if (!arg_array.is_object()) - return interpreter.throw_exception(ErrorType::FunctionArgsNotObject); + if (!arg_array.is_object()) { + interpreter.throw_exception(ErrorType::FunctionArgsNotObject); + return {}; + } auto length_property = arg_array.as_object().get("length"); if (interpreter.exception()) return {}; @@ -95,9 +99,10 @@ JS_DEFINE_NATIVE_FUNCTION(FunctionPrototype::bind) auto* this_object = interpreter.this_value(global_object).to_object(interpreter, global_object); if (!this_object) return {}; - if (!this_object->is_function()) - return interpreter.throw_exception(ErrorType::NotA, "Function"); - + if (!this_object->is_function()) { + interpreter.throw_exception(ErrorType::NotA, "Function"); + return {}; + } auto& this_function = static_cast(*this_object); auto bound_this_arg = interpreter.argument(0); @@ -115,8 +120,10 @@ JS_DEFINE_NATIVE_FUNCTION(FunctionPrototype::call) auto* this_object = interpreter.this_value(global_object).to_object(interpreter, global_object); if (!this_object) return {}; - if (!this_object->is_function()) - return interpreter.throw_exception(ErrorType::NotA, "Function"); + if (!this_object->is_function()) { + interpreter.throw_exception(ErrorType::NotA, "Function"); + return {}; + } auto& function = static_cast(*this_object); auto this_arg = interpreter.argument(0); MarkedValueList arguments(interpreter.heap()); @@ -132,9 +139,10 @@ JS_DEFINE_NATIVE_FUNCTION(FunctionPrototype::to_string) auto* this_object = interpreter.this_value(global_object).to_object(interpreter, global_object); if (!this_object) return {}; - if (!this_object->is_function()) - return interpreter.throw_exception(ErrorType::NotA, "Function"); - + if (!this_object->is_function()) { + interpreter.throw_exception(ErrorType::NotA, "Function"); + return {}; + } String function_name = static_cast(this_object)->name(); String function_parameters = ""; String function_body; @@ -173,9 +181,10 @@ JS_DEFINE_NATIVE_FUNCTION(FunctionPrototype::symbol_has_instance) auto* this_object = interpreter.this_value(global_object).to_object(interpreter, global_object); if (!this_object) return {}; - if (!this_object->is_function()) - return interpreter.throw_exception(ErrorType::NotA, "Function"); - + if (!this_object->is_function()) { + interpreter.throw_exception(ErrorType::NotA, "Function"); + return {}; + } return ordinary_has_instance(interpreter, interpreter.argument(0), this_object); } diff --git a/Libraries/LibJS/Runtime/LexicalEnvironment.cpp b/Libraries/LibJS/Runtime/LexicalEnvironment.cpp index 0dae1029ce..813e52ddf5 100644 --- a/Libraries/LibJS/Runtime/LexicalEnvironment.cpp +++ b/Libraries/LibJS/Runtime/LexicalEnvironment.cpp @@ -112,9 +112,10 @@ bool LexicalEnvironment::has_this_binding() const Value LexicalEnvironment::get_this_binding() const { ASSERT(has_this_binding()); - if (this_binding_status() == ThisBindingStatus::Uninitialized) - return interpreter().throw_exception(ErrorType::ThisHasNotBeenInitialized); - + if (this_binding_status() == ThisBindingStatus::Uninitialized) { + interpreter().throw_exception(ErrorType::ThisHasNotBeenInitialized); + return {}; + } return m_this_value; } diff --git a/Libraries/LibJS/Runtime/NumberPrototype.cpp b/Libraries/LibJS/Runtime/NumberPrototype.cpp index 121c16ba51..0f7b0c8e3b 100644 --- a/Libraries/LibJS/Runtime/NumberPrototype.cpp +++ b/Libraries/LibJS/Runtime/NumberPrototype.cpp @@ -67,7 +67,8 @@ JS_DEFINE_NATIVE_FUNCTION(NumberPrototype::to_string) } else if (this_value.is_object() && this_value.as_object().is_number_object()) { number_value = static_cast(this_value.as_object()).value_of(); } else { - return interpreter.throw_exception(ErrorType::NumberIncompatibleThis, "toString"); + interpreter.throw_exception(ErrorType::NumberIncompatibleThis, "toString"); + return {}; } int radix; @@ -78,8 +79,10 @@ JS_DEFINE_NATIVE_FUNCTION(NumberPrototype::to_string) radix = argument.to_i32(interpreter); } - if (interpreter.exception() || radix < 2 || radix > 36) - return interpreter.throw_exception(ErrorType::InvalidRadix); + if (interpreter.exception() || radix < 2 || radix > 36) { + interpreter.throw_exception(ErrorType::InvalidRadix); + return {}; + } if (number_value.is_positive_infinity()) return js_string(interpreter, "Infinity"); diff --git a/Libraries/LibJS/Runtime/ObjectConstructor.cpp b/Libraries/LibJS/Runtime/ObjectConstructor.cpp index 5489714e36..7c09c7da6e 100644 --- a/Libraries/LibJS/Runtime/ObjectConstructor.cpp +++ b/Libraries/LibJS/Runtime/ObjectConstructor.cpp @@ -105,8 +105,10 @@ JS_DEFINE_NATIVE_FUNCTION(ObjectConstructor::get_prototype_of) JS_DEFINE_NATIVE_FUNCTION(ObjectConstructor::set_prototype_of) { - if (interpreter.argument_count() < 2) - return interpreter.throw_exception(ErrorType::ObjectSetPrototypeOfTwoArgs); + if (interpreter.argument_count() < 2) { + interpreter.throw_exception(ErrorType::ObjectSetPrototypeOfTwoArgs); + return {}; + } auto* object = interpreter.argument(0).to_object(interpreter, global_object); if (interpreter.exception()) return {}; @@ -162,10 +164,14 @@ JS_DEFINE_NATIVE_FUNCTION(ObjectConstructor::get_own_property_descriptor) JS_DEFINE_NATIVE_FUNCTION(ObjectConstructor::define_property_) { - if (!interpreter.argument(0).is_object()) - return interpreter.throw_exception(ErrorType::NotAnObject, "Object argument"); - if (!interpreter.argument(2).is_object()) - return interpreter.throw_exception(ErrorType::NotAnObject, "Descriptor argument"); + if (!interpreter.argument(0).is_object()) { + interpreter.throw_exception(ErrorType::NotAnObject, "Object argument"); + return {}; + } + if (!interpreter.argument(2).is_object()) { + interpreter.throw_exception(ErrorType::NotAnObject, "Descriptor argument"); + return {}; + } auto& object = interpreter.argument(0).as_object(); auto property_key = StringOrSymbol::from_value(interpreter, interpreter.argument(1)); if (interpreter.exception()) @@ -191,8 +197,10 @@ JS_DEFINE_NATIVE_FUNCTION(ObjectConstructor::is) JS_DEFINE_NATIVE_FUNCTION(ObjectConstructor::keys) { - if (!interpreter.argument_count()) - return interpreter.throw_exception(ErrorType::ConvertUndefinedToObject); + if (!interpreter.argument_count()) { + interpreter.throw_exception(ErrorType::ConvertUndefinedToObject); + return {}; + } auto* obj_arg = interpreter.argument(0).to_object(interpreter, global_object); if (interpreter.exception()) @@ -203,9 +211,10 @@ JS_DEFINE_NATIVE_FUNCTION(ObjectConstructor::keys) JS_DEFINE_NATIVE_FUNCTION(ObjectConstructor::values) { - if (!interpreter.argument_count()) - return interpreter.throw_exception(ErrorType::ConvertUndefinedToObject); - + if (!interpreter.argument_count()) { + interpreter.throw_exception(ErrorType::ConvertUndefinedToObject); + return {}; + } auto* obj_arg = interpreter.argument(0).to_object(interpreter, global_object); if (interpreter.exception()) return {}; @@ -215,9 +224,10 @@ JS_DEFINE_NATIVE_FUNCTION(ObjectConstructor::values) JS_DEFINE_NATIVE_FUNCTION(ObjectConstructor::entries) { - if (!interpreter.argument_count()) - return interpreter.throw_exception(ErrorType::ConvertUndefinedToObject); - + if (!interpreter.argument_count()) { + interpreter.throw_exception(ErrorType::ConvertUndefinedToObject); + return {}; + } auto* obj_arg = interpreter.argument(0).to_object(interpreter, global_object); if (interpreter.exception()) return {}; diff --git a/Libraries/LibJS/Runtime/ProxyConstructor.cpp b/Libraries/LibJS/Runtime/ProxyConstructor.cpp index 299d1eac8d..c5b76307d5 100644 --- a/Libraries/LibJS/Runtime/ProxyConstructor.cpp +++ b/Libraries/LibJS/Runtime/ProxyConstructor.cpp @@ -51,22 +51,28 @@ ProxyConstructor::~ProxyConstructor() Value ProxyConstructor::call(Interpreter& interpreter) { - return interpreter.throw_exception(ErrorType::ProxyCallWithNew); + interpreter.throw_exception(ErrorType::ProxyCallWithNew); + return {}; } Value ProxyConstructor::construct(Interpreter& interpreter, Function&) { - if (interpreter.argument_count() < 2) - return interpreter.throw_exception(ErrorType::ProxyTwoArguments); + if (interpreter.argument_count() < 2) { + interpreter.throw_exception(ErrorType::ProxyTwoArguments); + return {}; + } auto target = interpreter.argument(0); auto handler = interpreter.argument(1); - if (!target.is_object()) - return interpreter.throw_exception(ErrorType::ProxyConstructorBadType, "target", target.to_string_without_side_effects().characters()); - if (!handler.is_object()) - return interpreter.throw_exception(ErrorType::ProxyConstructorBadType, "handler", handler.to_string_without_side_effects().characters()); - + if (!target.is_object()) { + interpreter.throw_exception(ErrorType::ProxyConstructorBadType, "target", target.to_string_without_side_effects().characters()); + return {}; + } + if (!handler.is_object()) { + interpreter.throw_exception(ErrorType::ProxyConstructorBadType, "handler", handler.to_string_without_side_effects().characters()); + return {}; + } return ProxyObject::create(global_object(), target.as_object(), handler.as_object()); } diff --git a/Libraries/LibJS/Runtime/ProxyObject.cpp b/Libraries/LibJS/Runtime/ProxyObject.cpp index 6d532d03a8..a609f2e356 100644 --- a/Libraries/LibJS/Runtime/ProxyObject.cpp +++ b/Libraries/LibJS/Runtime/ProxyObject.cpp @@ -375,8 +375,10 @@ Value ProxyObject::get(const PropertyName& name, Value) const return {}; if (trap.is_empty() || trap.is_undefined() || trap.is_null()) return m_target.get(name); - if (!trap.is_function()) - return interpreter().throw_exception(ErrorType::ProxyInvalidTrap, "get"); + if (!trap.is_function()) { + interpreter().throw_exception(ErrorType::ProxyInvalidTrap, "get"); + return {}; + } MarkedValueList arguments(interpreter().heap()); arguments.append(Value(&m_target)); arguments.append(js_string(interpreter(), name.to_string())); @@ -388,10 +390,14 @@ Value ProxyObject::get(const PropertyName& name, Value) const if (target_desc.has_value()) { if (interpreter().exception()) return {}; - if (target_desc.value().is_data_descriptor() && !target_desc.value().attributes.is_writable() && !same_value(interpreter(), trap_result, target_desc.value().value)) - return interpreter().throw_exception(ErrorType::ProxyGetImmutableDataProperty); - if (target_desc.value().is_accessor_descriptor() && target_desc.value().getter == nullptr && !trap_result.is_undefined()) - return interpreter().throw_exception(ErrorType::ProxyGetNonConfigurableAccessor); + if (target_desc.value().is_data_descriptor() && !target_desc.value().attributes.is_writable() && !same_value(interpreter(), trap_result, target_desc.value().value)) { + interpreter().throw_exception(ErrorType::ProxyGetImmutableDataProperty); + return {}; + } + if (target_desc.value().is_accessor_descriptor() && target_desc.value().getter == nullptr && !trap_result.is_undefined()) { + interpreter().throw_exception(ErrorType::ProxyGetNonConfigurableAccessor); + return {}; + } } return trap_result; } @@ -445,8 +451,10 @@ Value ProxyObject::delete_property(const PropertyName& name) return {}; if (trap.is_empty() || trap.is_undefined() || trap.is_null()) return m_target.delete_property(name); - if (!trap.is_function()) - return interpreter().throw_exception(ErrorType::ProxyInvalidTrap, "deleteProperty"); + if (!trap.is_function()) { + interpreter().throw_exception(ErrorType::ProxyInvalidTrap, "deleteProperty"); + return {}; + } MarkedValueList arguments(interpreter().heap()); arguments.append(Value(&m_target)); arguments.append(js_string(interpreter(), name.to_string())); @@ -460,8 +468,10 @@ Value ProxyObject::delete_property(const PropertyName& name) return {}; if (!target_desc.has_value()) return Value(true); - if (!target_desc.value().attributes.is_configurable()) - return interpreter().throw_exception(ErrorType::ProxyDeleteNonConfigurable); + if (!target_desc.value().attributes.is_configurable()) { + interpreter().throw_exception(ErrorType::ProxyDeleteNonConfigurable); + return {}; + } return Value(true); } @@ -472,10 +482,12 @@ void ProxyObject::visit_children(Cell::Visitor& visitor) visitor.visit(&m_handler); } -Value ProxyObject::call(Interpreter& interpreter) { - if (!is_function()) - return interpreter.throw_exception(ErrorType::NotAFunction, Value(this).to_string_without_side_effects().characters()); - +Value ProxyObject::call(Interpreter& interpreter) +{ + if (!is_function()) { + interpreter.throw_exception(ErrorType::NotAFunction, Value(this).to_string_without_side_effects().characters()); + return {}; + } if (m_is_revoked) { interpreter.throw_exception(ErrorType::ProxyRevoked); return {}; @@ -485,9 +497,10 @@ Value ProxyObject::call(Interpreter& interpreter) { return {}; if (trap.is_empty() || trap.is_undefined() || trap.is_null()) return static_cast(m_target).call(interpreter); - if (!trap.is_function()) - return interpreter.throw_exception(ErrorType::ProxyInvalidTrap, "apply"); - + if (!trap.is_function()) { + interpreter.throw_exception(ErrorType::ProxyInvalidTrap, "apply"); + return {}; + } MarkedValueList arguments(interpreter.heap()); arguments.append(Value(&m_target)); arguments.append(Value(&m_handler)); @@ -501,10 +514,12 @@ Value ProxyObject::call(Interpreter& interpreter) { return interpreter.call(trap.as_function(), Value(&m_handler), move(arguments)); } -Value ProxyObject::construct(Interpreter& interpreter, Function& new_target) { - if (!is_function()) - return interpreter.throw_exception(ErrorType::NotAConstructor, Value(this).to_string_without_side_effects().characters()); - +Value ProxyObject::construct(Interpreter& interpreter, Function& new_target) +{ + if (!is_function()) { + interpreter.throw_exception(ErrorType::NotAConstructor, Value(this).to_string_without_side_effects().characters()); + return {}; + } if (m_is_revoked) { interpreter.throw_exception(ErrorType::ProxyRevoked); return {}; @@ -514,8 +529,10 @@ Value ProxyObject::construct(Interpreter& interpreter, Function& new_target) { return {}; if (trap.is_empty() || trap.is_undefined() || trap.is_null()) return static_cast(m_target).construct(interpreter, new_target); - if (!trap.is_function()) - return interpreter.throw_exception(ErrorType::ProxyInvalidTrap, "construct"); + if (!trap.is_function()) { + interpreter.throw_exception(ErrorType::ProxyInvalidTrap, "construct"); + return {}; + } MarkedValueList arguments(interpreter.heap()); arguments.append(Value(&m_target)); @@ -526,8 +543,10 @@ Value ProxyObject::construct(Interpreter& interpreter, Function& new_target) { arguments.append(arguments_array); arguments.append(Value(&new_target)); auto result = interpreter.call(trap.as_function(), Value(&m_handler), move(arguments)); - if (!result.is_object()) - return interpreter.throw_exception(ErrorType::ProxyConstructBadReturnType); + if (!result.is_object()) { + interpreter.throw_exception(ErrorType::ProxyConstructBadReturnType); + return {}; + } return result; } diff --git a/Libraries/LibJS/Runtime/ReflectObject.cpp b/Libraries/LibJS/Runtime/ReflectObject.cpp index 12362c99e3..ec667690a8 100644 --- a/Libraries/LibJS/Runtime/ReflectObject.cpp +++ b/Libraries/LibJS/Runtime/ReflectObject.cpp @@ -143,8 +143,10 @@ JS_DEFINE_NATIVE_FUNCTION(ReflectObject::define_property) auto* target = get_target_object_from(interpreter, "defineProperty"); if (!target) return {}; - if (!interpreter.argument(2).is_object()) - return interpreter.throw_exception(ErrorType::ReflectBadDescriptorArgument); + if (!interpreter.argument(2).is_object()) { + interpreter.throw_exception(ErrorType::ReflectBadDescriptorArgument); + return {}; + } auto property_key = StringOrSymbol::from_value(interpreter, interpreter.argument(1)); if (interpreter.exception()) return {}; diff --git a/Libraries/LibJS/Runtime/ScriptFunction.cpp b/Libraries/LibJS/Runtime/ScriptFunction.cpp index 7a7a78dc92..1a5b607de9 100644 --- a/Libraries/LibJS/Runtime/ScriptFunction.cpp +++ b/Libraries/LibJS/Runtime/ScriptFunction.cpp @@ -135,8 +135,10 @@ Value ScriptFunction::call(Interpreter& interpreter) Value ScriptFunction::construct(Interpreter& interpreter, Function&) { - if (m_is_arrow_function) - return interpreter.throw_exception(ErrorType::NotAConstructor, m_name.characters()); + if (m_is_arrow_function) { + interpreter.throw_exception(ErrorType::NotAConstructor, m_name.characters()); + return {}; + } return call(interpreter); } diff --git a/Libraries/LibJS/Runtime/StringIteratorPrototype.cpp b/Libraries/LibJS/Runtime/StringIteratorPrototype.cpp index 15b3f15716..858601f68d 100644 --- a/Libraries/LibJS/Runtime/StringIteratorPrototype.cpp +++ b/Libraries/LibJS/Runtime/StringIteratorPrototype.cpp @@ -53,8 +53,10 @@ StringIteratorPrototype::~StringIteratorPrototype() JS_DEFINE_NATIVE_FUNCTION(StringIteratorPrototype::next) { auto this_value = interpreter.this_value(global_object); - if (!this_value.is_object() || !this_value.as_object().is_string_iterator_object()) - return interpreter.throw_exception(ErrorType::NotA, "String Iterator"); + if (!this_value.is_object() || !this_value.as_object().is_string_iterator_object()) { + interpreter.throw_exception(ErrorType::NotA, "String Iterator"); + return {}; + } auto& this_object = this_value.as_object(); auto& iterator = static_cast(this_object); diff --git a/Libraries/LibJS/Runtime/StringPrototype.cpp b/Libraries/LibJS/Runtime/StringPrototype.cpp index 1b00208d30..2b66771df0 100644 --- a/Libraries/LibJS/Runtime/StringPrototype.cpp +++ b/Libraries/LibJS/Runtime/StringPrototype.cpp @@ -140,10 +140,14 @@ JS_DEFINE_NATIVE_FUNCTION(StringPrototype::repeat) auto count_value = interpreter.argument(0).to_number(interpreter); if (interpreter.exception()) return {}; - if (count_value.as_double() < 0) - return interpreter.throw_exception(ErrorType::StringRepeatCountMustBe, "positive"); - if (count_value.is_infinity()) - return interpreter.throw_exception(ErrorType::StringRepeatCountMustBe, "finite"); + if (count_value.as_double() < 0) { + interpreter.throw_exception(ErrorType::StringRepeatCountMustBe, "positive"); + return {}; + } + if (count_value.is_infinity()) { + interpreter.throw_exception(ErrorType::StringRepeatCountMustBe, "finite"); + return {}; + } auto count = count_value.to_size_t(interpreter); if (interpreter.exception()) return {}; @@ -455,8 +459,10 @@ JS_DEFINE_NATIVE_FUNCTION(StringPrototype::last_index_of) JS_DEFINE_NATIVE_FUNCTION(StringPrototype::symbol_iterator) { auto this_object = interpreter.this_value(global_object); - if (this_object.is_undefined() || this_object.is_null()) - return interpreter.throw_exception(ErrorType::ToObjectNullOrUndef); + if (this_object.is_undefined() || this_object.is_null()) { + interpreter.throw_exception(ErrorType::ToObjectNullOrUndef); + return {}; + } auto string = this_object.to_string(interpreter); if (interpreter.exception()) diff --git a/Libraries/LibJS/Runtime/Uint8ClampedArray.cpp b/Libraries/LibJS/Runtime/Uint8ClampedArray.cpp index 99738b8f47..318b983eaa 100644 --- a/Libraries/LibJS/Runtime/Uint8ClampedArray.cpp +++ b/Libraries/LibJS/Runtime/Uint8ClampedArray.cpp @@ -58,8 +58,10 @@ JS_DEFINE_NATIVE_GETTER(Uint8ClampedArray::length_getter) auto* this_object = interpreter.this_value(global_object).to_object(interpreter, global_object); if (!this_object) return {}; - if (StringView(this_object->class_name()) != "Uint8ClampedArray") - return interpreter.throw_exception(ErrorType::NotA, "Uint8ClampedArray"); + if (StringView(this_object->class_name()) != "Uint8ClampedArray") { + interpreter.throw_exception(ErrorType::NotA, "Uint8ClampedArray"); + return {}; + } return Value(static_cast(this_object)->length()); } diff --git a/Libraries/LibJS/Runtime/Value.cpp b/Libraries/LibJS/Runtime/Value.cpp index af52fe2e51..43140cafb0 100644 --- a/Libraries/LibJS/Runtime/Value.cpp +++ b/Libraries/LibJS/Runtime/Value.cpp @@ -686,8 +686,10 @@ Value exp(Interpreter& interpreter, Value lhs, Value rhs) Value in(Interpreter& interpreter, Value lhs, Value rhs) { - if (!rhs.is_object()) - return interpreter.throw_exception(ErrorType::InOperatorWithObject); + if (!rhs.is_object()) { + interpreter.throw_exception(ErrorType::InOperatorWithObject); + return {}; + } auto lhs_string = lhs.to_string(interpreter); if (interpreter.exception()) return {}; @@ -696,22 +698,25 @@ Value in(Interpreter& interpreter, Value lhs, Value rhs) Value instance_of(Interpreter& interpreter, Value lhs, Value rhs) { - if (!rhs.is_object()) - return interpreter.throw_exception(ErrorType::NotAnObject, rhs.to_string_without_side_effects().characters()); - + if (!rhs.is_object()) { + interpreter.throw_exception(ErrorType::NotAnObject, rhs.to_string_without_side_effects().characters()); + return {}; + } auto has_instance_method = rhs.as_object().get(interpreter.well_known_symbol_has_instance()); if (!has_instance_method.is_empty()) { - if (!has_instance_method.is_function()) - return interpreter.throw_exception(ErrorType::NotAFunction, has_instance_method.to_string_without_side_effects().characters()); - + if (!has_instance_method.is_function()) { + interpreter.throw_exception(ErrorType::NotAFunction, has_instance_method.to_string_without_side_effects().characters()); + return {}; + } MarkedValueList arguments(interpreter.heap()); arguments.append(lhs); return Value(interpreter.call(has_instance_method.as_function(), rhs, move(arguments)).to_boolean()); } - if (!rhs.is_function()) - return interpreter.throw_exception(ErrorType::NotAFunction, rhs.to_string_without_side_effects().characters()); - + if (!rhs.is_function()) { + interpreter.throw_exception(ErrorType::NotAFunction, rhs.to_string_without_side_effects().characters()); + return {}; + } return ordinary_has_instance(interpreter, lhs, rhs); } @@ -734,9 +739,10 @@ Value ordinary_has_instance(Interpreter& interpreter, Value lhs, Value rhs) if (interpreter.exception()) return {}; - if (!rhs_prototype.is_object()) - return interpreter.throw_exception(ErrorType::InstanceOfOperatorBadPrototype, rhs_prototype.to_string_without_side_effects().characters()); - + if (!rhs_prototype.is_object()) { + interpreter.throw_exception(ErrorType::InstanceOfOperatorBadPrototype, rhs_prototype.to_string_without_side_effects().characters()); + return {}; + } while (true) { lhs_object = lhs_object->prototype(); if (interpreter.exception()) diff --git a/Libraries/LibWeb/Bindings/WindowObject.cpp b/Libraries/LibWeb/Bindings/WindowObject.cpp index 14fd04e568..b2843531a8 100644 --- a/Libraries/LibWeb/Bindings/WindowObject.cpp +++ b/Libraries/LibWeb/Bindings/WindowObject.cpp @@ -139,14 +139,17 @@ JS_DEFINE_NATIVE_FUNCTION(WindowObject::set_interval) auto* impl = impl_from(interpreter, global_object); if (!impl) return {}; - if (!interpreter.argument_count()) - return interpreter.throw_exception(JS::ErrorType::BadArgCountAtLeastOne, "setInterval"); + if (!interpreter.argument_count()) { + interpreter.throw_exception(JS::ErrorType::BadArgCountAtLeastOne, "setInterval"); + return {}; + } auto* callback_object = interpreter.argument(0).to_object(interpreter, global_object); if (!callback_object) return {}; - if (!callback_object->is_function()) - return interpreter.throw_exception(JS::ErrorType::NotAFunctionNoParam); - + if (!callback_object->is_function()) { + interpreter.throw_exception(JS::ErrorType::NotAFunctionNoParam); + return {}; + } i32 interval = 0; if (interpreter.argument_count() >= 2) { interval = interpreter.argument(1).to_i32(interpreter); @@ -165,14 +168,17 @@ JS_DEFINE_NATIVE_FUNCTION(WindowObject::set_timeout) auto* impl = impl_from(interpreter, global_object); if (!impl) return {}; - if (!interpreter.argument_count()) - return interpreter.throw_exception(JS::ErrorType::BadArgCountAtLeastOne, "setTimeout"); + if (!interpreter.argument_count()) { + interpreter.throw_exception(JS::ErrorType::BadArgCountAtLeastOne, "setTimeout"); + return {}; + } auto* callback_object = interpreter.argument(0).to_object(interpreter, global_object); if (!callback_object) return {}; - if (!callback_object->is_function()) - return interpreter.throw_exception(JS::ErrorType::NotAFunctionNoParam); - + if (!callback_object->is_function()) { + interpreter.throw_exception(JS::ErrorType::NotAFunctionNoParam); + return {}; + } i32 interval = 0; if (interpreter.argument_count() >= 2) { interval = interpreter.argument(1).to_i32(interpreter); @@ -191,8 +197,10 @@ JS_DEFINE_NATIVE_FUNCTION(WindowObject::clear_timeout) auto* impl = impl_from(interpreter, global_object); if (!impl) return {}; - if (!interpreter.argument_count()) - return interpreter.throw_exception(JS::ErrorType::BadArgCountAtLeastOne, "clearTimeout"); + if (!interpreter.argument_count()) { + interpreter.throw_exception(JS::ErrorType::BadArgCountAtLeastOne, "clearTimeout"); + return {}; + } i32 timer_id = interpreter.argument(0).to_i32(interpreter); if (interpreter.exception()) return {}; @@ -205,8 +213,10 @@ JS_DEFINE_NATIVE_FUNCTION(WindowObject::clear_interval) auto* impl = impl_from(interpreter, global_object); if (!impl) return {}; - if (!interpreter.argument_count()) - return interpreter.throw_exception(JS::ErrorType::BadArgCountAtLeastOne, "clearInterval"); + if (!interpreter.argument_count()) { + interpreter.throw_exception(JS::ErrorType::BadArgCountAtLeastOne, "clearInterval"); + return {}; + } i32 timer_id = interpreter.argument(0).to_i32(interpreter); if (interpreter.exception()) return {}; @@ -219,13 +229,17 @@ JS_DEFINE_NATIVE_FUNCTION(WindowObject::request_animation_frame) auto* impl = impl_from(interpreter, global_object); if (!impl) return {}; - if (!interpreter.argument_count()) - return interpreter.throw_exception(JS::ErrorType::BadArgCountOne, "requestAnimationFrame"); + if (!interpreter.argument_count()) { + interpreter.throw_exception(JS::ErrorType::BadArgCountOne, "requestAnimationFrame"); + return {}; + } auto* callback_object = interpreter.argument(0).to_object(interpreter, global_object); if (!callback_object) return {}; - if (!callback_object->is_function()) - return interpreter.throw_exception(JS::ErrorType::NotAFunctionNoParam); + if (!callback_object->is_function()) { + interpreter.throw_exception(JS::ErrorType::NotAFunctionNoParam); + return {}; + } return JS::Value(impl->request_animation_frame(*static_cast(callback_object))); } @@ -234,8 +248,10 @@ JS_DEFINE_NATIVE_FUNCTION(WindowObject::cancel_animation_frame) auto* impl = impl_from(interpreter, global_object); if (!impl) return {}; - if (!interpreter.argument_count()) - return interpreter.throw_exception(JS::ErrorType::BadArgCountOne, "cancelAnimationFrame"); + if (!interpreter.argument_count()) { + interpreter.throw_exception(JS::ErrorType::BadArgCountOne, "cancelAnimationFrame"); + return {}; + } auto id = interpreter.argument(0).to_i32(interpreter); if (interpreter.exception()) return {}; @@ -248,8 +264,10 @@ JS_DEFINE_NATIVE_FUNCTION(WindowObject::atob) auto* impl = impl_from(interpreter, global_object); if (!impl) return {}; - if (!interpreter.argument_count()) - return interpreter.throw_exception(JS::ErrorType::BadArgCountOne, "atob"); + if (!interpreter.argument_count()) { + interpreter.throw_exception(JS::ErrorType::BadArgCountOne, "atob"); + return {}; + } auto string = interpreter.argument(0).to_string(interpreter); if (interpreter.exception()) return {}; @@ -264,8 +282,10 @@ JS_DEFINE_NATIVE_FUNCTION(WindowObject::btoa) auto* impl = impl_from(interpreter, global_object); if (!impl) return {}; - if (!interpreter.argument_count()) - return interpreter.throw_exception(JS::ErrorType::BadArgCountOne, "btoa"); + if (!interpreter.argument_count()) { + interpreter.throw_exception(JS::ErrorType::BadArgCountOne, "btoa"); + return {}; + } auto string = interpreter.argument(0).to_string(interpreter); if (interpreter.exception()) return {}; @@ -273,8 +293,10 @@ JS_DEFINE_NATIVE_FUNCTION(WindowObject::btoa) Vector byte_string; byte_string.ensure_capacity(string.length()); for (u32 code_point : Utf8View(string)) { - if (code_point > 0xff) - return interpreter.throw_exception(JS::ErrorType::NotAByteString, "btoa"); + if (code_point > 0xff) { + interpreter.throw_exception(JS::ErrorType::NotAByteString, "btoa"); + return {}; + } byte_string.append(code_point); } diff --git a/Libraries/LibWeb/CodeGenerators/WrapperGenerator.cpp b/Libraries/LibWeb/CodeGenerators/WrapperGenerator.cpp index c2cf0c1e71..b47b4c9363 100644 --- a/Libraries/LibWeb/CodeGenerators/WrapperGenerator.cpp +++ b/Libraries/LibWeb/CodeGenerators/WrapperGenerator.cpp @@ -645,12 +645,13 @@ void generate_implementation(const IDL::Interface& interface) out() << " if (!impl)"; out() << " return {};"; if (function.length() > 0) { - out() << " if (interpreter.argument_count() < " << function.length() << ")"; - + out() << " if (interpreter.argument_count() < " << function.length() << ") {"; if (function.length() == 1) - out() << " return interpreter.throw_exception(JS::ErrorType::BadArgCountOne, \"" << function.name << "\");"; + out() << " interpreter.throw_exception(JS::ErrorType::BadArgCountOne, \"" << function.name << "\");"; else - out() << " return interpreter.throw_exception(JS::ErrorType::BadArgCountMany, \"" << function.name << "\", \"" << function.length() << "\");"; + out() << " interpreter.throw_exception(JS::ErrorType::BadArgCountMany, \"" << function.name << "\", \"" << function.length() << "\");"; + out() << " return {};"; + out() << " }"; } StringBuilder arguments_builder;