LibJS: Add side-effect-free version of Value::to_string()

There are now two API's on Value:

- Value::to_string(Interpreter&) -- may throw.
- Value::to_string_without_side_effects() -- will never throw.

These are some pretty big sweeping changes, so it's possible that I did
some part the wrong way. We'll work it out as we go. :^)

Fixes #2123.

Libraries/LibJS/AST.cpp

@@ -123,9 +123,9 @@ Value CallExpression::execute(Interpreter& interpreter) const
                 expression_string = static_cast<const Identifier&>(*m_callee).string();
             else
                 expression_string = static_cast<const MemberExpression&>(*m_callee).to_string_approximation();
-            error_message = String::format("%s is not a %s (evaluated from '%s')", callee.to_string().characters(), call_type, expression_string.characters());
+            error_message = String::format("%s is not a %s (evaluated from '%s')", callee.to_string_without_side_effects().characters(), call_type, expression_string.characters());
         } else {
-            error_message = String::format("%s is not a %s", callee.to_string().characters(), call_type);
+            error_message = String::format("%s is not a %s", callee.to_string_without_side_effects().characters(), call_type);
         }
         return interpreter.throw_exception<TypeError>(error_message);
     }
@@ -151,7 +151,7 @@ Value CallExpression::execute(Interpreter& interpreter) const
                 for (auto ch : static_cast<const StringObject&>(value.as_object()).primitive_string().string())
                     iterables.append(Value(js_string(interpreter, String::format("%c", ch))));
             } else {
-                interpreter.throw_exception<TypeError>(String::format("%s is not iterable", value.to_string().characters()));
+                interpreter.throw_exception<TypeError>(String::format("%s is not iterable", value.to_string_without_side_effects().characters()));
             }
             for (auto& value : iterables)
                 arguments.append(value);
@@ -1154,7 +1154,9 @@ Value ObjectExpression::execute(Interpreter& interpreter) const
             continue;
         }
 
-        auto key = key_result.to_string();
+        auto key = key_result.to_string(interpreter);
+        if (interpreter.exception())
+            return {};
         auto value = property.value().execute(interpreter);
         if (interpreter.exception())
             return {};
@@ -1184,14 +1186,13 @@ PropertyName MemberExpression::computed_property_name(Interpreter& interpreter)
 
     ASSERT(!index.is_empty());
 
-    if (!index.to_number().is_finite_number())
+    if (index.is_integer() && index.to_i32() >= 0)
-        return PropertyName(index.to_string());
+        return PropertyName(index.to_i32());
 
-    auto index_as_double = index.to_double();
+    auto index_string = index.to_string(interpreter);
-    if (index_as_double < 0 || (i32)index_as_double != index_as_double)
+    if (interpreter.exception())
-        return PropertyName(index.to_string());
+        return {};
-
+    return PropertyName(index_string);
-    return PropertyName(index.to_i32());
 }
 
 String MemberExpression::to_string_approximation() const
@@ -1283,7 +1284,7 @@ Value ArrayExpression::execute(Interpreter& interpreter) const
                         array->elements().append(js_string(interpreter, string_to_spread.substring(i, 1)));
                     continue;
                 }
-                interpreter.throw_exception<TypeError>(String::format("%s is not iterable", value.to_string().characters()));
+                interpreter.throw_exception<TypeError>(String::format("%s is not iterable", value.to_string_without_side_effects().characters()));
                 return {};
             }
         }
@@ -1307,7 +1308,10 @@ Value TemplateLiteral::execute(Interpreter& interpreter) const
         auto expr = expression.execute(interpreter);
         if (interpreter.exception())
             return {};
-        string_builder.append(expr.to_string());
+        auto string = expr.to_string(interpreter);
+        if (interpreter.exception())
+            return {};
+        string_builder.append(string);
     }
 
     return js_string(interpreter, string_builder.build());
@@ -1330,7 +1334,7 @@ Value TaggedTemplateLiteral::execute(Interpreter& interpreter) const
     if (interpreter.exception())
         return {};
     if (!tag.is_function()) {
-        interpreter.throw_exception<TypeError>(String::format("%s is not a function", tag.to_string().characters()));
+        interpreter.throw_exception<TypeError>(String::format("%s is not a function", tag.to_string_without_side_effects().characters()));
         return {};
     }
     auto& tag_function = tag.as_function();

Libraries/LibJS/Console.cpp

@@ -116,7 +116,7 @@ Value Console::count()
     if (m_client)
         return m_client->count();
 
-    auto label = m_interpreter.argument_count() ? m_interpreter.argument(0).to_string() : "default";
+    auto label = m_interpreter.argument_count() ? m_interpreter.argument(0).to_string_without_side_effects() : "default";
 
     auto counter_value = counter_increment(label);
     dbg() << "log: " << label << ": " << counter_value;
