ICU-20499 Fixing code path for plural form in MutablePatternModifier.
diff --git a/icu4c/source/i18n/number_compact.cpp b/icu4c/source/i18n/number_compact.cpp
index cbcfb67..f330251 100644
--- a/icu4c/source/i18n/number_compact.cpp
+++ b/icu4c/source/i18n/number_compact.cpp
@@ -297,7 +297,7 @@
for (; i < precomputedModsLength; i++) {
const CompactModInfo &info = precomputedMods[i];
if (u_strcmp(patternString, info.patternString) == 0) {
- info.mod->applyToMicros(micros, quantity);
+ info.mod->applyToMicros(micros, quantity, status);
break;
}
}
diff --git a/icu4c/source/i18n/number_longnames.cpp b/icu4c/source/i18n/number_longnames.cpp
index cfea339..0cd1600 100644
--- a/icu4c/source/i18n/number_longnames.cpp
+++ b/icu4c/source/i18n/number_longnames.cpp
@@ -313,10 +313,8 @@
void LongNameHandler::processQuantity(DecimalQuantity &quantity, MicroProps µs,
UErrorCode &status) const {
parent->processQuantity(quantity, micros, status);
- // TODO: Avoid the copy here?
- DecimalQuantity copy(quantity);
- micros.rounder.apply(copy, status);
- micros.modOuter = &fModifiers[utils::getStandardPlural(rules, copy)];
+ StandardPlural::Form pluralForm = utils::getPluralSafe(micros.rounder, rules, quantity, status);
+ micros.modOuter = &fModifiers[pluralForm];
}
const Modifier* LongNameHandler::getModifier(int8_t /*signum*/, StandardPlural::Form plural) const {
diff --git a/icu4c/source/i18n/number_patternmodifier.cpp b/icu4c/source/i18n/number_patternmodifier.cpp
index 49ef814..75de439 100644
--- a/icu4c/source/i18n/number_patternmodifier.cpp
+++ b/icu4c/source/i18n/number_patternmodifier.cpp
@@ -127,18 +127,16 @@
void ImmutablePatternModifier::processQuantity(DecimalQuantity& quantity, MicroProps& micros,
UErrorCode& status) const {
parent->processQuantity(quantity, micros, status);
- applyToMicros(micros, quantity);
+ applyToMicros(micros, quantity, status);
}
-void ImmutablePatternModifier::applyToMicros(MicroProps& micros, DecimalQuantity& quantity) const {
+void ImmutablePatternModifier::applyToMicros(
+ MicroProps& micros, const DecimalQuantity& quantity, UErrorCode& status) const {
if (rules == nullptr) {
micros.modMiddle = pm->getModifierWithoutPlural(quantity.signum());
} else {
- // TODO: Fix this. Avoid the copy.
- DecimalQuantity copy(quantity);
- copy.roundToInfinity();
- StandardPlural::Form plural = utils::getStandardPlural(rules, copy);
- micros.modMiddle = pm->getModifier(quantity.signum(), plural);
+ StandardPlural::Form pluralForm = utils::getPluralSafe(micros.rounder, rules, quantity, status);
+ micros.modMiddle = pm->getModifier(quantity.signum(), pluralForm);
}
}
@@ -164,10 +162,8 @@
// This method needs to be const because it overrides a const method in the parent class.
auto nonConstThis = const_cast<MutablePatternModifier*>(this);
if (needsPlurals()) {
- // TODO: Fix this. Avoid the copy.
- DecimalQuantity copy(fq);
- micros.rounder.apply(copy, status);
- nonConstThis->setNumberProperties(fq.signum(), utils::getStandardPlural(fRules, copy));
+ StandardPlural::Form pluralForm = utils::getPluralSafe(micros.rounder, fRules, fq, status);
+ nonConstThis->setNumberProperties(fq.signum(), pluralForm);
} else {
nonConstThis->setNumberProperties(fq.signum(), StandardPlural::Form::COUNT);
}
diff --git a/icu4c/source/i18n/number_patternmodifier.h b/icu4c/source/i18n/number_patternmodifier.h
index 675ea70..27e293b 100644
--- a/icu4c/source/i18n/number_patternmodifier.h
+++ b/icu4c/source/i18n/number_patternmodifier.h
@@ -46,7 +46,7 @@
void processQuantity(DecimalQuantity&, MicroProps& micros, UErrorCode& status) const U_OVERRIDE;
- void applyToMicros(MicroProps& micros, DecimalQuantity& quantity) const;
+ void applyToMicros(MicroProps& micros, const DecimalQuantity& quantity, UErrorCode& status) const;
const Modifier* getModifier(int8_t signum, StandardPlural::Form plural) const;
diff --git a/icu4c/source/i18n/number_utils.h b/icu4c/source/i18n/number_utils.h
index 05b8ec0..203dec6 100644
--- a/icu4c/source/i18n/number_utils.h
+++ b/icu4c/source/i18n/number_utils.h
@@ -124,6 +124,23 @@
}
}
+/**
+ * Computes the plural form after copying the number and applying rounding rules.
+ */
+inline StandardPlural::Form getPluralSafe(
+ const RoundingImpl& rounder,
+ const PluralRules* rules,
+ const DecimalQuantity& dq,
+ UErrorCode& status) {
+ // TODO(ICU-20500): Avoid the copy?
+ DecimalQuantity copy(dq);
+ rounder.apply(copy, status);
+ if (U_FAILURE(status)) {
+ return StandardPlural::Form::OTHER;
+ }
+ return getStandardPlural(rules, copy);
+}
+
} // namespace utils
} // namespace impl
diff --git a/icu4c/source/test/intltest/numbertest.h b/icu4c/source/test/intltest/numbertest.h
index c07fe9e..e1c90a7 100644
--- a/icu4c/source/test/intltest/numbertest.h
+++ b/icu4c/source/test/intltest/numbertest.h
@@ -90,6 +90,7 @@
CurrencyUnit CAD;
CurrencyUnit ESP;
CurrencyUnit PTE;
+ CurrencyUnit RON;
MeasureUnit METER;
MeasureUnit DAY;
diff --git a/icu4c/source/test/intltest/numbertest_api.cpp b/icu4c/source/test/intltest/numbertest_api.cpp
index f27292f..902d27f 100644
--- a/icu4c/source/test/intltest/numbertest_api.cpp
+++ b/icu4c/source/test/intltest/numbertest_api.cpp
@@ -32,6 +32,7 @@
CAD(u"CAD", status),
ESP(u"ESP", status),
PTE(u"PTE", status),
+ RON(u"RON", status),
FRENCH_SYMBOLS(Locale::getFrench(), status),
SWISS_SYMBOLS(Locale("de-CH"), status),
MYANMAR_SYMBOLS(Locale("my"), status) {
@@ -788,6 +789,14 @@
Locale("pt-PT"),
444444.55,
u"444,444$55 PTE");
+
+ assertFormatSingle(
+ u"Plural form depending on visible digits (ICU-20499)",
+ u"currency/RON unit-width-full-name",
+ NumberFormatter::with().unit(RON).unitWidth(UNUM_UNIT_WIDTH_FULL_NAME),
+ Locale("ro-RO"),
+ 24,
+ u"24,00 lei românești");
}
void NumberFormatterApiTest::unitPercent() {
diff --git a/icu4c/source/test/intltest/numbertest_patternmodifier.cpp b/icu4c/source/test/intltest/numbertest_patternmodifier.cpp
index 2156c9b..ba382a6 100644
--- a/icu4c/source/test/intltest/numbertest_patternmodifier.cpp
+++ b/icu4c/source/test/intltest/numbertest_patternmodifier.cpp
@@ -119,7 +119,7 @@
return;
}
DecimalQuantity quantity;
- imod->applyToMicros(micros, quantity);
+ imod->applyToMicros(micros, quantity, status);
micros.modMiddle->apply(nsb, 1, 4, status);
assertSuccess("Spot 7", status);
assertEquals("Safe Path", u"xabcy", nsb.toUnicodeString());
@@ -151,7 +151,7 @@
NumberStringBuilder nsb2;
MicroProps micros2;
LocalPointer<ImmutablePatternModifier> immutable(mod.createImmutable(status));
- immutable->applyToMicros(micros2, fq);
+ immutable->applyToMicros(micros2, fq, status);
micros2.modMiddle->apply(nsb2, 0, 0, status);
assertSuccess("Spot 4", status);
diff --git a/icu4c/source/test/intltest/numfmtst.cpp b/icu4c/source/test/intltest/numfmtst.cpp
index 4625b13..7dd66e5 100644
--- a/icu4c/source/test/intltest/numfmtst.cpp
+++ b/icu4c/source/test/intltest/numfmtst.cpp
@@ -229,6 +229,7 @@
TESTCASE_AUTO(Test20348_CurrencyPrefixOverride);
TESTCASE_AUTO(Test20358_GroupingInPattern);
TESTCASE_AUTO(Test13731_DefaultCurrency);
+ TESTCASE_AUTO(Test20499_CurrencyVisibleDigitsPlural);
TESTCASE_AUTO_END;
}
@@ -9608,4 +9609,17 @@
}
}
+void NumberFormatTest::Test20499_CurrencyVisibleDigitsPlural() {
+ IcuTestErrorCode status(*this, "Test20499_CurrencyVisibleDigitsPlural");
+ LocalPointer<NumberFormat> nf(NumberFormat::createInstance(
+ "ro-RO", UNumberFormatStyle::UNUM_CURRENCY_PLURAL, status), status);
+ const char16_t* expected = u"24,00 lei românești";
+ for (int32_t i=0; i<5; i++) {
+ UnicodeString actual;
+ nf->format(24, actual, status);
+ assertEquals(UnicodeString(u"iteration ") + Int64ToUnicodeString(i),
+ expected, actual);
+ }
+}
+
#endif /* #if !UCONFIG_NO_FORMATTING */
diff --git a/icu4c/source/test/intltest/numfmtst.h b/icu4c/source/test/intltest/numfmtst.h
index b62cde5..7dbd31c 100644
--- a/icu4c/source/test/intltest/numfmtst.h
+++ b/icu4c/source/test/intltest/numfmtst.h
@@ -295,6 +295,7 @@
void Test20348_CurrencyPrefixOverride();
void Test20358_GroupingInPattern();
void Test13731_DefaultCurrency();
+ void Test20499_CurrencyVisibleDigitsPlural();
private:
UBool testFormattableAsUFormattable(const char *file, int line, Formattable &f);
diff --git a/icu4j/main/classes/core/src/com/ibm/icu/impl/number/LongNameHandler.java b/icu4j/main/classes/core/src/com/ibm/icu/impl/number/LongNameHandler.java
index e31ea8b..c0c6a74 100644
--- a/icu4j/main/classes/core/src/com/ibm/icu/impl/number/LongNameHandler.java
+++ b/icu4j/main/classes/core/src/com/ibm/icu/impl/number/LongNameHandler.java
@@ -290,10 +290,8 @@
@Override
public MicroProps processQuantity(DecimalQuantity quantity) {
MicroProps micros = parent.processQuantity(quantity);
- // TODO: Avoid the copy here?
- DecimalQuantity copy = quantity.createCopy();
- micros.rounder.apply(copy);
- micros.modOuter = modifiers.get(copy.getStandardPlural(rules));
+ StandardPlural pluralForm = RoundingUtils.getPluralSafe(micros.rounder, rules, quantity);
+ micros.modOuter = modifiers.get(pluralForm);
return micros;
}
diff --git a/icu4j/main/classes/core/src/com/ibm/icu/impl/number/MutablePatternModifier.java b/icu4j/main/classes/core/src/com/ibm/icu/impl/number/MutablePatternModifier.java
index 4cdde30..b1c1e9a 100644
--- a/icu4j/main/classes/core/src/com/ibm/icu/impl/number/MutablePatternModifier.java
+++ b/icu4j/main/classes/core/src/com/ibm/icu/impl/number/MutablePatternModifier.java
@@ -243,11 +243,8 @@
if (rules == null) {
micros.modMiddle = pm.getModifierWithoutPlural(quantity.signum());
} else {
- // TODO: Fix this. Avoid the copy.
- DecimalQuantity copy = quantity.createCopy();
- copy.roundToInfinity();
- StandardPlural plural = copy.getStandardPlural(rules);
- micros.modMiddle = pm.getModifier(quantity.signum(), plural);
+ StandardPlural pluralForm = RoundingUtils.getPluralSafe(micros.rounder, rules, quantity);
+ micros.modMiddle = pm.getModifier(quantity.signum(), pluralForm);
}
}
@@ -273,10 +270,8 @@
public MicroProps processQuantity(DecimalQuantity fq) {
MicroProps micros = parent.processQuantity(fq);
if (needsPlurals()) {
- // TODO: Fix this. Avoid the copy.
- DecimalQuantity copy = fq.createCopy();
- micros.rounder.apply(copy);
- setNumberProperties(fq.signum(), copy.getStandardPlural(rules));
+ StandardPlural pluralForm = RoundingUtils.getPluralSafe(micros.rounder, rules, fq);
+ setNumberProperties(fq.signum(), pluralForm);
} else {
setNumberProperties(fq.signum(), null);
}
diff --git a/icu4j/main/classes/core/src/com/ibm/icu/impl/number/RoundingUtils.java b/icu4j/main/classes/core/src/com/ibm/icu/impl/number/RoundingUtils.java
index 5921f60..0f35a66 100644
--- a/icu4j/main/classes/core/src/com/ibm/icu/impl/number/RoundingUtils.java
+++ b/icu4j/main/classes/core/src/com/ibm/icu/impl/number/RoundingUtils.java
@@ -6,7 +6,10 @@
import java.math.MathContext;
import java.math.RoundingMode;
+import com.ibm.icu.impl.StandardPlural;
+import com.ibm.icu.number.Precision;
import com.ibm.icu.number.Scale;
+import com.ibm.icu.text.PluralRules;
/** @author sffc */
public class RoundingUtils {
@@ -219,4 +222,15 @@
return null;
}
}
+
+ /**
+ * Computes the plural form after copying the number and applying rounding rules.
+ */
+ public static StandardPlural getPluralSafe(
+ Precision rounder, PluralRules rules, DecimalQuantity dq) {
+ // TODO(ICU-20500): Avoid the copy?
+ DecimalQuantity copy = dq.createCopy();
+ rounder.apply(copy);
+ return copy.getStandardPlural(rules);
+ }
}
diff --git a/icu4j/main/classes/core/src/com/ibm/icu/number/CompactNotation.java b/icu4j/main/classes/core/src/com/ibm/icu/number/CompactNotation.java
index 04e0eb9..9502dc3 100644
--- a/icu4j/main/classes/core/src/com/ibm/icu/number/CompactNotation.java
+++ b/icu4j/main/classes/core/src/com/ibm/icu/number/CompactNotation.java
@@ -128,7 +128,6 @@
magnitude = 0;
micros.rounder.apply(quantity);
} else {
- // TODO: Revisit chooseMultiplierAndApply
int multiplier = micros.rounder.chooseMultiplierAndApply(quantity, data);
magnitude = quantity.isZero() ? 0 : quantity.getMagnitude();
magnitude -= multiplier;
diff --git a/icu4j/main/tests/core/src/com/ibm/icu/dev/test/format/NumberFormatTest.java b/icu4j/main/tests/core/src/com/ibm/icu/dev/test/format/NumberFormatTest.java
index cc70295..0c76bd0 100644
--- a/icu4j/main/tests/core/src/com/ibm/icu/dev/test/format/NumberFormatTest.java
+++ b/icu4j/main/tests/core/src/com/ibm/icu/dev/test/format/NumberFormatTest.java
@@ -4152,7 +4152,7 @@
int fracForRoundIncr = 0;
if (roundIncrUsed) {
double testIncr = item.roundIncr;
- for (; testIncr > (double)((int)testIncr); testIncr *= 10.0, fracForRoundIncr++);
+ for (; testIncr > ((int)testIncr); testIncr *= 10.0, fracForRoundIncr++);
}
int minInt = df.getMinimumIntegerDigits();
@@ -6614,4 +6614,15 @@
assertEquals("currency", "XXX", nf.getCurrency().getCurrencyCode());
}
}
+
+ @Test
+ public void test20499_CurrencyVisibleDigitsPlural() {
+ ULocale locale = new ULocale("ro-RO");
+ NumberFormat nf = NumberFormat.getInstance(locale, NumberFormat.PLURALCURRENCYSTYLE);
+ String expected = "24,00 lei românești";
+ for (int i=0; i<5; i++) {
+ String actual = nf.format(24);
+ assertEquals("iteration " + i, expected, actual);
+ }
+ }
}
diff --git a/icu4j/main/tests/core/src/com/ibm/icu/dev/test/number/NumberFormatterApiTest.java b/icu4j/main/tests/core/src/com/ibm/icu/dev/test/number/NumberFormatterApiTest.java
index 3d655af..9421097 100644
--- a/icu4j/main/tests/core/src/com/ibm/icu/dev/test/number/NumberFormatterApiTest.java
+++ b/icu4j/main/tests/core/src/com/ibm/icu/dev/test/number/NumberFormatterApiTest.java
@@ -65,6 +65,7 @@
private static final Currency CAD = Currency.getInstance("CAD");
private static final Currency ESP = Currency.getInstance("ESP");
private static final Currency PTE = Currency.getInstance("PTE");
+ private static final Currency RON = Currency.getInstance("RON");
@Test
public void notationSimple() {
@@ -728,7 +729,7 @@
// NOTE: This is a bit of a hack on CLDR's part. They set the currency symbol to U+200B (zero-
// width space), and they set the decimal separator to the $ symbol.
assertFormatSingle(
- "Currency-dependent symbols (Test)",
+ "Currency-dependent symbols (Test Short)",
"currency/PTE unit-width-short",
NumberFormatter.with().unit(PTE).unitWidth(UnitWidth.SHORT),
ULocale.forLanguageTag("pt-PT"),
@@ -736,7 +737,7 @@
"444,444$55 \u200B");
assertFormatSingle(
- "Currency-dependent symbols (Test)",
+ "Currency-dependent symbols (Test Narrow)",
"currency/PTE unit-width-narrow",
NumberFormatter.with().unit(PTE).unitWidth(UnitWidth.NARROW),
ULocale.forLanguageTag("pt-PT"),
@@ -744,12 +745,20 @@
"444,444$55 \u200B");
assertFormatSingle(
- "Currency-dependent symbols (Test)",
+ "Currency-dependent symbols (Test ISO Code)",
"currency/PTE unit-width-iso-code",
NumberFormatter.with().unit(PTE).unitWidth(UnitWidth.ISO_CODE),
ULocale.forLanguageTag("pt-PT"),
444444.55,
"444,444$55 PTE");
+
+ assertFormatSingle(
+ "Plural form depending on visible digits (ICU-20499)",
+ "currency/RON unit-width-full-name",
+ NumberFormatter.with().unit(RON).unitWidth(UnitWidth.FULL_NAME),
+ ULocale.forLanguageTag("ro-RO"),
+ 24,
+ "24,00 lei românești");
}
@Test
