From 0d47c4e7a0b946972a4a190ebf1644eaacd329e6 Mon Sep 17 00:00:00 2001 From: Timothy Flynn Date: Sat, 14 Jan 2023 23:11:50 -0500 Subject: [PATCH] LibJS: Port the TrimString AO to String This also adds spec comments to parseFloat to make it clear that we are now deviating a bit from the spec (the TrimString invocation should be infallible, but we want to propagate OOM errors). --- .../Libraries/LibJS/Runtime/GlobalObject.cpp | 48 +++++++++++++------ .../LibJS/Runtime/StringPrototype.cpp | 6 +-- .../Libraries/LibJS/Runtime/StringPrototype.h | 2 +- 3 files changed, 37 insertions(+), 19 deletions(-) diff --git a/Userland/Libraries/LibJS/Runtime/GlobalObject.cpp b/Userland/Libraries/LibJS/Runtime/GlobalObject.cpp index 216a383aef..6b64f02602 100644 --- a/Userland/Libraries/LibJS/Runtime/GlobalObject.cpp +++ b/Userland/Libraries/LibJS/Runtime/GlobalObject.cpp @@ -223,26 +223,41 @@ JS_DEFINE_NATIVE_FUNCTION(GlobalObject::is_finite) // 19.2.4 parseFloat ( string ), https://tc39.es/ecma262/#sec-parsefloat-string JS_DEFINE_NATIVE_FUNCTION(GlobalObject::parse_float) { + // OPTIMIZATION: We can skip the number-to-string-to-number round trip when the value is already a number. if (vm.argument(0).is_number()) return vm.argument(0); + + // 1. Let inputString be ? ToString(string). auto input_string = TRY(vm.argument(0).to_deprecated_string(vm)); - auto trimmed_string = MUST(trim_string(vm, PrimitiveString::create(vm, input_string), TrimMode::Left)); + + // 2. Let trimmedString be ! TrimString(inputString, start). + // NOTE: We TRY this operation only to propagate OOM errors. + auto trimmed_string = TRY(trim_string(vm, PrimitiveString::create(vm, input_string), TrimMode::Left)); if (trimmed_string.is_empty()) return js_nan(); - auto result = parse_first_floating_point(trimmed_string.characters(), trimmed_string.characters() + trimmed_string.length()); - if (result.parsed_value()) - return result.value; + // 3. If neither trimmedString nor any prefix of trimmedString satisfies the syntax of a StrDecimalLiteral (see 7.1.4.1), return NaN. + // 4. Let numberString be the longest prefix of trimmedString, which might be trimmedString itself, that satisfies the syntax of a StrDecimalLiteral. + // 5. Let parsedNumber be ParseText(StringToCodePoints(numberString), StrDecimalLiteral). + // 6. Assert: parsedNumber is a Parse Node. + // 7. Return StringNumericValue of parsedNumber. + auto trimmed_string_view = trimmed_string.bytes_as_string_view(); + auto const* begin = trimmed_string_view.characters_without_null_termination(); + auto const* end = begin + trimmed_string_view.length(); - bool starts_with_sign = trimmed_string[0] == '-' || trimmed_string[0] == '+'; - auto signless_view = starts_with_sign ? trimmed_string.substring_view(1) : trimmed_string.view(); - if (signless_view.starts_with("Infinity"sv, AK::CaseSensitivity::CaseSensitive)) { + auto parsed_number = parse_first_floating_point(begin, end); + if (parsed_number.parsed_value()) + return parsed_number.value; + + auto first_code_point = *trimmed_string.code_points().begin(); + if (first_code_point == '-' || first_code_point == '+') + trimmed_string_view = trimmed_string_view.substring_view(1); + + if (trimmed_string_view.starts_with("Infinity"sv, AK::CaseSensitivity::CaseSensitive)) { // Only an immediate - means we should return negative infinity - if (trimmed_string[0] == '-') - return js_negative_infinity(); - - return js_infinity(); + return first_code_point == '-' ? js_negative_infinity() : js_infinity(); } + return js_nan(); } @@ -253,17 +268,20 @@ JS_DEFINE_NATIVE_FUNCTION(GlobalObject::parse_int) auto input_string = TRY(vm.argument(0).to_deprecated_string(vm)); // 2. Let S be ! TrimString(inputString, start). - auto string = MUST(trim_string(vm, PrimitiveString::create(vm, input_string), TrimMode::Left)); + // NOTE: We TRY this operation only to propagate OOM errors. + auto string = TRY(trim_string(vm, PrimitiveString::create(vm, input_string), TrimMode::Left)); // 3. Let sign be 1. auto sign = 1; // 4. If S is not empty and the first code unit of S is the code unit 0x002D (HYPHEN-MINUS), set sign to -1. - if (!string.is_empty() && string[0] == 0x2D) + auto first_code_point = string.is_empty() ? OptionalNone {} : Optional { *string.code_points().begin() }; + if (first_code_point == 0x2Du) sign = -1; + // 5. If S is not empty and the first code unit of S is the code unit 0x002B (PLUS SIGN) or the code unit 0x002D (HYPHEN-MINUS), remove the first code unit from S. - auto trimmed_view = string.view(); - if (!string.is_empty() && (string[0] == 0x2B || string[0] == 0x2D)) + auto trimmed_view = string.bytes_as_string_view(); + if (first_code_point == 0x2Bu || first_code_point == 0x2Du) trimmed_view = trimmed_view.substring_view(1); // 6. Let R be ℝ(? ToInt32(radix)). diff --git a/Userland/Libraries/LibJS/Runtime/StringPrototype.cpp b/Userland/Libraries/LibJS/Runtime/StringPrototype.cpp index eac68af686..c2788deadd 100644 --- a/Userland/Libraries/LibJS/Runtime/StringPrototype.cpp +++ b/Userland/Libraries/LibJS/Runtime/StringPrototype.cpp @@ -1025,13 +1025,13 @@ JS_DEFINE_NATIVE_FUNCTION(StringPrototype::to_well_formed) return PrimitiveString::create(vm, TRY(result.to_string())); } -ThrowCompletionOr trim_string(VM& vm, Value input_value, TrimMode where) +ThrowCompletionOr trim_string(VM& vm, Value input_value, TrimMode where) { // 1. Let str be ? RequireObjectCoercible(string). auto input_string = TRY(require_object_coercible(vm, input_value)); // 2. Let S be ? ToString(str). - auto string = TRY(input_string.to_deprecated_string(vm)); + auto string = TRY(input_string.to_string(vm)); // 3. If where is start, let T be the String value that is a copy of S with leading white space removed. // 4. Else if where is end, let T be the String value that is a copy of S with trailing white space removed. @@ -1041,7 +1041,7 @@ ThrowCompletionOr trim_string(VM& vm, Value input_value, TrimM auto trimmed_string = Utf8View(string).trim(whitespace_characters, where).as_string(); // 6. Return T. - return trimmed_string; + return TRY_OR_THROW_OOM(vm, String::from_utf8(trimmed_string)); } // 22.1.3.30 String.prototype.trim ( ), https://tc39.es/ecma262/#sec-string.prototype.trim diff --git a/Userland/Libraries/LibJS/Runtime/StringPrototype.h b/Userland/Libraries/LibJS/Runtime/StringPrototype.h index 71ac983a5e..76670eb3a8 100644 --- a/Userland/Libraries/LibJS/Runtime/StringPrototype.h +++ b/Userland/Libraries/LibJS/Runtime/StringPrototype.h @@ -19,7 +19,7 @@ struct CodePoint { CodePoint code_point_at(Utf16View const& string, size_t position); static constexpr Utf8View whitespace_characters = Utf8View("\x09\x0A\x0B\x0C\x0D\x20\xC2\xA0\xE1\x9A\x80\xE2\x80\x80\xE2\x80\x81\xE2\x80\x82\xE2\x80\x83\xE2\x80\x84\xE2\x80\x85\xE2\x80\x86\xE2\x80\x87\xE2\x80\x88\xE2\x80\x89\xE2\x80\x8A\xE2\x80\xAF\xE2\x81\x9F\xE3\x80\x80\xE2\x80\xA8\xE2\x80\xA9\xEF\xBB\xBF"sv); -ThrowCompletionOr trim_string(VM&, Value string, TrimMode where); +ThrowCompletionOr trim_string(VM&, Value string, TrimMode where); class StringPrototype final : public StringObject { JS_OBJECT(StringPrototype, StringObject);