@@ -129,7 +129,7 @@ Value Console::count_reset()
     if (m_client)
         return m_client->count_reset();
 
-    auto label = m_interpreter.argument_count() ? m_interpreter.argument(0).to_string() : "default";
+    auto label = m_interpreter.argument_count() ? m_interpreter.argument(0).to_string_without_side_effects() : "default";
 
     if (counter_reset(label))
         dbg() << "log: " << label << ": 0";

Libraries/LibJS/Interpreter.cpp

@@ -266,7 +266,7 @@ String Interpreter::join_arguments() const
 {
     StringBuilder joined_arguments;
     for (size_t i = 0; i < argument_count(); ++i) {
-        joined_arguments.append(argument(i).to_string().characters());
+        joined_arguments.append(argument(i).to_string_without_side_effects().characters());
         if (i != argument_count() - 1)
             joined_arguments.append(' ');
     }

Libraries/LibJS/Runtime/ArrayPrototype.cpp

@@ -77,7 +77,7 @@ static Function* callback_from_args(Interpreter& interpreter, const String& name
     }
     auto callback = interpreter.argument(0);
     if (!callback.is_function()) {
-        interpreter.throw_exception<TypeError>(String::format("%s is not a function", callback.to_string().characters()));
+        interpreter.throw_exception<TypeError>(String::format("%s is not a function", callback.to_string_without_side_effects().characters()));
         return nullptr;
     }
     return &callback.as_function();
@@ -227,8 +227,12 @@ static Value join_array_with_separator(Interpreter& interpreter, const Array& ar
         if (i != 0)
             builder.append(separator);
         auto value = array.elements()[i];
-        if (!value.is_empty() && !value.is_undefined() && !value.is_null())
+        if (!value.is_empty() && !value.is_undefined() && !value.is_null()) {
-            builder.append(value.to_string());
+            auto string = value.to_string(interpreter);
+            if (interpreter.exception())
+                return {};
+            builder.append(string);
+        }
     }
     return js_string(interpreter, builder.to_string());
 }
@@ -249,8 +253,11 @@ Value ArrayPrototype::join(Interpreter& interpreter)
         return {};
 
     String separator = ",";
-    if (interpreter.argument_count() == 1)
+    if (interpreter.argument_count() == 1) {
-        separator = interpreter.argument(0).to_string();
+        separator = interpreter.argument(0).to_string(interpreter);
+        if (interpreter.exception())
+            return {};
+    }
 
     return join_array_with_separator(interpreter, *array, separator);
 }

Libraries/LibJS/Runtime/ErrorConstructor.cpp

@@ -50,29 +50,35 @@ Value ErrorConstructor::call(Interpreter& interpreter)
 Value ErrorConstructor::construct(Interpreter& interpreter)
 {
     String message = "";
-    if (!interpreter.call_frame().arguments.is_empty() && !interpreter.call_frame().arguments[0].is_undefined())
+    if (!interpreter.call_frame().arguments.is_empty() && !interpreter.call_frame().arguments[0].is_undefined()) {
-        message = interpreter.call_frame().arguments[0].to_string();
+        message = interpreter.call_frame().arguments[0].to_string(interpreter);
+        if (interpreter.exception())
+            return {};
+    }
     return Error::create(interpreter.global_object(), "Error", message);
 }
 
