From: Shane Carr Date: Sat, 7 Apr 2018 08:49:11 +0000 (+0000) Subject: ICU-13634 Fixing CurrencyPluralInfo support in formatting, allowing for currency... X-Git-Tag: release-62-rc~200^2~47 X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=06485f3b6bd8c5a9ff6d3da687cfd34eff57a946;p=icu ICU-13634 Fixing CurrencyPluralInfo support in formatting, allowing for currency long names to be formatted. X-SVN-Rev: 41211 --- diff --git a/icu4c/source/i18n/decimfmt.cpp b/icu4c/source/i18n/decimfmt.cpp index 569a2a8a7b8..bf975a31d99 100644 --- a/icu4c/source/i18n/decimfmt.cpp +++ b/icu4c/source/i18n/decimfmt.cpp @@ -62,6 +62,15 @@ DecimalFormat::DecimalFormat(const UnicodeString& pattern, DecimalFormatSymbols* } else { setPropertiesFromPattern(pattern, IGNORE_ROUNDING_IF_CURRENCY, status); } + // Note: in Java, CurrencyPluralInfo is set in NumberFormat.java, but in C++, it is not set there, + // so we have to set it here. + if (style == UNumberFormatStyle::UNUM_CURRENCY_PLURAL) { + LocalPointer cpi( + new CurrencyPluralInfo(fSymbols->getLocale(), status), + status); + if (U_FAILURE(status)) { return; } + fProperties->currencyPluralInfo.fPtr.adoptInstead(cpi.orphan()); + } refreshFormatter(status); } diff --git a/icu4c/source/i18n/number_decimalquantity.cpp b/icu4c/source/i18n/number_decimalquantity.cpp index 8b369547aef..9c16fe4c3e7 100644 --- a/icu4c/source/i18n/number_decimalquantity.cpp +++ b/icu4c/source/i18n/number_decimalquantity.cpp @@ -84,11 +84,29 @@ DecimalQuantity::DecimalQuantity(const DecimalQuantity &other) { *this = other; } +DecimalQuantity::DecimalQuantity(DecimalQuantity&& src) U_NOEXCEPT { + *this = std::move(src); +} + DecimalQuantity &DecimalQuantity::operator=(const DecimalQuantity &other) { if (this == &other) { return *this; } copyBcdFrom(other); + copyFieldsFrom(other); + return *this; +} + +DecimalQuantity& DecimalQuantity::operator=(DecimalQuantity&& src) U_NOEXCEPT { + if (this == &src) { + return *this; + } + moveBcdFrom(src); + copyFieldsFrom(src); + return *this; +} + +void DecimalQuantity::copyFieldsFrom(const DecimalQuantity& other) { bogus = other.bogus; lOptPos = other.lOptPos; lReqPos = other.lReqPos; @@ -100,7 +118,6 @@ DecimalQuantity &DecimalQuantity::operator=(const DecimalQuantity &other) { origDouble = other.origDouble; origDelta = other.origDelta; isApproximate = other.isApproximate; - return *this; } void DecimalQuantity::clear() { @@ -474,7 +491,6 @@ void DecimalQuantity::_setToDecNum(const DecNum& decnum, UErrorCode& status) { int64_t DecimalQuantity::toLong() const { // NOTE: Call sites should be guarded by fitsInLong(), like this: // if (dq.fitsInLong()) { /* use dq.toLong() */ } else { /* use some fallback */ } - U_ASSERT(fitsInLong()); int64_t result = 0L; for (int32_t magnitude = scale + precision - 1; magnitude >= 0; magnitude--) { result = result * 10 + getDigitPos(magnitude - scale); @@ -1032,6 +1048,20 @@ void DecimalQuantity::copyBcdFrom(const DecimalQuantity &other) { } } +void DecimalQuantity::moveBcdFrom(DecimalQuantity &other) { + setBcdToZero(); + if (other.usingBytes) { + usingBytes = true; + fBCD.bcdBytes.ptr = other.fBCD.bcdBytes.ptr; + fBCD.bcdBytes.len = other.fBCD.bcdBytes.len; + // Take ownership away from the old instance: + other.fBCD.bcdBytes.ptr = nullptr; + other.usingBytes = false; + } else { + fBCD.bcdLong = other.fBCD.bcdLong; + } +} + const char16_t* DecimalQuantity::checkHealth() const { if (usingBytes) { if (precision == 0) { return u"Zero precision but we are in byte mode"; } diff --git a/icu4c/source/i18n/number_decimalquantity.h b/icu4c/source/i18n/number_decimalquantity.h index bcd3aa7cf66..268cdd9967b 100644 --- a/icu4c/source/i18n/number_decimalquantity.h +++ b/icu4c/source/i18n/number_decimalquantity.h @@ -35,6 +35,9 @@ class U_I18N_API DecimalQuantity : public IFixedDecimal, public UMemory { /** Copy constructor. */ DecimalQuantity(const DecimalQuantity &other); + /** Move constructor. */ + DecimalQuantity(DecimalQuantity &&src) U_NOEXCEPT; + DecimalQuantity(); ~DecimalQuantity() override; @@ -46,6 +49,9 @@ class U_I18N_API DecimalQuantity : public IFixedDecimal, public UMemory { */ DecimalQuantity &operator=(const DecimalQuantity &other); + /** Move assignment */ + DecimalQuantity &operator=(DecimalQuantity&& src) U_NOEXCEPT; + /** * Sets the minimum and maximum integer digits that this {@link DecimalQuantity} should generate. * This method does not perform rounding. @@ -434,8 +440,12 @@ class U_I18N_API DecimalQuantity : public IFixedDecimal, public UMemory { void readDoubleConversionToBcd(const char* buffer, int32_t length, int32_t point); + void copyFieldsFrom(const DecimalQuantity& other); + void copyBcdFrom(const DecimalQuantity &other); + void moveBcdFrom(DecimalQuantity& src); + /** * Removes trailing zeros from the BCD (adjusting the scale as required) and then computes the * precision. The precision is the number of digits in the number up through the greatest nonzero diff --git a/icu4c/source/i18n/number_formatimpl.cpp b/icu4c/source/i18n/number_formatimpl.cpp index 7b88dc3ec88..62947be5711 100644 --- a/icu4c/source/i18n/number_formatimpl.cpp +++ b/icu4c/source/i18n/number_formatimpl.cpp @@ -233,7 +233,7 @@ NumberFormatterImpl::macrosToMicroGenerator(const MacroProps& macros, bool safe, if (info.exists) { pattern = info.pattern; // It's clunky to clone an object here, but this code is not frequently executed. - DecimalFormatSymbols* symbols = new DecimalFormatSymbols(*fMicros.symbols); + auto* symbols = new DecimalFormatSymbols(*fMicros.symbols); fMicros.symbols = symbols; fSymbols.adoptInstead(symbols); symbols->setSymbol( diff --git a/icu4c/source/i18n/number_mapper.cpp b/icu4c/source/i18n/number_mapper.cpp index 9acf60382b2..ef13267dcd8 100644 --- a/icu4c/source/i18n/number_mapper.cpp +++ b/icu4c/source/i18n/number_mapper.cpp @@ -304,6 +304,8 @@ MacroProps NumberPropertyMapper::oldToNew(const DecimalFormatProperties& propert void PropertiesAffixPatternProvider::setTo(const DecimalFormatProperties& properties, UErrorCode&) { + fBogus = false; + // There are two ways to set affixes in DecimalFormat: via the pattern string (applyPattern), and via the // explicit setters (setPositivePrefix and friends). The way to resolve the settings is as follows: // @@ -432,11 +434,16 @@ bool PropertiesAffixPatternProvider::hasBody() const { void CurrencyPluralInfoAffixProvider::setTo(const CurrencyPluralInfo& cpi, UErrorCode& status) { + fBogus = false; for (int32_t plural = 0; plural < StandardPlural::COUNT; plural++) { const char* keyword = StandardPlural::getKeyword(static_cast(plural)); UnicodeString patternString; patternString = cpi.getCurrencyPluralPattern(keyword, patternString); - PatternParser::parseToPatternInfo(patternString, affixesByPlural[plural], status); + // ParsedPatternInfo does not support being overwritten if it was written previously; + // instead, we need to write to a fresh instance and move it into place. + ParsedPatternInfo temp; + PatternParser::parseToPatternInfo(patternString, temp, status); + affixesByPlural[plural] = std::move(temp); } } diff --git a/icu4c/source/i18n/number_patternstring.cpp b/icu4c/source/i18n/number_patternstring.cpp index 8ea130fb4a5..e32ad6b130d 100644 --- a/icu4c/source/i18n/number_patternstring.cpp +++ b/icu4c/source/i18n/number_patternstring.cpp @@ -20,6 +20,7 @@ using namespace icu; using namespace icu::number; using namespace icu::number::impl; + void PatternParser::parseToPatternInfo(const UnicodeString& patternString, ParsedPatternInfo& patternInfo, UErrorCode& status) { patternInfo.consumePattern(patternString, status); @@ -44,6 +45,7 @@ PatternParser::parseToExistingProperties(const UnicodeString& pattern, DecimalFo parseToExistingPropertiesImpl(pattern, properties, ignoreRounding, status); } + char16_t ParsedPatternInfo::charAt(int32_t flags, int32_t index) const { const Endpoints& endpoints = getEndpoints(flags); if (index < 0 || index >= endpoints.end - endpoints.start) { @@ -134,6 +136,10 @@ void ParsedPatternInfo::consumePattern(const UnicodeString& patternString, UErro if (U_FAILURE(status)) { return; } this->pattern = patternString; + // This class is not intended for writing twice! + // Use move assignment to overwrite instead. + U_ASSERT(state.offset == 0); + // pattern := subpattern (';' subpattern)? currentSubpattern = &positive; consumeSubpattern(status); @@ -178,12 +184,13 @@ void ParsedPatternInfo::consumePadding(PadPosition paddingLocation, UErrorCode& if (state.peek() != u'*') { return; } - if (!currentSubpattern->paddingLocation.isNull()) { + if (currentSubpattern->hasPadding) { state.toParseException(u"Cannot have multiple pad specifiers"); status = U_MULTIPLE_PAD_SPECIFIERS; return; } currentSubpattern->paddingLocation = paddingLocation; + currentSubpattern->hasPadding = true; state.next(); // consume the '*' currentSubpattern->paddingEndpoints.start = state.offset; consumeLiteral(status); @@ -580,7 +587,7 @@ PatternParser::patternInfoToProperties(DecimalFormatProperties& properties, Pars UnicodeString posSuffix = patternInfo.getString(0); // Padding settings - if (!positive.paddingLocation.isNull()) { + if (positive.hasPadding) { // The width of the positive prefix and suffix templates are included in the padding int paddingWidth = positive.widthExceptAffixes + AffixUtils::estimateLength(UnicodeStringCharSequence(posPrefix), status) + diff --git a/icu4c/source/i18n/number_patternstring.h b/icu4c/source/i18n/number_patternstring.h index 742ee3547cd..a87cc7415c3 100644 --- a/icu4c/source/i18n/number_patternstring.h +++ b/icu4c/source/i18n/number_patternstring.h @@ -41,7 +41,9 @@ struct U_I18N_API ParsedSubpatternInfo { int32_t fractionTotal = 0; // for convenience bool hasDecimal = false; int32_t widthExceptAffixes = 0; - NullableValue paddingLocation; + // Note: NullableValue causes issues here with std::move. + bool hasPadding = false; + UNumberFormatPadPosition paddingLocation = UNUM_PAD_BEFORE_PREFIX; DecimalQuantity rounding; bool exponentHasPlusSign = false; int32_t exponentZeros = 0; @@ -67,6 +69,9 @@ struct U_I18N_API ParsedPatternInfo : public AffixPatternProvider, public UMemor ~ParsedPatternInfo() U_OVERRIDE = default; + // Need to declare this explicitly because of the destructor + ParsedPatternInfo& operator=(ParsedPatternInfo&& src) U_NOEXCEPT = default; + static int32_t getLengthFromEndpoints(const Endpoints& endpoints); char16_t charAt(int32_t flags, int32_t index) const U_OVERRIDE; @@ -95,6 +100,13 @@ struct U_I18N_API ParsedPatternInfo : public AffixPatternProvider, public UMemor explicit ParserState(const UnicodeString& _pattern) : pattern(_pattern) {}; + ParserState& operator=(ParserState&& src) U_NOEXCEPT { + // Leave pattern reference alone; it will continue to point to the same place in memory, + // which gets overwritten by ParsedPatternInfo's implicit move assignment. + offset = src.offset; + return *this; + } + UChar32 peek(); UChar32 next(); diff --git a/icu4c/source/test/intltest/numbertest.h b/icu4c/source/test/intltest/numbertest.h index 2da93bcbb81..a3678d5913a 100644 --- a/icu4c/source/test/intltest/numbertest.h +++ b/icu4c/source/test/intltest/numbertest.h @@ -116,6 +116,7 @@ class DecimalQuantityTest : public IntlTest { public: void testDecimalQuantityBehaviorStandalone(); void testSwitchStorage(); + void testCopyMove(); void testAppend(); void testConvertToAccurateDouble(); void testUseApproximateDoubleWhenAble(); diff --git a/icu4c/source/test/intltest/numbertest_decimalquantity.cpp b/icu4c/source/test/intltest/numbertest_decimalquantity.cpp index ba1a67c2159..320dbe0eaba 100644 --- a/icu4c/source/test/intltest/numbertest_decimalquantity.cpp +++ b/icu4c/source/test/intltest/numbertest_decimalquantity.cpp @@ -16,7 +16,8 @@ void DecimalQuantityTest::runIndexedTest(int32_t index, UBool exec, const char * } TESTCASE_AUTO_BEGIN; TESTCASE_AUTO(testDecimalQuantityBehaviorStandalone); - TESTCASE_AUTO(testSwitchStorage); + TESTCASE_AUTO(testSwitchStorage);; + TESTCASE_AUTO(testCopyMove); TESTCASE_AUTO(testAppend); TESTCASE_AUTO(testConvertToAccurateDouble); TESTCASE_AUTO(testUseApproximateDoubleWhenAble); @@ -119,6 +120,53 @@ void DecimalQuantityTest::testSwitchStorage() { assertHealth(fq); } +void DecimalQuantityTest::testCopyMove() { + // Small numbers (fits in BCD long) + { + DecimalQuantity a; + a.setToLong(1234123412341234L); + DecimalQuantity b = a; // copy constructor + assertToStringAndHealth(a, u""); + assertToStringAndHealth(b, u""); + DecimalQuantity c(std::move(a)); // move constructor + assertToStringAndHealth(c, u""); + c.setToLong(54321L); + assertToStringAndHealth(c, u""); + c = b; // copy assignment + assertToStringAndHealth(b, u""); + assertToStringAndHealth(c, u""); + b.setToLong(45678); + c.setToLong(56789); + c = std::move(b); // move assignment + assertToStringAndHealth(c, u""); + a = std::move(c); // move assignment to a defunct object + assertToStringAndHealth(a, u""); + } + + // Large numbers (requires byte allocation) + { + IcuTestErrorCode status(*this, "testCopyMove"); + DecimalQuantity a; + a.setToDecNumber({"1234567890123456789", -1}, status); + DecimalQuantity b = a; // copy constructor + assertToStringAndHealth(a, u""); + assertToStringAndHealth(b, u""); + DecimalQuantity c(std::move(a)); // move constructor + assertToStringAndHealth(c, u""); + c.setToDecNumber({"9876543210987654321", -1}, status); + assertToStringAndHealth(c, u""); + c = b; // copy assignment + assertToStringAndHealth(b, u""); + assertToStringAndHealth(c, u""); + b.setToDecNumber({"876543210987654321", -1}, status); + c.setToDecNumber({"987654321098765432", -1}, status); + c = std::move(b); // move assignment + assertToStringAndHealth(c, u""); + a = std::move(c); // move assignment to a defunct object + assertToStringAndHealth(a, u""); + } +} + void DecimalQuantityTest::testAppend() { DecimalQuantity fq; fq.appendDigit(1, 0, true); diff --git a/icu4c/source/test/intltest/numfmtst.cpp b/icu4c/source/test/intltest/numfmtst.cpp index 8e75246e2e0..54817637b4e 100644 --- a/icu4c/source/test/intltest/numfmtst.cpp +++ b/icu4c/source/test/intltest/numfmtst.cpp @@ -3804,13 +3804,13 @@ NumberFormatTest::TestMultiCurrencySign() { // currency format using currency ISO name, such as "USD", // currency format using plural name, such as "US dollars". // for US locale - {"en_US", "\\u00A4#,##0.00;-\\u00A4#,##0.00", "1234.56", "$1,234.56", "USD1,234.56", "US dollars1,234.56"}, - {"en_US", "\\u00A4#,##0.00;-\\u00A4#,##0.00", "-1234.56", "-$1,234.56", "-USD1,234.56", "-US dollars1,234.56"}, - {"en_US", "\\u00A4#,##0.00;-\\u00A4#,##0.00", "1", "$1.00", "USD1.00", "US dollars1.00"}, + {"en_US", "\\u00A4#,##0.00;-\\u00A4#,##0.00", "1234.56", "$1,234.56", "USD\\u00A01,234.56", "US dollars\\u00A01,234.56"}, + {"en_US", "\\u00A4#,##0.00;-\\u00A4#,##0.00", "-1234.56", "-$1,234.56", "-USD\\u00A01,234.56", "-US dollars\\u00A01,234.56"}, + {"en_US", "\\u00A4#,##0.00;-\\u00A4#,##0.00", "1", "$1.00", "USD\\u00A01.00", "US dollars\\u00A01.00"}, // for CHINA locale - {"zh_CN", "\\u00A4#,##0.00;(\\u00A4#,##0.00)", "1234.56", "\\uFFE51,234.56", "CNY1,234.56", "\\u4EBA\\u6C11\\u5E011,234.56"}, - {"zh_CN", "\\u00A4#,##0.00;(\\u00A4#,##0.00)", "-1234.56", "(\\uFFE51,234.56)", "(CNY1,234.56)", "(\\u4EBA\\u6C11\\u5E011,234.56)"}, - {"zh_CN", "\\u00A4#,##0.00;(\\u00A4#,##0.00)", "1", "\\uFFE51.00", "CNY1.00", "\\u4EBA\\u6C11\\u5E011.00"} + {"zh_CN", "\\u00A4#,##0.00;(\\u00A4#,##0.00)", "1234.56", "\\uFFE51,234.56", "CNY\\u00A01,234.56", "\\u4EBA\\u6C11\\u5E01\\u00A01,234.56"}, + {"zh_CN", "\\u00A4#,##0.00;(\\u00A4#,##0.00)", "-1234.56", "(\\uFFE51,234.56)", "(CNY\\u00A01,234.56)", "(\\u4EBA\\u6C11\\u5E01\\u00A01,234.56)"}, + {"zh_CN", "\\u00A4#,##0.00;(\\u00A4#,##0.00)", "1", "\\uFFE51.00", "CNY\\u00A01.00", "\\u4EBA\\u6C11\\u5E01\\u00A01.00"} }; const UChar doubleCurrencySign[] = {0xA4, 0xA4, 0}; @@ -3896,7 +3896,7 @@ NumberFormatTest::TestCurrencyFormatForMixParsing() { "$1,234.56", // string to be parsed "USD1,234.56", "US dollars1,234.56", - "1,234.56 US dollars" + "1,234.56 US dollars" // NOTE: Fails in 62 because currency format is not compatible with pattern }; const CurrencyAmount* curramt = NULL; for (uint32_t i = 0; i < UPRV_LENGTHOF(formats); ++i) { @@ -3949,10 +3949,10 @@ NumberFormatTest::TestDecimalFormatCurrencyParse() { // string to be parsed, the parsed result (number) {"$1.00", "1"}, {"USD1.00", "1"}, - {"1.00 US dollar", "1"}, + {"1.00 US dollar", "1"}, // NOTE: Fails in 62 because currency format is not compatible with pattern {"$1,234.56", "1234.56"}, {"USD1,234.56", "1234.56"}, - {"1,234.56 US dollar", "1234.56"}, + {"1,234.56 US dollar", "1234.56"}, // NOTE: Fails in 62 because currency format is not compatible with pattern }; for (uint32_t i = 0; i < UPRV_LENGTHOF(DATA); ++i) { UnicodeString stringToBeParsed = ctou(DATA[i][0]); @@ -3960,6 +3960,7 @@ NumberFormatTest::TestDecimalFormatCurrencyParse() { UErrorCode status = U_ZERO_ERROR; Formattable result; fmt->parse(stringToBeParsed, result, status); + logln((UnicodeString)"Input: " + stringToBeParsed + "; output: " + result.getDouble(status)); if (U_FAILURE(status) || (result.getType() == Formattable::kDouble && result.getDouble() != parsedResult) || @@ -3983,13 +3984,13 @@ NumberFormatTest::TestCurrencyIsoPluralFormat() { // format result using ISOCURRENCYSTYLE, // format result using PLURALCURRENCYSTYLE, - {"en_US", "1", "USD", "$1.00", "USD1.00", "1.00 US dollars"}, - {"en_US", "1234.56", "USD", "$1,234.56", "USD1,234.56", "1,234.56 US dollars"}, - {"en_US", "-1234.56", "USD", "-$1,234.56", "-USD1,234.56", "-1,234.56 US dollars"}, - {"zh_CN", "1", "USD", "US$1.00", "USD1.00", "1.00\\u7F8E\\u5143"}, - {"zh_CN", "1234.56", "USD", "US$1,234.56", "USD1,234.56", "1,234.56\\u7F8E\\u5143"}, - {"zh_CN", "1", "CNY", "\\uFFE51.00", "CNY1.00", "1.00\\u4EBA\\u6C11\\u5E01"}, - {"zh_CN", "1234.56", "CNY", "\\uFFE51,234.56", "CNY1,234.56", "1,234.56\\u4EBA\\u6C11\\u5E01"}, + {"en_US", "1", "USD", "$1.00", "USD\\u00A01.00", "1.00 US dollars"}, + {"en_US", "1234.56", "USD", "$1,234.56", "USD\\u00A01,234.56", "1,234.56 US dollars"}, + {"en_US", "-1234.56", "USD", "-$1,234.56", "-USD\\u00A01,234.56", "-1,234.56 US dollars"}, + {"zh_CN", "1", "USD", "US$1.00", "USD\\u00A01.00", "1.00\\u7F8E\\u5143"}, + {"zh_CN", "1234.56", "USD", "US$1,234.56", "USD\\u00A01,234.56", "1,234.56\\u7F8E\\u5143"}, + {"zh_CN", "1", "CNY", "\\uFFE51.00", "CNY\\u00A01.00", "1.00\\u4EBA\\u6C11\\u5E01"}, + {"zh_CN", "1234.56", "CNY", "\\uFFE51,234.56", "CNY\\u00A01,234.56", "1,234.56\\u4EBA\\u6C11\\u5E01"}, {"ru_RU", "1", "RUB", "1,00\\u00A0\\u20BD", "1,00\\u00A0RUB", "1,00 \\u0440\\u043E\\u0441\\u0441\\u0438\\u0439\\u0441\\u043A\\u043E\\u0433\\u043E \\u0440\\u0443\\u0431\\u043B\\u044F"}, {"ru_RU", "2", "RUB", "2,00\\u00A0\\u20BD", "2,00\\u00A0RUB", "2,00 \\u0440\\u043E\\u0441\\u0441\\u0438\\u0439\\u0441\\u043A\\u043E\\u0433\\u043E \\u0440\\u0443\\u0431\\u043B\\u044F"}, {"ru_RU", "5", "RUB", "5,00\\u00A0\\u20BD", "5,00\\u00A0RUB", "5,00 \\u0440\\u043E\\u0441\\u0441\\u0438\\u0439\\u0441\\u043A\\u043E\\u0433\\u043E \\u0440\\u0443\\u0431\\u043B\\u044F"}, @@ -4005,12 +4006,14 @@ NumberFormatTest::TestCurrencyIsoPluralFormat() { }; for (int32_t i=0; i