From 8c196b6f899e9ae70dde39e13be1522a4a70f566 Mon Sep 17 00:00:00 2001 From: Shane Carr Date: Thu, 14 Feb 2019 14:42:36 -0800 Subject: [PATCH] ICU-20380 Adding error code to DecimalFormat::toNumberFormatter(). --- icu4c/source/i18n/decimfmt.cpp | 23 ++++++----- icu4c/source/i18n/number_fluent.cpp | 6 +++ icu4c/source/i18n/unicode/decimfmt.h | 55 +++++++++++++++++++++---- icu4c/source/test/intltest/dcfmapts.cpp | 18 ++++---- 4 files changed, 76 insertions(+), 26 deletions(-) diff --git a/icu4c/source/i18n/decimfmt.cpp b/icu4c/source/i18n/decimfmt.cpp index 11edbad47da..741e7a3c0e4 100644 --- a/icu4c/source/i18n/decimfmt.cpp +++ b/icu4c/source/i18n/decimfmt.cpp @@ -1568,17 +1568,20 @@ void DecimalFormat::formatToDecimalQuantity(const Formattable& number, DecimalQu output = std::move(obj.quantity); } -const number::LocalizedNumberFormatter& DecimalFormat::toNumberFormatter() const { - // TODO: See ICU-20366 and ICU-20380. Currently there isn't really any good way to report an error here. - if (fields != nullptr) { - return *fields->formatter; +const number::LocalizedNumberFormatter* DecimalFormat::toNumberFormatter(UErrorCode& status) const { + // We sometimes need to return nullptr here (see ICU-20380) + if (U_FAILURE(status)) { return nullptr; } + if (fields == nullptr) { + // We only get here if an OOM error happend during construction, copy construction, assignment, or modification. + status = U_MEMORY_ALLOCATION_ERROR; + return nullptr; } - // The code below is undefined behavior and may crash anyways, but we don't really have any choice since - // this method returns a reference. - // TODO: Should we have a static-allocated LocalizedNumberFormatter (in a failed state) somewhere so that - // it could be used here? - number::LocalizedNumberFormatter *bad = nullptr; - return static_cast(*bad); + return &*fields->formatter; +} + +const number::LocalizedNumberFormatter& DecimalFormat::toNumberFormatter() const { + UErrorCode localStatus = U_ZERO_ERROR; + return *toNumberFormatter(localStatus); } /** Rebuilds the formatter object from the property bag. */ diff --git a/icu4c/source/i18n/number_fluent.cpp b/icu4c/source/i18n/number_fluent.cpp index 84e7d05ac59..ec3eebda01a 100644 --- a/icu4c/source/i18n/number_fluent.cpp +++ b/icu4c/source/i18n/number_fluent.cpp @@ -322,6 +322,9 @@ Derived NumberFormatterSettings::macros(impl::MacroProps&& macros)&& { template UnicodeString NumberFormatterSettings::toSkeleton(UErrorCode& status) const { + if (U_FAILURE(status)) { + return ICU_Utility::makeBogusString(); + } if (fMacros.copyErrorTo(status)) { return ICU_Utility::makeBogusString(); } @@ -764,6 +767,9 @@ int32_t LocalizedNumberFormatter::getCallCount() const { } Format* LocalizedNumberFormatter::toFormat(UErrorCode& status) const { + if (U_FAILURE(status)) { + return nullptr; + } LocalPointer retval( new LocalizedNumberFormatterAsFormat(*this, fMacros.locale), status); return retval.orphan(); diff --git a/icu4c/source/i18n/unicode/decimfmt.h b/icu4c/source/i18n/unicode/decimfmt.h index c585dec91eb..b95c63a06a3 100644 --- a/icu4c/source/i18n/unicode/decimfmt.h +++ b/icu4c/source/i18n/unicode/decimfmt.h @@ -2066,8 +2066,32 @@ class U_I18N_API DecimalFormat : public NumberFormat { #ifndef U_HIDE_DRAFT_API /** - * Converts this DecimalFormat to a NumberFormatter. Starting in ICU 60, - * NumberFormatter is the recommended way to format numbers. + * Converts this DecimalFormat to a (Localized)NumberFormatter. Starting + * in ICU 60, NumberFormatter is the recommended way to format numbers. + * You can use the returned LocalizedNumberFormatter to format numbers and + * get a FormattedNumber, which contains a string as well as additional + * annotations about the formatted value. + * + * If a memory allocation failure occurs, the return value of this method + * might be null. If you are concerned about correct recovery from + * out-of-memory situations, use this pattern: + * + * 
+     * FormattedNumber result;
+     * if (auto* ptr = df->toNumberFormatter(status)) {
+     *     result = ptr->formatDouble(123, status);
+     * }
+     *
+ * + * If you are not concerned about out-of-memory situations, or if your + * environment throws exceptions when memory allocation failure occurs, + * you can chain the methods, like this: + * + * 
+     * FormattedNumber result = df
+     *     ->toNumberFormatter(status)
+     *     ->formatDouble(123, status);
+     *
* * NOTE: The returned LocalizedNumberFormatter is owned by this DecimalFormat. * If a non-const method is called on the DecimalFormat, or if the DecimalFormat @@ -2075,17 +2099,30 @@ class U_I18N_API DecimalFormat : public NumberFormat { * beyond the lifetime of the DecimalFormat, copy it to a local variable: * * 
-     * LocalizedNumberFormatter f = df->toNumberFormatter();
+     * LocalizedNumberFormatter lnf;
+     * if (auto* ptr = df->toNumberFormatter(status)) {
+     *     lnf = *ptr;
+     * }
      *
* - * It is, however, safe to use the return value for chaining: + * @param status Set on failure, like U_MEMORY_ALLOCATION_ERROR. + * @return A pointer to an internal object, or nullptr on failure. + * Do not delete the return value! + * @draft ICU 64 + */ + const number::LocalizedNumberFormatter* toNumberFormatter(UErrorCode& status) const; + + /** + * Deprecated: Like {@link #toNumberFormatter(UErrorCode&) const}, + * but does not take an error code. * - * 
-     * FormattedNumber result = df->toNumberFormatter().formatDouble(123, status);
-     *
+ * The new signature should be used in case an error occurs while returning the + * LocalizedNumberFormatter. * - * @return The output variable, for chaining. - * @draft ICU 62 + * This old signature will be removed in ICU 65. + * + * @return A reference to an internal object. + * @deprecated ICU 64 */ const number::LocalizedNumberFormatter& toNumberFormatter() const; #endif /* U_HIDE_DRAFT_API */ diff --git a/icu4c/source/test/intltest/dcfmapts.cpp b/icu4c/source/test/intltest/dcfmapts.cpp index e79fada23b6..5973a9eb474 100644 --- a/icu4c/source/test/intltest/dcfmapts.cpp +++ b/icu4c/source/test/intltest/dcfmapts.cpp @@ -1368,13 +1368,17 @@ void IntlTestDecimalFormatAPI::testInvalidObject() { df->getCurrencyUsage(); - // TODO: See ICU-20366 and ICU-20380. - // This method should return a pointer, or at least have an error code parameter, - const number::LocalizedNumberFormatter& lnf = df->toNumberFormatter(); - (void)lnf; // suppress unused variable warning. - // Note: The existance of a null reference is undefined behavior in C++. - // Attempting to touch/examine a null reference is also undefined. Doing - // so generally will cause a crash or segmentation fault. + const number::LocalizedNumberFormatter* lnf = df->toNumberFormatter(status); + assertEquals("toNumberFormatter should return nullptr", + (int64_t) nullptr, (int64_t) lnf); + + // Should not crash when chaining to error code enabled methods on the LNF + lnf->formatInt(1, status); + lnf->formatDouble(1.0, status); + lnf->formatDecimal("1", status); + lnf->toFormat(status); + lnf->toSkeleton(status); + lnf->copyErrorTo(status); } }