-#define __JS_ENUMERATE(ClassName, snake_name, PrototypeName, ConstructorName)                                        \
+#define __JS_ENUMERATE(ClassName, snake_name, PrototypeName, ConstructorName)                                          \
-    ConstructorName::ConstructorName()                                                                               \
+    ConstructorName::ConstructorName()                                                                                 \
-        : NativeFunction(*interpreter().global_object().function_prototype())                                        \
+        : NativeFunction(*interpreter().global_object().function_prototype())                                          \
-    {                                                                                                                \
+    {                                                                                                                  \
-        put("prototype", interpreter().global_object().snake_name##_prototype(), 0);                                 \
+        put("prototype", interpreter().global_object().snake_name##_prototype(), 0);                                   \
-        put("length", Value(1), Attribute::Configurable);                                                            \
+        put("length", Value(1), Attribute::Configurable);                                                              \
-    }                                                                                                                \
+    }                                                                                                                  \
-    ConstructorName::~ConstructorName() { }                                                                          \
+    ConstructorName::~ConstructorName() { }                                                                            \
-    Value ConstructorName::call(Interpreter& interpreter)                                                            \
+    Value ConstructorName::call(Interpreter& interpreter)                                                              \
-    {                                                                                                                \
+    {                                                                                                                  \
-        return construct(interpreter);                                                                               \
+        return construct(interpreter);                                                                                 \
-    }                                                                                                                \
+    }                                                                                                                  \
-    Value ConstructorName::construct(Interpreter& interpreter)                                                       \
+    Value ConstructorName::construct(Interpreter& interpreter)                                                         \
-    {                                                                                                                \
+    {                                                                                                                  \
-        String message = "";                                                                                         \
+        String message = "";                                                                                           \
-        if (!interpreter.call_frame().arguments.is_empty() && !interpreter.call_frame().arguments[0].is_undefined()) \
+        if (!interpreter.call_frame().arguments.is_empty() && !interpreter.call_frame().arguments[0].is_undefined()) { \
-            message = interpreter.call_frame().arguments[0].to_string();                                             \
+            message = interpreter.call_frame().arguments[0].to_string(interpreter);                                    \
-        return ClassName::create(interpreter.global_object(), message);                                              \
+            if (interpreter.exception())                                                                               \
+                return {};                                                                                             \
+        }                                                                                                              \
+        return ClassName::create(interpreter.global_object(), message);                                                \
     }
 
 JS_ENUMERATE_ERROR_SUBCLASSES

Libraries/LibJS/Runtime/ErrorPrototype.cpp

@@ -67,7 +67,9 @@ void ErrorPrototype::name_setter(Interpreter& interpreter, Value value)
         interpreter.throw_exception<TypeError>("Not an Error object");
         return;
     }
-    auto name = FlyString(value.to_string());
+    auto name = value.to_string(interpreter);
+    if (interpreter.exception())
+        return;
     static_cast<Error*>(this_object)->set_name(name);
 }
 
@@ -89,13 +91,19 @@ Value ErrorPrototype::to_string(Interpreter& interpreter)
 
     String name = "Error";
     auto object_name_property = this_object.get("name");
-    if (!object_name_property.is_empty() && !object_name_property.is_undefined())
+    if (!object_name_property.is_empty() && !object_name_property.is_undefined()) {
-        name = object_name_property.to_string();
+        name = object_name_property.to_string(interpreter);
+        if (interpreter.exception())
+            return {};
+    }
 
     String message = "";
     auto object_message_property = this_object.get("message");
-    if (!object_message_property.is_empty() && !object_message_property.is_undefined())
+    if (!object_message_property.is_empty() && !object_message_property.is_undefined()) {
-        message = object_message_property.to_string();
+        message = object_message_property.to_string(interpreter);
+        if (interpreter.exception())
+            return {};
+    }
 
     if (name.length() == 0)
         return js_string(interpreter, message);

Libraries/LibJS/Runtime/FunctionConstructor.cpp

@@ -55,16 +55,24 @@ Value FunctionConstructor::construct(Interpreter& interpreter)
 {
     String parameters_source = "";
     String body_source = "";
-    if (interpreter.argument_count() == 1)
+    if (interpreter.argument_count() == 1) {
-        body_source = interpreter.argument(0).to_string();
+        body_source = interpreter.argument(0).to_string(interpreter);
+        if (interpreter.exception())
+            return {};
+    }
     if (interpreter.argument_count() > 1) {
         Vector<String> parameters;
-        for (size_t i = 0; i < interpreter.argument_count() - 1; ++i)
+        for (size_t i = 0; i < interpreter.argument_count() - 1; ++i) {
-            parameters.append(interpreter.argument(i).to_string());
+            parameters.append(interpreter.argument(i).to_string(interpreter));
+            if (interpreter.exception())
+                return {};
+        }
         StringBuilder parameters_builder;
         parameters_builder.join(',', parameters);
         parameters_source = parameters_builder.build();
-        body_source = interpreter.argument(interpreter.argument_count() - 1).to_string();
+        body_source = interpreter.argument(interpreter.argument_count() - 1).to_string(interpreter);
+        if (interpreter.exception())
+            return {};
     }
     auto source = String::format("function anonymous(%s) { %s }", parameters_source.characters(), body_source.characters());
     auto parser = Parser(Lexer(source));

Libraries/LibJS/Runtime/Object.cpp

@@ -485,7 +485,10 @@ Value Object::to_string() const
             interpreter.throw_exception<TypeError>("Cannot convert object to string");
         if (interpreter.exception())
             return {};
-        return js_string(heap(), to_string_result.to_string());
+        auto* string = to_string_result.to_primitive_string(interpreter);
+        if (interpreter.exception())
+            return {};
+        return string;
     }
     return js_string(heap(), String::format("[object %s]", class_name()));
 }

Libraries/LibJS/Runtime/ObjectConstructor.cpp

@@ -112,7 +112,9 @@ Value ObjectConstructor::get_own_property_descriptor(Interpreter& interpreter)
     auto* object = interpreter.argument(0).to_object(interpreter.heap());
     if (interpreter.exception())
         return {};
-    auto property_key = interpreter.argument(1).to_string();
+    auto property_key = interpreter.argument(1).to_string(interpreter);
+    if (interpreter.exception())
+        return {};
     return object->get_own_property_descriptor(property_key);
 }
 
