Skip to content

Commit 7a648a3

Browse files
committed
Bug 1701695 - Add NumberFormat::TryCreate; r=tcampbell
This adds a fallible factory method to create new NumberFormat instances. This allows us to report initialization errors at time of initialization, rather than when format is called, and remove internal checks in the implementation for successful initialization. The existing fluent code assumes that creating a NumberFormat instance always succeeds. This patch updates that code to handle failures. Differential Revision: https://phabricator.services.mozilla.com/D114593
1 parent 6eaea87 commit 7a648a3

File tree

7 files changed

+141
-79
lines changed

7 files changed

+141
-79
lines changed

config/check_vanilla_allocations.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -160,6 +160,10 @@ def main():
160160
if filename == "Decimal.o":
161161
continue
162162

163+
# Ignore allocations from the m-c intl/components implementations.
164+
if "intl_components" in filename:
165+
continue
166+
163167
fn = m.group(2)
164168
if filename == "Utility.o":
165169
util_Utility_cpp.add(fn)

intl/components/gtest/TestNumberFormat.cpp

Lines changed: 41 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -30,49 +30,54 @@ class Buffer {
3030

3131
TEST(IntlNumberFormat, Basic)
3232
{
33-
NumberFormat nf("en-US");
33+
NumberFormatOptions options;
34+
UniquePtr<NumberFormat> nf =
35+
NumberFormat::TryCreate("en-US", options).unwrap();
3436
Buffer<uint8_t> buf8;
35-
ASSERT_TRUE(nf.format(1234.56, buf8).isOk());
37+
ASSERT_TRUE(nf->format(1234.56, buf8).isOk());
3638
ASSERT_EQ(
3739
std::string_view(static_cast<const char*>(buf8.data()), buf8.mWritten),
3840
"1,234.56");
3941
Buffer<char16_t> buf16;
40-
ASSERT_TRUE(nf.format(1234.56, buf16).isOk());
42+
ASSERT_TRUE(nf->format(1234.56, buf16).isOk());
4143
ASSERT_EQ(std::u16string_view(static_cast<const char16_t*>(buf16.data()),
4244
buf16.mWritten),
4345
u"1,234.56");
44-
const char16_t* res16 = nf.format(1234.56).unwrap().data();
46+
const char16_t* res16 = nf->format(1234.56).unwrap().data();
4547
ASSERT_TRUE(res16 != nullptr);
4648
ASSERT_EQ(std::u16string_view(res16), u"1,234.56");
4749

48-
NumberFormat nfAr("ar");
49-
ASSERT_TRUE(nfAr.format(1234.56, buf8).isOk());
50+
UniquePtr<NumberFormat> nfAr =
51+
NumberFormat::TryCreate("ar", options).unwrap();
52+
ASSERT_TRUE(nfAr->format(1234.56, buf8).isOk());
5053
ASSERT_EQ(
5154
std::string_view(static_cast<const char*>(buf8.data()), buf8.mWritten),
5255
"١٬٢٣٤٫٥٦");
53-
ASSERT_TRUE(nfAr.format(1234.56, buf16).isOk());
56+
ASSERT_TRUE(nfAr->format(1234.56, buf16).isOk());
5457
ASSERT_EQ(std::u16string_view(static_cast<const char16_t*>(buf16.data()),
5558
buf16.mWritten),
5659
u"١٬٢٣٤٫٥٦");
57-
res16 = nfAr.format(1234.56).unwrap().data();
60+
res16 = nfAr->format(1234.56).unwrap().data();
5861
ASSERT_TRUE(res16 != nullptr);
5962
ASSERT_EQ(std::u16string_view(res16), u"١٬٢٣٤٫٥٦");
6063
}
6164

6265
TEST(IntlNumberFormat, Numbers)
6366
{
64-
NumberFormat nf("es-ES");
67+
NumberFormatOptions options;
68+
UniquePtr<NumberFormat> nf =
69+
NumberFormat::TryCreate("es-ES", options).unwrap();
6570
Buffer<uint8_t> buf8;
66-
ASSERT_TRUE(nf.format(123456.789, buf8).isOk());
71+
ASSERT_TRUE(nf->format(123456.789, buf8).isOk());
6772
ASSERT_EQ(
6873
std::string_view(static_cast<const char*>(buf8.data()), buf8.mWritten),
6974
"123.456,789");
7075
Buffer<char16_t> buf16;
71-
ASSERT_TRUE(nf.format(123456.789, buf16).isOk());
76+
ASSERT_TRUE(nf->format(123456.789, buf16).isOk());
7277
ASSERT_EQ(std::u16string_view(static_cast<const char16_t*>(buf16.data()),
7378
buf16.mWritten),
7479
u"123.456,789");
75-
const char16_t* res = nf.format(123456.789).unwrap().data();
80+
const char16_t* res = nf->format(123456.789).unwrap().data();
7681
ASSERT_TRUE(res != nullptr);
7782
ASSERT_EQ(std::u16string_view(res), u"123.456,789");
7883
}
@@ -81,13 +86,14 @@ TEST(IntlNumberFormat, SignificantDigits)
8186
{
8287
NumberFormatOptions options;
8388
options.mSignificantDigits = Some(std::make_pair(3, 5));
84-
NumberFormat nf("es-ES", options);
89+
UniquePtr<NumberFormat> nf =
90+
NumberFormat::TryCreate("es-ES", options).unwrap();
8591
Buffer<uint8_t> buf8;
86-
ASSERT_TRUE(nf.format(123456.789, buf8).isOk());
92+
ASSERT_TRUE(nf->format(123456.789, buf8).isOk());
8793
ASSERT_EQ(
8894
std::string_view(static_cast<const char*>(buf8.data()), buf8.mWritten),
8995
"123.460");
90-
ASSERT_TRUE(nf.format(0.7, buf8).isOk());
96+
ASSERT_TRUE(nf->format(0.7, buf8).isOk());
9197
ASSERT_EQ(
9298
std::string_view(static_cast<const char*>(buf8.data()), buf8.mWritten),
9399
"0,700");
@@ -98,18 +104,19 @@ TEST(IntlNumberFormat, Currency)
98104
NumberFormatOptions options;
99105
options.mCurrency =
100106
Some(std::make_pair("MXN", NumberFormatOptions::CurrencyDisplay::Symbol));
101-
NumberFormat nf("es-MX", options);
107+
UniquePtr<NumberFormat> nf =
108+
NumberFormat::TryCreate("es-MX", options).unwrap();
102109
Buffer<uint8_t> buf8;
103-
ASSERT_TRUE(nf.format(123456.789, buf8).isOk());
110+
ASSERT_TRUE(nf->format(123456.789, buf8).isOk());
104111
ASSERT_EQ(
105112
std::string_view(static_cast<const char*>(buf8.data()), buf8.mWritten),
106113
"$123,456.79");
107114
Buffer<char16_t> buf16;
108-
ASSERT_TRUE(nf.format(123456.789, buf16).isOk());
115+
ASSERT_TRUE(nf->format(123456.789, buf16).isOk());
109116
ASSERT_EQ(std::u16string_view(static_cast<const char16_t*>(buf16.data()),
110117
buf16.mWritten),
111118
u"$123,456.79");
112-
const char16_t* res = nf.format(123456.789).unwrap().data();
119+
const char16_t* res = nf->format(123456.789).unwrap().data();
113120
ASSERT_TRUE(res != nullptr);
114121
ASSERT_EQ(std::u16string_view(res), u"$123,456.79");
115122
}
@@ -119,18 +126,19 @@ TEST(IntlNumberFormat, Unit)
119126
NumberFormatOptions options;
120127
options.mUnit = Some(std::make_pair("meter-per-second",
121128
NumberFormatOptions::UnitDisplay::Long));
122-
NumberFormat nf("es-MX", options);
129+
UniquePtr<NumberFormat> nf =
130+
NumberFormat::TryCreate("es-MX", options).unwrap();
123131
Buffer<uint8_t> buf8;
124-
ASSERT_TRUE(nf.format(12.34, buf8).isOk());
132+
ASSERT_TRUE(nf->format(12.34, buf8).isOk());
125133
ASSERT_EQ(
126134
std::string_view(static_cast<const char*>(buf8.data()), buf8.mWritten),
127135
"12.34 metros por segundo");
128136
Buffer<char16_t> buf16;
129-
ASSERT_TRUE(nf.format(12.34, buf16).isOk());
137+
ASSERT_TRUE(nf->format(12.34, buf16).isOk());
130138
ASSERT_EQ(std::u16string_view(static_cast<const char16_t*>(buf16.data()),
131139
buf16.mWritten),
132140
u"12.34 metros por segundo");
133-
const char16_t* res = nf.format(12.34).unwrap().data();
141+
const char16_t* res = nf->format(12.34).unwrap().data();
134142
ASSERT_TRUE(res != nullptr);
135143
ASSERT_EQ(std::u16string_view(res), u"12.34 metros por segundo");
136144

@@ -139,24 +147,28 @@ TEST(IntlNumberFormat, Unit)
139147
const char* unit = "meter-per-second-with-some-trailing-garbage";
140148
options.mUnit = Some(std::make_pair(std::string_view(unit, 5),
141149
NumberFormatOptions::UnitDisplay::Long));
142-
NumberFormat nf2("es-MX", options);
143-
res = nf2.format(12.34).unwrap().data();
150+
UniquePtr<NumberFormat> nf2 =
151+
NumberFormat::TryCreate("es-MX", options).unwrap();
152+
res = nf2->format(12.34).unwrap().data();
144153
ASSERT_TRUE(res != nullptr);
145154
ASSERT_EQ(std::u16string_view(res), u"12.34 metros");
146155

147156
options.mUnit = Some(std::make_pair(std::string_view(unit, 16),
148157
NumberFormatOptions::UnitDisplay::Long));
149-
NumberFormat nf3("es-MX", options);
150-
res = nf3.format(12.34).unwrap().data();
158+
UniquePtr<NumberFormat> nf3 =
159+
NumberFormat::TryCreate("es-MX", options).unwrap();
160+
res = nf3->format(12.34).unwrap().data();
151161
ASSERT_TRUE(res != nullptr);
152162
ASSERT_EQ(std::u16string_view(res), u"12.34 metros por segundo");
153163
}
154164

