From: Shane Carr Date: Wed, 25 Oct 2017 01:26:48 +0000 (+0000) Subject: ICU-13415 Setting error code in terminal NumberFormatter methods when applicable... X-Git-Tag: release-61-rc~205 X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=1ba787537131f2ec29ba8af4d576abf58df18a79;p=icu ICU-13415 Setting error code in terminal NumberFormatter methods when applicable. Renaming unproposed error codes for consistency with existing error codes in utypes.h. X-SVN-Rev: 40632 --- diff --git a/icu4c/source/i18n/number_fluent.cpp b/icu4c/source/i18n/number_fluent.cpp index 1a29eb8c177..76c3a7ce5c5 100644 --- a/icu4c/source/i18n/number_fluent.cpp +++ b/icu4c/source/i18n/number_fluent.cpp @@ -247,33 +247,33 @@ LocalizedNumberFormatter::~LocalizedNumberFormatter() { } FormattedNumber LocalizedNumberFormatter::formatInt(int64_t value, UErrorCode &status) const { - if (U_FAILURE(status)) { return FormattedNumber(); } + if (U_FAILURE(status)) { return FormattedNumber(U_ILLEGAL_ARGUMENT_ERROR); } auto results = new NumberFormatterResults(); if (results == nullptr) { status = U_MEMORY_ALLOCATION_ERROR; - return FormattedNumber(); + return FormattedNumber(status); } results->quantity.setToLong(value); return formatImpl(results, status); } FormattedNumber LocalizedNumberFormatter::formatDouble(double value, UErrorCode &status) const { - if (U_FAILURE(status)) { return FormattedNumber(); } + if (U_FAILURE(status)) { return FormattedNumber(U_ILLEGAL_ARGUMENT_ERROR); } auto results = new NumberFormatterResults(); if (results == nullptr) { status = U_MEMORY_ALLOCATION_ERROR; - return FormattedNumber(); + return FormattedNumber(status); } results->quantity.setToDouble(value); return formatImpl(results, status); } FormattedNumber LocalizedNumberFormatter::formatDecimal(StringPiece value, UErrorCode &status) const { - if (U_FAILURE(status)) { return FormattedNumber(); } + if (U_FAILURE(status)) { return FormattedNumber(U_ILLEGAL_ARGUMENT_ERROR); } auto results = new NumberFormatterResults(); if (results == nullptr) { status = U_MEMORY_ALLOCATION_ERROR; - return FormattedNumber(); + return FormattedNumber(status); } results->quantity.setToDecNumber(value); return formatImpl(results, status); @@ -317,28 +317,48 @@ LocalizedNumberFormatter::formatImpl(impl::NumberFormatterResults *results, UErr NumberFormatterImpl::applyStatic(fMacros, results->quantity, results->string, status); } - return FormattedNumber(results); + // Do not save the results object if we encountered a failure. + if (U_SUCCESS(status)) { + return FormattedNumber(results); + } else { + delete results; + return FormattedNumber(status); + } } UnicodeString FormattedNumber::toString() const { - if (fResults == nullptr) { return {}; } + if (fResults == nullptr) { + // TODO: http://bugs.icu-project.org/trac/ticket/13437 + return {}; + } return fResults->string.toUnicodeString(); } Appendable &FormattedNumber::appendTo(Appendable &appendable) { - if (fResults == nullptr) { return appendable; } + if (fResults == nullptr) { + // TODO: http://bugs.icu-project.org/trac/ticket/13437 + return appendable; + } appendable.appendString(fResults->string.chars(), fResults->string.length()); return appendable; } void FormattedNumber::populateFieldPosition(FieldPosition &fieldPosition, UErrorCode &status) { - if (fResults == nullptr) { return; } + if (U_FAILURE(status)) { return; } + if (fResults == nullptr) { + status = fErrorCode; + return; + } fResults->string.populateFieldPosition(fieldPosition, 0, status); } void FormattedNumber::populateFieldPositionIterator(FieldPositionIterator &iterator, UErrorCode &status) { - if (fResults == nullptr) { return; } + if (U_FAILURE(status)) { return; } + if (fResults == nullptr) { + status = fErrorCode; + return; + } fResults->string.populateFieldPositionIterator(iterator, status); } diff --git a/icu4c/source/i18n/number_integerwidth.cpp b/icu4c/source/i18n/number_integerwidth.cpp index 61a74acf6c7..10dacfc4acb 100644 --- a/icu4c/source/i18n/number_integerwidth.cpp +++ b/icu4c/source/i18n/number_integerwidth.cpp @@ -22,7 +22,7 @@ IntegerWidth IntegerWidth::zeroFillTo(int32_t minInt) { if (minInt >= 0 && minInt <= kMaxIntFracSig) { return {static_cast(minInt), -1}; } else { - return {U_NUMBER_DIGIT_WIDTH_OUT_OF_RANGE_ERROR}; + return {U_NUMBER_DIGIT_WIDTH_OUTOFBOUNDS_ERROR}; } } @@ -31,7 +31,7 @@ IntegerWidth IntegerWidth::truncateAt(int32_t maxInt) { if (maxInt >= 0 && maxInt <= kMaxIntFracSig) { return {fUnion.minMaxInt.fMinInt, static_cast(maxInt)}; } else { - return {U_NUMBER_DIGIT_WIDTH_OUT_OF_RANGE_ERROR}; + return {U_NUMBER_DIGIT_WIDTH_OUTOFBOUNDS_ERROR}; } } diff --git a/icu4c/source/i18n/number_notation.cpp b/icu4c/source/i18n/number_notation.cpp index b5a7ee5bcb5..ff0cd9505de 100644 --- a/icu4c/source/i18n/number_notation.cpp +++ b/icu4c/source/i18n/number_notation.cpp @@ -60,7 +60,7 @@ ScientificNotation::withMinExponentDigits(int32_t minExponentDigits) const { NotationUnion union_ = {settings}; return {NTN_SCIENTIFIC, union_}; } else { - return {U_NUMBER_DIGIT_WIDTH_OUT_OF_RANGE_ERROR}; + return {U_NUMBER_DIGIT_WIDTH_OUTOFBOUNDS_ERROR}; } } diff --git a/icu4c/source/i18n/number_padding.cpp b/icu4c/source/i18n/number_padding.cpp index e5e2f8603ef..a478af60541 100644 --- a/icu4c/source/i18n/number_padding.cpp +++ b/icu4c/source/i18n/number_padding.cpp @@ -43,7 +43,7 @@ Padder Padder::codePoints(UChar32 cp, int32_t targetWidth, UNumberFormatPadPosit if (targetWidth >= 0) { return {cp, targetWidth, position}; } else { - return {U_NUMBER_PADDING_WIDTH_OUT_OF_RANGE_ERROR}; + return {U_NUMBER_PADDING_WIDTH_OUTOFBOUNDS_ERROR}; } } diff --git a/icu4c/source/i18n/number_rounding.cpp b/icu4c/source/i18n/number_rounding.cpp index 38b40ab3d9e..5c494f09544 100644 --- a/icu4c/source/i18n/number_rounding.cpp +++ b/icu4c/source/i18n/number_rounding.cpp @@ -58,7 +58,7 @@ FractionRounder Rounder::fixedFraction(int32_t minMaxFractionPlaces) { if (minMaxFractionPlaces >= 0 && minMaxFractionPlaces <= kMaxIntFracSig) { return constructFraction(minMaxFractionPlaces, minMaxFractionPlaces); } else { - return {U_NUMBER_DIGIT_WIDTH_OUT_OF_RANGE_ERROR}; + return {U_NUMBER_DIGIT_WIDTH_OUTOFBOUNDS_ERROR}; } } @@ -66,7 +66,7 @@ FractionRounder Rounder::minFraction(int32_t minFractionPlaces) { if (minFractionPlaces >= 0 && minFractionPlaces <= kMaxIntFracSig) { return constructFraction(minFractionPlaces, -1); } else { - return {U_NUMBER_DIGIT_WIDTH_OUT_OF_RANGE_ERROR}; + return {U_NUMBER_DIGIT_WIDTH_OUTOFBOUNDS_ERROR}; } } @@ -74,7 +74,7 @@ FractionRounder Rounder::maxFraction(int32_t maxFractionPlaces) { if (maxFractionPlaces >= 0 && maxFractionPlaces <= kMaxIntFracSig) { return constructFraction(0, maxFractionPlaces); } else { - return {U_NUMBER_DIGIT_WIDTH_OUT_OF_RANGE_ERROR}; + return {U_NUMBER_DIGIT_WIDTH_OUTOFBOUNDS_ERROR}; } } @@ -83,7 +83,7 @@ FractionRounder Rounder::minMaxFraction(int32_t minFractionPlaces, int32_t maxFr minFractionPlaces <= maxFractionPlaces) { return constructFraction(minFractionPlaces, maxFractionPlaces); } else { - return {U_NUMBER_DIGIT_WIDTH_OUT_OF_RANGE_ERROR}; + return {U_NUMBER_DIGIT_WIDTH_OUTOFBOUNDS_ERROR}; } } @@ -91,7 +91,7 @@ Rounder Rounder::fixedDigits(int32_t minMaxSignificantDigits) { if (minMaxSignificantDigits >= 0 && minMaxSignificantDigits <= kMaxIntFracSig) { return constructSignificant(minMaxSignificantDigits, minMaxSignificantDigits); } else { - return {U_NUMBER_DIGIT_WIDTH_OUT_OF_RANGE_ERROR}; + return {U_NUMBER_DIGIT_WIDTH_OUTOFBOUNDS_ERROR}; } } @@ -99,7 +99,7 @@ Rounder Rounder::minDigits(int32_t minSignificantDigits) { if (minSignificantDigits >= 0 && minSignificantDigits <= kMaxIntFracSig) { return constructSignificant(minSignificantDigits, -1); } else { - return {U_NUMBER_DIGIT_WIDTH_OUT_OF_RANGE_ERROR}; + return {U_NUMBER_DIGIT_WIDTH_OUTOFBOUNDS_ERROR}; } } @@ -107,7 +107,7 @@ Rounder Rounder::maxDigits(int32_t maxSignificantDigits) { if (maxSignificantDigits >= 0 && maxSignificantDigits <= kMaxIntFracSig) { return constructSignificant(0, maxSignificantDigits); } else { - return {U_NUMBER_DIGIT_WIDTH_OUT_OF_RANGE_ERROR}; + return {U_NUMBER_DIGIT_WIDTH_OUTOFBOUNDS_ERROR}; } } @@ -116,7 +116,7 @@ Rounder Rounder::minMaxDigits(int32_t minSignificantDigits, int32_t maxSignifica minSignificantDigits <= maxSignificantDigits) { return constructSignificant(minSignificantDigits, maxSignificantDigits); } else { - return {U_NUMBER_DIGIT_WIDTH_OUT_OF_RANGE_ERROR}; + return {U_NUMBER_DIGIT_WIDTH_OUTOFBOUNDS_ERROR}; } } @@ -124,7 +124,7 @@ IncrementRounder Rounder::increment(double roundingIncrement) { if (roundingIncrement > 0.0) { return constructIncrement(roundingIncrement, 0); } else { - return {U_NUMBER_DIGIT_WIDTH_OUT_OF_RANGE_ERROR}; + return {U_NUMBER_DIGIT_WIDTH_OUTOFBOUNDS_ERROR}; } } @@ -142,7 +142,7 @@ Rounder FractionRounder::withMinDigits(int32_t minSignificantDigits) const { if (minSignificantDigits >= 0 && minSignificantDigits <= kMaxIntFracSig) { return constructFractionSignificant(*this, minSignificantDigits, -1); } else { - return {U_NUMBER_DIGIT_WIDTH_OUT_OF_RANGE_ERROR}; + return {U_NUMBER_DIGIT_WIDTH_OUTOFBOUNDS_ERROR}; } } @@ -151,7 +151,7 @@ Rounder FractionRounder::withMaxDigits(int32_t maxSignificantDigits) const { if (maxSignificantDigits >= 0 && maxSignificantDigits <= kMaxIntFracSig) { return constructFractionSignificant(*this, -1, maxSignificantDigits); } else { - return {U_NUMBER_DIGIT_WIDTH_OUT_OF_RANGE_ERROR}; + return {U_NUMBER_DIGIT_WIDTH_OUTOFBOUNDS_ERROR}; } } @@ -185,7 +185,7 @@ Rounder IncrementRounder::withMinFraction(int32_t minFrac) const { if (minFrac >= 0 && minFrac <= kMaxIntFracSig) { return constructIncrement(fUnion.increment.fIncrement, minFrac); } else { - return {U_NUMBER_DIGIT_WIDTH_OUT_OF_RANGE_ERROR}; + return {U_NUMBER_DIGIT_WIDTH_OUTOFBOUNDS_ERROR}; } } diff --git a/icu4c/source/i18n/number_types.h b/icu4c/source/i18n/number_types.h index a595b66a4e2..2bc21bd40dc 100644 --- a/icu4c/source/i18n/number_types.h +++ b/icu4c/source/i18n/number_types.h @@ -43,8 +43,8 @@ static constexpr char16_t kFallbackPaddingString[] = u" "; static constexpr char16_t kDefaultCurrency[] = u"XXX"; // FIXME: New error codes: -static constexpr UErrorCode U_NUMBER_DIGIT_WIDTH_OUT_OF_RANGE_ERROR = U_ILLEGAL_ARGUMENT_ERROR; -static constexpr UErrorCode U_NUMBER_PADDING_WIDTH_OUT_OF_RANGE_ERROR = U_ILLEGAL_ARGUMENT_ERROR; +static constexpr UErrorCode U_NUMBER_DIGIT_WIDTH_OUTOFBOUNDS_ERROR = U_ILLEGAL_ARGUMENT_ERROR; +static constexpr UErrorCode U_NUMBER_PADDING_WIDTH_OUTOFBOUNDS_ERROR = U_ILLEGAL_ARGUMENT_ERROR; // Forward declarations: diff --git a/icu4c/source/i18n/unicode/numberformatter.h b/icu4c/source/i18n/unicode/numberformatter.h index f469ba82151..4a11c2f9157 100644 --- a/icu4c/source/i18n/unicode/numberformatter.h +++ b/icu4c/source/i18n/unicode/numberformatter.h @@ -1853,8 +1853,8 @@ class U_I18N_API LocalizedNumberFormatter *

* This function is very hot, being called in every call to the number formatting pipeline. * - * @param fq - * The quantity to be formatted. + * @param results + * The results object. This method takes ownership. * @return The formatted number result. */ FormattedNumber formatImpl(impl::NumberFormatterResults *results, UErrorCode &status) const; @@ -1941,10 +1941,14 @@ class U_I18N_API FormattedNumber : public UMemory { // Can't use LocalPointer because NumberFormatterResults is forward-declared const impl::NumberFormatterResults *fResults; - // Default constructor for error states - FormattedNumber() : fResults(nullptr) {} + // Error code for the terminal methods + UErrorCode fErrorCode; - explicit FormattedNumber(impl::NumberFormatterResults *results) : fResults(results) {} + explicit FormattedNumber(impl::NumberFormatterResults *results) + : fResults(results), fErrorCode(U_ZERO_ERROR) {}; + + explicit FormattedNumber(UErrorCode errorCode) + : fResults(nullptr), fErrorCode(errorCode) {}; // To give LocalizedNumberFormatter format methods access to this class's constructor: friend class LocalizedNumberFormatter; diff --git a/icu4c/source/test/intltest/numbertest_api.cpp b/icu4c/source/test/intltest/numbertest_api.cpp index b936f5d51cf..63514043eeb 100644 --- a/icu4c/source/test/intltest/numbertest_api.cpp +++ b/icu4c/source/test/intltest/numbertest_api.cpp @@ -1422,12 +1422,19 @@ void NumberFormatterApiTest::errors() { -1)); { - UErrorCode status = U_ZERO_ERROR; - lnf.formatInt(1, status); + UErrorCode status1 = U_ZERO_ERROR; + UErrorCode status2 = U_ZERO_ERROR; + FormattedNumber fn = lnf.formatInt(1, status1); assertEquals( "Should fail with U_ILLEGAL_ARGUMENT_ERROR since rounder is not legal", U_ILLEGAL_ARGUMENT_ERROR, - status); + status1); + FieldPosition fp; + fn.populateFieldPosition(fp, status2); + assertEquals( + "Should fail with U_ILLEGAL_ARGUMENT_ERROR on terminal method", + U_ILLEGAL_ARGUMENT_ERROR, + status2); } {