@@ -123,7 +125,9 @@ Value ObjectConstructor::define_property(Interpreter& interpreter)
     if (!interpreter.argument(2).is_object())
         return interpreter.throw_exception<TypeError>("Descriptor argument is not an object");
     auto& object = interpreter.argument(0).as_object();
-    auto property_key = interpreter.argument(1).to_string();
+    auto property_key = interpreter.argument(1).to_string(interpreter);
+    if (interpreter.exception())
+        return {};
     auto& descriptor = interpreter.argument(2).as_object();
     object.define_property(property_key, descriptor);
     return &object;

Libraries/LibJS/Runtime/ObjectPrototype.cpp

@@ -57,7 +57,10 @@ Value ObjectPrototype::has_own_property(Interpreter& interpreter)
     auto* this_object = interpreter.this_value().to_object(interpreter.heap());
     if (!this_object)
         return {};
-    return Value(this_object->has_own_property(interpreter.argument(0).to_string()));
+    auto name = interpreter.argument(0).to_string(interpreter);
+    if (interpreter.exception())
+        return {};
+    return Value(this_object->has_own_property(name));
 }
 
 Value ObjectPrototype::to_string(Interpreter& interpreter)

Libraries/LibJS/Runtime/ReflectObject.cpp

@@ -138,7 +138,9 @@ Value ReflectObject::define_property(Interpreter& interpreter)
         return {};
     if (!interpreter.argument(2).is_object())
         return interpreter.throw_exception<TypeError>("Descriptor argument is not an object");
-    auto property_key = interpreter.argument(1).to_string();
+    auto property_key = interpreter.argument(1).to_string(interpreter);
+    if (interpreter.exception())
+        return {};
     auto& descriptor = interpreter.argument(2).as_object();
     auto success = target->define_property(property_key, descriptor, false);
     return Value(success);
@@ -151,13 +153,15 @@ Value ReflectObject::delete_property(Interpreter& interpreter)
         return {};
 
     auto property_key = interpreter.argument(1);
-    PropertyName property = PropertyName(property_key.to_string());
+    auto property_name = PropertyName(property_key.to_string(interpreter));
+    if (interpreter.exception())
+        return {};
     if (property_key.to_number().is_finite_number()) {
         auto property_key_as_double = property_key.to_double();
         if (property_key_as_double >= 0 && (i32)property_key_as_double == property_key_as_double)
-            property = PropertyName(property_key_as_double);
+            property_name = PropertyName(property_key_as_double);
     }
-    return target->delete_property(property);
+    return target->delete_property(property_name);
 }
 
 Value ReflectObject::get(Interpreter& interpreter)
@@ -166,7 +170,9 @@ Value ReflectObject::get(Interpreter& interpreter)
     auto* target = get_target_object_from(interpreter, "get");
     if (!target)
         return {};
-    auto property_key = interpreter.argument(1).to_string();
+    auto property_key = interpreter.argument(1).to_string(interpreter);
+    if (interpreter.exception())
+        return {};
     return target->get(property_key).value_or(js_undefined());
 }
 
@@ -175,7 +181,9 @@ Value ReflectObject::get_own_property_descriptor(Interpreter& interpreter)
     auto* target = get_target_object_from(interpreter, "getOwnPropertyDescriptor");
     if (!target)
         return {};
-    auto property_key = interpreter.argument(1).to_string();
+    auto property_key = interpreter.argument(1).to_string(interpreter);
+    if (interpreter.exception())
+        return {};
     return target->get_own_property_descriptor(property_key);
 }
 
@@ -192,7 +200,9 @@ Value ReflectObject::has(Interpreter& interpreter)
     auto* target = get_target_object_from(interpreter, "has");
     if (!target)
         return {};
-    auto property_key = interpreter.argument(1).to_string();
+    auto property_key = interpreter.argument(1).to_string(interpreter);
+    if (interpreter.exception())
+        return {};
     return Value(target->has_property(property_key));
 }
 
@@ -224,7 +234,9 @@ Value ReflectObject::set(Interpreter& interpreter)
     auto* target = get_target_object_from(interpreter, "set");
     if (!target)
         return {};
-    auto property_key = interpreter.argument(1).to_string();
+    auto property_key = interpreter.argument(1).to_string(interpreter);
+    if (interpreter.exception())
+        return {};
     auto value = interpreter.argument(2);
     return Value(target->put(property_key, value));
 }

Libraries/LibJS/Runtime/StringConstructor.cpp

@@ -51,7 +51,10 @@ Value StringConstructor::call(Interpreter& interpreter)
 {
     if (!interpreter.argument_count())
         return js_string(interpreter, "");
-    return js_string(interpreter, interpreter.argument(0).to_string());
+    auto* string = interpreter.argument(0).to_primitive_string(interpreter);
+    if (interpreter.exception())
+        return {};
+    return string;
 }
 
 Value StringConstructor::construct(Interpreter& interpreter)