155165
TEST(IntlNumberFormat, FormatToParts)
156166
{
157-
NumberFormat nf("es-ES");
167+
NumberFormatOptions options;
168+
UniquePtr<NumberFormat> nf =
169+
NumberFormat::TryCreate("es-ES", options).unwrap();
158170
NumberPartVector parts;
159-
const char16_t* res = nf.formatToParts(123456.789, parts).unwrap().data();
171+
const char16_t* res = nf->formatToParts(123456.789, parts).unwrap().data();
160172
ASSERT_TRUE(res != nullptr);
161173
ASSERT_EQ(std::u16string_view(res), u"123.456,789");
162174
ASSERT_EQ(parts.length(), 5U);

intl/components/src/NumberFormat.cpp

Lines changed: 24 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -11,18 +11,16 @@
1111
namespace mozilla {
1212
namespace intl {
1313

14-
NumberFormat::NumberFormat(std::string_view aLocale,
15-
const NumberFormatOptions& aOptions) {
16-
mFormatForUnit = aOptions.mUnit.isSome();
17-
NumberFormatterSkeleton skeleton(aOptions);
18-
mNumberFormatter = skeleton.toFormatter(aLocale);
19-
if (mNumberFormatter) {
20-
UErrorCode status = U_ZERO_ERROR;
21-
mFormattedNumber = unumf_openResult(&status);
22-
if (U_SUCCESS(status)) {
23-
mIsInitialized = true;
24-
}
14+
/*static*/ Result<UniquePtr<NumberFormat>, NumberFormat::FormatError>
15+
NumberFormat::TryCreate(std::string_view aLocale,
16+
const NumberFormatOptions& aOptions) {
17+
UniquePtr<NumberFormat> nf = MakeUnique<NumberFormat>();
18+
Result<Ok, FormatError> result = nf->initialize(aLocale, aOptions);
19+
if (result.isOk()) {
20+
return nf;
2521
}
22+
23+
return Err(result.unwrapErr());
2624
}
2725

2826
NumberFormat::~NumberFormat() {
@@ -34,6 +32,21 @@ NumberFormat::~NumberFormat() {
3432
}
3533
}
3634

35+
Result<Ok, NumberFormat::FormatError> NumberFormat::initialize(
36+
std::string_view aLocale, const NumberFormatOptions& aOptions) {
37+
mFormatForUnit = aOptions.mUnit.isSome();
38+
NumberFormatterSkeleton skeleton(aOptions);
39+
mNumberFormatter = skeleton.toFormatter(aLocale);
40+
if (mNumberFormatter) {
41+
UErrorCode status = U_ZERO_ERROR;
42+
mFormattedNumber = unumf_openResult(&status);
43+
if (U_SUCCESS(status)) {
44+
return Ok();
45+
}
46+
}
47+
return Err(FormatError::InternalError);
48+
}
49+
3750
bool NumberFormat::formatInternal(double number) const {
3851
// ICU incorrectly formats NaN values with the sign bit set, as if they
3952
// were negative. Replace all NaNs with a single pattern with sign bit

intl/components/src/NumberFormat.h

Lines changed: 26 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -174,22 +174,25 @@ using NumberPartVector = mozilla::Vector<NumberPart, 8 * sizeof(NumberPart)>;
174174
*/
175175
class NumberFormat final {
176176
public:
177+
enum class FormatError {
178+
InternalError,
179+
OutOfMemory,
180+
};
181+
177182
/**
178183
* Initialize a new NumberFormat for the provided locale and using the
179184
* provided options.
180185
*
181186
* https://tc39.es/ecma402/#sec-initializenumberformat
182187
*/
183-
explicit NumberFormat(std::string_view aLocale,
184-
const NumberFormatOptions& aOptions = {});
188+
static Result<UniquePtr<NumberFormat>, NumberFormat::FormatError> TryCreate(
189+
std::string_view aLocale, const NumberFormatOptions& aOptions);
185190

191+
NumberFormat() = default;
192+
NumberFormat(const NumberFormat&) = delete;
193+
NumberFormat& operator=(const NumberFormat&) = delete;
186194
~NumberFormat();
187195

188-
enum class FormatError {
189-
InternalError,
190-
OutOfMemory,
191-
};
192-
193196
/**
194197
* Formats a double to a utf-16 string. The string view is valid until
195198
* another number is formatted. Accessing the string view after this event
@@ -199,7 +202,7 @@ class NumberFormat final {
199202
*/
200203
Result<std::u16string_view, NumberFormat::FormatError> format(
201204
double number) const {
202-
if (!mIsInitialized || !formatInternal(number)) {
205+
if (!formatInternal(number)) {
203206
return Err(FormatError::InternalError);
204207
}
205208

@@ -215,7 +218,7 @@ class NumberFormat final {
215218
*/
216219
Result<std::u16string_view, NumberFormat::FormatError> formatToParts(
217220
double number, NumberPartVector& parts) const {
218-
if (!mIsInitialized || !formatInternal(number)) {
221+
if (!formatInternal(number)) {
219222
return Err(FormatError::InternalError);
220223
}
221224

@@ -231,7 +234,7 @@ class NumberFormat final {
231234
*/
232235
template <typename B>
233236
Result<Ok, NumberFormat::FormatError> format(double number, B& buffer) const {
234-
if (!mIsInitialized || !formatInternal(number)) {
237+
if (!formatInternal(number)) {
235238
return Err(FormatError::InternalError);
236239
}
237240

@@ -247,7 +250,7 @@ class NumberFormat final {
247250
*/
248251
Result<std::u16string_view, NumberFormat::FormatError> format(
249252
int64_t number) const {
250-
if (!mIsInitialized || !formatInternal(number)) {
253+
if (!formatInternal(number)) {
251254
return Err(FormatError::InternalError);
252255
}
253256

@@ -263,7 +266,7 @@ class NumberFormat final {
263266
*/
264267
Result<std::u16string_view, NumberFormat::FormatError> formatToParts(
265268
int64_t number, NumberPartVector& parts) const {
266-
if (!mIsInitialized || !formatInternal(number)) {
269+
if (!formatInternal(number)) {
267270
return Err(FormatError::InternalError);
268271
}
269272

@@ -278,7 +281,7 @@ class NumberFormat final {
278281
template <typename B>
279282
Result<Ok, NumberFormat::FormatError> format(int64_t number,
280283
B& buffer) const {
281-
if (!mIsInitialized || !formatInternal(number)) {
284+
if (!formatInternal(number)) {
282285
return Err(FormatError::InternalError);
283286
}
284287

@@ -294,7 +297,7 @@ class NumberFormat final {
294297
*/
295298
Result<std::u16string_view, NumberFormat::FormatError> format(
296299
std::string_view number) const {
297-
if (!mIsInitialized || !formatInternal(number)) {
300+
if (!formatInternal(number)) {
298301
return Err(FormatError::InternalError);
299302
}
300303

@@ -311,7 +314,7 @@ class NumberFormat final {
311314
*/
312315
Result<std::u16string_view, NumberFormat::FormatError> formatToParts(
313316
std::string_view number, NumberPartVector& parts) const {
314-
if (!mIsInitialized || !formatInternal(number)) {
317+
if (!formatInternal(number)) {
315318
return Err(FormatError::InternalError);
316319
}
317320

@@ -329,7 +332,7 @@ class NumberFormat final {
329332
template <typename B>
330333
Result<Ok, NumberFormat::FormatError> format(std::string_view number,
331334
B& buffer) const {
332-
if (!mIsInitialized || !formatInternal(number)) {
335+
if (!formatInternal(number)) {
333336
return Err(FormatError::InternalError);
334337
}
335338

@@ -340,7 +343,9 @@ class NumberFormat final {
340343
UNumberFormatter* mNumberFormatter = nullptr;
341344
UFormattedNumber* mFormattedNumber = nullptr;
342345
bool mFormatForUnit = false;
343-
bool mIsInitialized = false;
346+
347+
Result<Ok, NumberFormat::FormatError> initialize(
348+
std::string_view aLocale, const NumberFormatOptions& aOptions);
344349

345350
[[nodiscard]] bool formatInternal(double number) const;
346351
[[nodiscard]] bool formatInternal(int64_t number) const;
@@ -357,6 +362,10 @@ class NumberFormat final {
357362

358363
template <typename C, typename B>
359364
Result<Ok, NumberFormat::FormatError> formatResult(B& buffer) const {
365+
// We only support buffers with uint8_t or char16_t for now.
366+
static_assert(std::is_same<C, uint8_t>::value ||
367+
std::is_same<C, char16_t>::value);
368+
360369
return formatResult().andThen([&buffer](std::u16string_view result)
361370
-> Result<Ok, NumberFormat::FormatError> {
362371
if constexpr (std::is_same<C, uint8_t>::value) {

intl/l10n/FluentBundle.cpp

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -254,8 +254,17 @@ ffi::RawNumberFormatter* FluentBuiltInNumberFormatterCreate(
254254
aOptions->minimum_fraction_digits, aOptions->maximum_fraction_digits));
255255
}
256256

257-
return reinterpret_cast<ffi::RawNumberFormatter*>(
258-
new NumberFormat(aLocale->get(), options));
257+
Result<UniquePtr<NumberFormat>, NumberFormat::FormatError> result =
258+
NumberFormat::TryCreate(aLocale->get(), options);
259+
260+
MOZ_ASSERT(result.isOk());
261+
262+
if (result.isOk()) {
263+
return reinterpret_cast<ffi::RawNumberFormatter*>(
264+
result.unwrap().release());
265+
}
266+
267+
return nullptr;
259268
}
260269

261270
uint8_t* FluentBuiltInNumberFormatterFormat(

0 commit comments

Comments
 (0)