LibJS: Store Intl mathematical values as strings when appropriate

The IntlMV is meant to be arbitrarily precise. If the user provides a
string value to be formatted, we lose precision by converting extremely
large values to a double. We were never able to address this, as support
for arbitrary precision was a big FIXME. But ICU can handle it by just
passing the raw string on through.
This commit is contained in:
Timothy Flynn 2024-06-09 20:01:21 -04:00 committed by Andreas Kling
parent f6bee0f5a8
commit 3b68bb6e73
Notes: sideshowbarker 2024-07-18 03:20:18 +09:00
5 changed files with 25 additions and 19 deletions

View file

@ -19,20 +19,20 @@ double MathematicalValue::as_number() const
return m_value.get<double>();
}
bool MathematicalValue::is_bigint() const
bool MathematicalValue::is_string() const
{
return m_value.has<Crypto::SignedBigInteger>();
return m_value.has<String>();
}
Crypto::SignedBigInteger const& MathematicalValue::as_bigint() const
String const& MathematicalValue::as_string() const
{
VERIFY(is_bigint());
return m_value.get<Crypto::SignedBigInteger>();
VERIFY(is_string());
return m_value.get<String>();
}
bool MathematicalValue::is_mathematical_value() const
{
return is_number() || is_bigint();
return is_number() || is_string();
}
bool MathematicalValue::is_positive_infinity() const
@ -69,8 +69,8 @@ bool MathematicalValue::is_nan() const
[](double value) -> ::Locale::NumberFormat::Value {
return value;
},
[](Crypto::SignedBigInteger const& value) -> ::Locale::NumberFormat::Value {
return MUST(value.to_base(10));
[](String const& value) -> ::Locale::NumberFormat::Value {
return value;
},
[](auto symbol) -> ::Locale::NumberFormat::Value {
switch (symbol) {

View file

@ -6,6 +6,7 @@
#pragma once
#include <AK/String.h>
#include <AK/Variant.h>
#include <LibCrypto/BigInt/SignedBigInteger.h>
#include <LibJS/Runtime/BigInt.h>
@ -31,7 +32,7 @@ public:
{
}
explicit MathematicalValue(Crypto::SignedBigInteger value)
explicit MathematicalValue(String value)
: m_value(move(value))
{
}
@ -44,15 +45,15 @@ public:
MathematicalValue(Value value)
: m_value(value.is_number()
? value_from_number(value.as_double())
: ValueType(value.as_bigint().big_integer()))
: ValueType(MUST(value.as_bigint().big_integer().to_base(10))))
{
}
bool is_number() const;
double as_number() const;
bool is_bigint() const;
Crypto::SignedBigInteger const& as_bigint() const;
bool is_string() const;
String const& as_string() const;
bool is_mathematical_value() const;
bool is_positive_infinity() const;
@ -63,7 +64,7 @@ public:
::Locale::NumberFormat::Value to_value() const;
private:
using ValueType = Variant<double, Crypto::SignedBigInteger, Symbol>;
using ValueType = Variant<double, String, Symbol>;
static ValueType value_from_number(double number);

View file

@ -192,7 +192,7 @@ ThrowCompletionOr<MathematicalValue> to_intl_mathematical_value(VM& vm, Value va
// 2. If Type(primValue) is BigInt, return the mathematical value of primValue.
if (primitive_value.is_bigint())
return primitive_value.as_bigint().big_integer();
return MUST(value.as_bigint().big_integer().to_base(10));
// FIXME: The remaining steps are being refactored into a new Runtime Semantic, StringIntlMV.
// We short-circuit some of these steps to avoid known pitfalls.
@ -212,6 +212,9 @@ ThrowCompletionOr<MathematicalValue> to_intl_mathematical_value(VM& vm, Value va
// 6. Let mv be the MV, a mathematical value, of ? ToNumber(str), as described in 7.1.4.1.1.
auto mathematical_value = TRY(primitive_value.to_number(vm)).as_double();
if (Value(mathematical_value).is_nan())
return MathematicalValue::Symbol::NotANumber;
// 7. If mv is 0 and the first non white space code point in str is -, return negative-zero.
if (mathematical_value == 0.0 && string.bytes_as_string_view().trim_whitespace(TrimMode::Left).starts_with('-'))
return MathematicalValue::Symbol::NegativeZero;
@ -225,7 +228,7 @@ ThrowCompletionOr<MathematicalValue> to_intl_mathematical_value(VM& vm, Value va
return MathematicalValue::Symbol::NegativeInfinity;
// 10. Return mv.
return mathematical_value;
return string;
}
// 15.5.19 PartitionNumberRangePattern ( numberFormat, x, y ), https://tc39.es/ecma402/#sec-partitionnumberrangepattern

View file

@ -46,11 +46,15 @@ describe("style=decimal", () => {
expect(en.format(1)).toBe("1");
expect(en.format(12)).toBe("12");
expect(en.format(123)).toBe("123");
expect(en.format("987654321987654321")).toBe("987,654,321,987,654,321");
const ar = new Intl.NumberFormat("ar");
expect(ar.format(1)).toBe("\u0661");
expect(ar.format(12)).toBe("\u0661\u0662");
expect(ar.format(123)).toBe("\u0661\u0662\u0663");
expect(ar.format("987654321987654321")).toBe(
"\u0669\u0668\u0667\u066c\u0666\u0665\u0664\u066c\u0663\u0662\u0661\u066c\u0669\u0668\u0667\u066c\u0666\u0665\u0664\u066c\u0663\u0662\u0661"
);
});
test("integer digits", () => {

View file

@ -753,10 +753,8 @@ private:
while (static_cast<bool>(formatted->nextPosition(position, status)) && icu_success(status)) {
if (position.getCategory() == UFIELD_CATEGORY_NUMBER_RANGE_SPAN) {
if (position.getField() == 0)
start_range.emplace(position.getField(), position.getStart(), position.getLimit());
else
end_range.emplace(position.getField(), position.getStart(), position.getLimit());
auto& range = position.getField() == 0 ? start_range : end_range;
range = Range { position.getField(), position.getStart(), position.getLimit() };
} else {
ranges.empend(position.getField(), position.getStart(), position.getLimit());
}