@@ -60,7 +63,7 @@ Value StringConstructor::construct(Interpreter& interpreter)
     if (!interpreter.argument_count())
         primitive_string = js_string(interpreter, "");
     else
-        primitive_string = js_string(interpreter, interpreter.argument(0).to_string());
+        primitive_string = interpreter.argument(0).to_primitive_string(interpreter);
     if (!primitive_string)
         return {};
     return StringObject::create(interpreter.global_object(), *primitive_string);
@@ -84,9 +87,14 @@ Value StringConstructor::raw(Interpreter& interpreter)
     StringBuilder builder;
 
     for (size_t i = 0; i < raw_array_elements.size(); ++i) {
-        builder.append(raw_array_elements.at(i).to_string());
+        builder.append(raw_array_elements.at(i).to_string(interpreter));
-        if (i + 1 < interpreter.argument_count() && i < raw_array_elements.size() - 1)
+        if (interpreter.exception())
-            builder.append(interpreter.argument(i + 1).to_string());
+            return {};
+        if (i + 1 < interpreter.argument_count() && i < raw_array_elements.size() - 1) {
+            builder.append(interpreter.argument(i + 1).to_string(interpreter));
+            if (interpreter.exception())
+                return {};
+        }
     }
 
     return js_string(interpreter, builder.build());

Libraries/LibJS/Runtime/StringPrototype.cpp

@@ -56,7 +56,7 @@ static String string_from(Interpreter& interpreter)
     auto* this_object = interpreter.this_value().to_object(interpreter.heap());
     if (!this_object)
         return {};
-    return Value(this_object).to_string();
+    return Value(this_object).to_string(interpreter);
 }
 
 StringPrototype::StringPrototype()
@@ -127,7 +127,9 @@ Value StringPrototype::starts_with(Interpreter& interpreter)
         return {};
     if (!interpreter.argument_count())
         return Value(false);
-    auto search_string = interpreter.argument(0).to_string();
+    auto search_string = interpreter.argument(0).to_string(interpreter);
+    if (interpreter.exception())
+        return {};
     auto search_string_length = static_cast<i32>(search_string.length());
     i32 position = 0;
     if (interpreter.argument_count() > 1) {
@@ -152,7 +154,9 @@ Value StringPrototype::index_of(Interpreter& interpreter)
     Value needle_value = js_undefined();
     if (interpreter.argument_count() >= 1)
         needle_value = interpreter.argument(0);
-    auto needle = needle_value.to_string();
+    auto needle = needle_value.to_string(interpreter);
+    if (interpreter.exception())
+        return {};
     return Value((i32)string.index_of(needle).value_or(-1));
 }
 
