ICU-20380 Adding error code to DecimalFormat::toNumberFormatter().

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<const number::LocalizedNumberFormatter &>(*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. */

icu4c/source/i18n/number_fluent.cpp

@@ -322,6 +322,9 @@ Derived NumberFormatterSettings<Derived>::macros(impl::MacroProps&& macros)&& {
 
 template<typename Derived>
 UnicodeString NumberFormatterSettings<Derived>::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<LocalizedNumberFormatterAsFormat> retval(
             new LocalizedNumberFormatterAsFormat(*this, fMacros.locale), status);
     return retval.orphan();

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:
+     *
+     * <pre>
+     * FormattedNumber result;
+     * if (auto* ptr = df->toNumberFormatter(status)) {
+     *     result = ptr->formatDouble(123, status);
+     * }
+     * </pre>
+     *
+     * 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:
+     *
+     * <pre>
+     * FormattedNumber result = df
+     *     ->toNumberFormatter(status)
+     *     ->formatDouble(123, status);
+     * </pre>
      *
      * 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:
      *
      * <pre>
-     * LocalizedNumberFormatter f = df->toNumberFormatter();
+     * LocalizedNumberFormatter lnf;
+     * if (auto* ptr = df->toNumberFormatter(status)) {
+     *     lnf = *ptr;
+     * }
      * </pre>
      *
-     * 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.
      *
-     * <pre>
-     * FormattedNumber result = df->toNumberFormatter().formatDouble(123, status);
-     * </pre>
+     * 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 */

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);
         }
 
     }