@@ -204,8 +208,11 @@ static Value pad_string(Interpreter& interpreter, const String& string, PadPlace
         return js_string(interpreter, string);
 
     String fill_string = " ";
-    if (!interpreter.argument(1).is_undefined())
+    if (!interpreter.argument(1).is_undefined()) {
-        fill_string = interpreter.argument(1).to_string();
+        fill_string = interpreter.argument(1).to_string(interpreter);
+        if (interpreter.exception())
+            return {};
+    }
     if (fill_string.is_empty())
         return js_string(interpreter, string);
 
@@ -269,7 +276,9 @@ Value StringPrototype::concat(Interpreter& interpreter)
     StringBuilder builder;
     builder.append(string);
     for (size_t i = 0; i < interpreter.argument_count(); ++i) {
-        auto string_argument = interpreter.argument(i).to_string();
+        auto string_argument = interpreter.argument(i).to_string(interpreter);
+        if (interpreter.exception())
+            return {};
         builder.append(string_argument);
     }
     return js_string(interpreter, builder.to_string());
@@ -324,7 +333,9 @@ Value StringPrototype::includes(Interpreter& interpreter)
     auto string = string_from(interpreter);
     if (string.is_null())
         return {};
-    auto search_string = interpreter.argument(0).to_string();
+    auto search_string = interpreter.argument(0).to_string(interpreter);
+    if (interpreter.exception())
+        return {};
     i32 position = 0;
 
     if (interpreter.argument_count() >= 2) {
@@ -393,7 +404,9 @@ Value StringPrototype::last_index_of(Interpreter& interpreter)
     if (interpreter.argument_count() == 0)
         return Value(-1);
 
-    auto search_string = interpreter.argument(0).to_string();
+    auto search_string = interpreter.argument(0).to_string(interpreter);
+    if (interpreter.exception())
+        return {};
     if (search_string.length() > string.length())
         return Value(-1);
 

Libraries/LibJS/Runtime/Value.cpp

@@ -26,6 +26,7 @@
 
 #include <AK/FlyString.h>
 #include <AK/String.h>
+#include <AK/StringBuilder.h>
 #include <LibJS/Heap/Heap.h>
 #include <LibJS/Interpreter.h>
 #include <LibJS/Runtime/Array.h>
@@ -60,7 +61,7 @@ Function& Value::as_function()
     return static_cast<Function&>(as_object());
 }
 
-String Value::to_string() const
+String Value::to_string_without_side_effects() const
 {
     if (is_boolean())
         return as_bool() ? "true" : "false";
@@ -78,8 +79,47 @@ String Value::to_string() const
         if (is_infinity())
             return as_double() < 0 ? "-Infinity" : "Infinity";
 
-        // FIXME: This needs improvement.
+        if (is_integer())
-        if ((double)to_i32() == as_double())
+            return String::number(to_i32());
+        return String::format("%.4f", as_double());
+    }
+
+    if (is_string())
+        return m_value.as_string->string();
+
+    ASSERT(is_object());
+    return String::format("[object %s]", as_object().class_name());
+}
+
+PrimitiveString* Value::to_primitive_string(Interpreter & interpreter)
+{
+    if (is_string())
+        return &as_string();
+    auto string = to_string(interpreter);
+    if (interpreter.exception())
+        return nullptr;
+    return js_string(interpreter, string);
+}
+
+String Value::to_string(Interpreter& interpreter) const
+{
+    if (is_boolean())
+        return as_bool() ? "true" : "false";
+
+    if (is_null())
+        return "null";
+
+    if (is_undefined())
+        return "undefined";
+
+    if (is_number()) {
+        if (is_nan())
+            return "NaN";
+
+        if (is_infinity())
+            return as_double() < 0 ? "-Infinity" : "Infinity";
+
+        if (is_integer())
             return String::number(to_i32());
         return String::format("%.4f", as_double());
     }
@@ -89,13 +129,11 @@ String Value::to_string() const
         // FIXME: Maybe we should pass in the Interpreter& and call interpreter.exception() instead?
         if (primitive_value.is_empty())
             return {};
-        return primitive_value.to_string();
+        return primitive_value.to_string(interpreter);
     }
 
-    if (is_string())
+    ASSERT(is_string());
-        return m_value.as_string->string();
+    return m_value.as_string->string();
-
-    ASSERT_NOT_REACHED();
 }
 
 bool Value::to_boolean() const
@@ -305,10 +343,24 @@ Value unsigned_right_shift(Interpreter&, Value lhs, Value rhs)
 Value add(Interpreter& interpreter, Value lhs, Value rhs)
 {
     auto lhs_primitive = lhs.to_primitive(interpreter);
+    if (interpreter.exception())
+        return {};
     auto rhs_primitive = rhs.to_primitive(interpreter);
+    if (interpreter.exception())
+        return {};
 
-    if (lhs_primitive.is_string() || rhs_primitive.is_string())
+    if (lhs_primitive.is_string() || rhs_primitive.is_string()) {
-        return js_string(interpreter.heap(), String::format("%s%s", lhs_primitive.to_string().characters(), rhs_primitive.to_string().characters()));
+        auto lhs_string = lhs_primitive.to_string(interpreter);
+        if (interpreter.exception())
+            return {};
+        auto rhs_string = rhs_primitive.to_string(interpreter);
+        if (interpreter.exception())
+            return {};
+        StringBuilder builder(lhs_string.length() + rhs_string.length());
+        builder.append(lhs_string);
+        builder.append(rhs_string);
+        return js_string(interpreter, builder.to_string());
+    }
 
     return Value(lhs_primitive.to_number().as_double() + rhs_primitive.to_number().as_double());
 }
@@ -350,7 +402,11 @@ Value in(Interpreter& interpreter, Value lhs, Value rhs)
     if (!rhs.is_object())
         return interpreter.throw_exception<TypeError>("'in' operator must be used on object");
 
-    return Value(!rhs.as_object().get(lhs.to_string()).is_empty());
+    auto lhs_string = lhs.to_string(interpreter);
+    if (interpreter.exception())
+        return {};
+
+    return Value(!rhs.as_object().get(lhs_string).is_empty());
 }
 
 Value instance_of(Interpreter&, Value lhs, Value rhs)
@@ -367,7 +423,7 @@ Value instance_of(Interpreter&, Value lhs, Value rhs)
 
 const LogStream& operator<<(const LogStream& stream, const Value& value)
 {
-    return stream << (value.is_empty() ? "<empty>" : value.to_string());
+    return stream << (value.is_empty() ? "<empty>" : value.to_string_without_side_effects());
 }
 
 bool same_value(Interpreter& interpreter, Value lhs, Value rhs)

Libraries/LibJS/Runtime/Value.h

@@ -159,9 +159,12 @@ public:
         return m_value.as_cell;
     }
 
+    String to_string_without_side_effects() const;
+
     Function& as_function();
 
-    String to_string() const;
+    String to_string(Interpreter&) const;
+    PrimitiveString* to_primitive_string(Interpreter&);
     bool to_boolean() const;
     Value to_number() const;
     i32 to_i32() const;

Libraries/LibJS/Tests/array-spread.js

@@ -30,6 +30,7 @@ try {
         message: "1 is not iterable",
     });
 
+
     assertThrowsError(() => {
         [...{}];
     }, {

Libraries/LibJS/Tests/function-TypeError.js

@@ -43,7 +43,7 @@ try {
         new isNaN();
     }, {
         error: TypeError,
-        message: "function isNaN() {\n  [NativeFunction]\n} is not a constructor (evaluated from 'isNaN')"
+        message: "[object NativeFunction] is not a constructor (evaluated from 'isNaN')"
     });
 
     console.log("PASS");

Libraries/LibWeb/Bindings/CanvasRenderingContext2DWrapper.cpp

@@ -161,8 +161,12 @@ JS::Value CanvasRenderingContext2DWrapper::fill_style_getter(JS::Interpreter& in
 
 void CanvasRenderingContext2DWrapper::fill_style_setter(JS::Interpreter& interpreter, JS::Value value)
 {
-    if (auto* impl = impl_from(interpreter))
+    if (auto* impl = impl_from(interpreter)) {
-        impl->set_fill_style(value.to_string());
+        auto string = value.to_string(interpreter);
+        if (interpreter.exception())
+            return;
+        impl->set_fill_style(string);
+    }
 }
 
 JS::Value CanvasRenderingContext2DWrapper::stroke_style_getter(JS::Interpreter& interpreter)
@@ -175,8 +179,12 @@ JS::Value CanvasRenderingContext2DWrapper::stroke_style_getter(JS::Interpreter&
 
 void CanvasRenderingContext2DWrapper::stroke_style_setter(JS::Interpreter& interpreter, JS::Value value)
 {
-    if (auto* impl = impl_from(interpreter))
+    if (auto* impl = impl_from(interpreter)){
-        impl->set_stroke_style(value.to_string());
+        auto string = value.to_string(interpreter);
+        if (interpreter.exception())
+            return;
+        impl->set_stroke_style(string);
+    }
 }
 
 JS::Value CanvasRenderingContext2DWrapper::line_width_getter(JS::Interpreter& interpreter)

Libraries/LibWeb/Bindings/DocumentWrapper.cpp

@@ -78,7 +78,9 @@ JS::Value DocumentWrapper::get_element_by_id(JS::Interpreter& interpreter)
     auto& arguments = interpreter.call_frame().arguments;
     if (arguments.is_empty())
         return JS::js_null();
-    auto id = arguments[0].to_string();
+    auto id = arguments[0].to_string(interpreter);
+    if (interpreter.exception())
+        return {};
     auto* element = document->get_element_by_id(id);
     if (!element)
         return JS::js_null();
@@ -93,7 +95,9 @@ JS::Value DocumentWrapper::query_selector_all(JS::Interpreter& interpreter)
     auto& arguments = interpreter.call_frame().arguments;
     if (arguments.is_empty())
         return JS::js_null();
-    auto selector = arguments[0].to_string();
+    auto selector = arguments[0].to_string(interpreter);
+    if (interpreter.exception())
+        return {};
     auto elements = document->query_selector_all(selector);
     // FIXME: This should be a static NodeList, not a plain JS::Array.
     auto* node_list = JS::Array::create(interpreter.global_object());

Libraries/LibWeb/Bindings/ElementWrapper.cpp

@@ -74,8 +74,12 @@ JS::Value ElementWrapper::inner_html_getter(JS::Interpreter& interpreter)
 
 void ElementWrapper::inner_html_setter(JS::Interpreter& interpreter, JS::Value value)
 {
-    if (auto* impl = impl_from(interpreter))
+    if (auto* impl = impl_from(interpreter)) {
-        impl->set_inner_html(value.to_string());
+        auto string = value.to_string(interpreter);
+        if (interpreter.exception())
+            return;
+        impl->set_inner_html(string);
+    }
 }
 
 JS::Value ElementWrapper::id_getter(JS::Interpreter& interpreter)
@@ -87,8 +91,12 @@ JS::Value ElementWrapper::id_getter(JS::Interpreter& interpreter)
 
 void ElementWrapper::id_setter(JS::Interpreter& interpreter, JS::Value value)
 {
-    if (auto* impl = impl_from(interpreter))
+    if (auto* impl = impl_from(interpreter)) {
-        impl->set_attribute("id", value.to_string());
+        auto string = value.to_string(interpreter);
+        if (interpreter.exception())
+            return;
+        impl->set_attribute("id", string);
+    }
 }
 
 }

Libraries/LibWeb/Bindings/EventTargetWrapper.cpp

@@ -56,7 +56,9 @@ JS::Value EventTargetWrapper::add_event_listener(JS::Interpreter& interpreter)
     auto& arguments = interpreter.call_frame().arguments;
     if (arguments.size() < 2)
         return JS::js_undefined();
-    auto event_name = arguments[0].to_string();
+    auto event_name = arguments[0].to_string(interpreter);
+    if (interpreter.exception())
+        return {};
     ASSERT(arguments[1].is_object());
     ASSERT(arguments[1].as_object().is_function());
     auto& function = static_cast<JS::Function&>(const_cast<Object&>(arguments[1].as_object()));

Libraries/LibWeb/Bindings/HTMLCanvasElementWrapper.cpp

@@ -76,7 +76,10 @@ JS::Value HTMLCanvasElementWrapper::get_context(JS::Interpreter& interpreter)
         return {};
     auto& arguments = interpreter.call_frame().arguments;
     if (arguments.size() >= 1) {
-        auto* context = impl->get_context(arguments[0].to_string());
+        auto string = arguments[0].to_string(interpreter);
+        if (interpreter.exception())
+            return {};
+        auto* context = impl->get_context(string);
         return wrap(interpreter.heap(), *context);
     }
     return JS::js_undefined();

Libraries/LibWeb/Bindings/WindowObject.cpp

@@ -98,8 +98,11 @@ JS::Value WindowObject::alert(JS::Interpreter& interpreter)
     if (!impl)
         return {};
     String message = "";
-    if (interpreter.argument_count())
+    if (interpreter.argument_count()) {
-        message = interpreter.argument(0).to_string();
+        message = interpreter.argument(0).to_string(interpreter);
+        if (interpreter.exception())
+            return {};
+    }
     impl->alert(message);
     return JS::js_undefined();
 }
@@ -110,8 +113,11 @@ JS::Value WindowObject::confirm(JS::Interpreter& interpreter)
     if (!impl)
         return {};
     String message = "";
-    if (interpreter.argument_count())
+    if (interpreter.argument_count()) {
-        message = interpreter.argument(0).to_string();
+        message = interpreter.argument(0).to_string(interpreter);
+        if (interpreter.exception())
+            return {};
+    }
     return JS::Value(impl->confirm(message));
 }
 

Libraries/LibWeb/Bindings/XMLHttpRequestPrototype.cpp

@@ -71,7 +71,13 @@ JS::Value XMLHttpRequestPrototype::open(JS::Interpreter& interpreter)
     auto* impl = impl_from(interpreter);
     if (!impl)
         return {};
-    impl->open(interpreter.argument(0).to_string(), interpreter.argument(1).to_string());
+    auto arg0 = interpreter.argument(0).to_string(interpreter);
+    if (interpreter.exception())
+        return {};
+    auto arg1 = interpreter.argument(1).to_string(interpreter);
+    if (interpreter.exception())
+        return {};
+    impl->open(arg0, arg1);
     return JS::js_undefined();
 }
 

Userland/js.cpp

@@ -213,7 +213,7 @@ void print_value(JS::Value value, HashTable<JS::Object*>& seen_objects)
         printf("\033[34;1m");
     if (value.is_string())
         putchar('"');
-    printf("%s", value.to_string().characters());
+    printf("%s", value.to_string_without_side_effects().characters());
     if (value.is_string())
         putchar('"');
     printf("\033[0m");
@@ -316,7 +316,7 @@ JS::Value ReplObject::save_to_file(JS::Interpreter& interpreter)
 {
     if (!interpreter.argument_count())
         return JS::Value(false);
-    String save_path = interpreter.argument(0).to_string();
+    String save_path = interpreter.argument(0).to_string_without_side_effects();
     StringView path = StringView(save_path.characters());
     if (write_to_file(path)) {
         return JS::Value(true);
@@ -445,14 +445,14 @@ public:
     }
     virtual JS::Value count() override
     {
-        auto label = interpreter().argument_count() ? interpreter().argument(0).to_string() : "default";
+        auto label = interpreter().argument_count() ? interpreter().argument(0).to_string_without_side_effects() : "default";
         auto counter_value = m_console.counter_increment(label);
         printf("%s: %u\n", label.characters(), counter_value);
         return JS::js_undefined();
     }
     virtual JS::Value count_reset() override
     {
-        auto label = interpreter().argument_count() ? interpreter().argument(0).to_string() : "default";
+        auto label = interpreter().argument_count() ? interpreter().argument(0).to_string_without_side_effects() : "default";
         if (m_console.counter_reset(label)) {
             printf("%s: 0\n", label.characters());
         } else {