ICU-20709 Refactoring number formatter to apply pattern after compact notation.

This commit is contained in:
Shane Carr 2019-09-27 16:33:32 -07:00 committed by Shane F. Carr
parent 369e67221c
commit e7b540d1af
16 changed files with 118 additions and 107 deletions

View File

@ -215,19 +215,25 @@ void CompactData::CompactDataSink::put(const char *key, ResourceValue &value, UB
/// END OF CompactData.java; BEGIN CompactNotation.java ///
///////////////////////////////////////////////////////////
CompactHandler::CompactHandler(CompactStyle compactStyle, const Locale &locale, const char *nsName,
CompactType compactType, const PluralRules *rules,
MutablePatternModifier *buildReference, const MicroPropsGenerator *parent,
UErrorCode &status)
: rules(rules), parent(parent) {
CompactHandler::CompactHandler(
CompactStyle compactStyle,
const Locale &locale,
const char *nsName,
CompactType compactType,
const PluralRules *rules,
MutablePatternModifier *buildReference,
bool safe,
const MicroPropsGenerator *parent,
UErrorCode &status)
: rules(rules), parent(parent), safe(safe) {
data.populate(locale, nsName, compactStyle, compactType, status);
if (buildReference != nullptr) {
if (safe) {
// Safe code path
precomputeAllModifiers(*buildReference, status);
safe = TRUE;
} else {
// Unsafe code path
safe = FALSE;
// Store the MutablePatternModifier reference.
unsafePatternModifier = buildReference;
}
}
@ -309,8 +315,9 @@ void CompactHandler::processQuantity(DecimalQuantity &quantity, MicroProps &micr
// C++ Note: Use unsafePatternInfo for proper lifecycle.
ParsedPatternInfo &patternInfo = const_cast<CompactHandler *>(this)->unsafePatternInfo;
PatternParser::parseToPatternInfo(UnicodeString(patternString), patternInfo, status);
static_cast<MutablePatternModifier*>(const_cast<Modifier*>(micros.modMiddle))
->setPatternInfo(&patternInfo, UNUM_COMPACT_FIELD);
unsafePatternModifier->setPatternInfo(&unsafePatternInfo, UNUM_COMPACT_FIELD);
unsafePatternModifier->setNumberProperties(quantity.signum(), StandardPlural::Form::COUNT);
micros.modMiddle = unsafePatternModifier;
}
// We already performed rounding. Do not perform it again.

View File

@ -56,10 +56,16 @@ struct CompactModInfo {
class CompactHandler : public MicroPropsGenerator, public UMemory {
public:
CompactHandler(CompactStyle compactStyle, const Locale &locale, const char *nsName,
CompactType compactType, const PluralRules *rules,
MutablePatternModifier *buildReference, const MicroPropsGenerator *parent,
UErrorCode &status);
CompactHandler(
CompactStyle compactStyle,
const Locale &locale,
const char *nsName,
CompactType compactType,
const PluralRules *rules,
MutablePatternModifier *buildReference,
bool safe,
const MicroPropsGenerator *parent,
UErrorCode &status);
~CompactHandler() U_OVERRIDE;
@ -74,6 +80,7 @@ class CompactHandler : public MicroPropsGenerator, public UMemory {
int32_t precomputedModsLength = 0;
CompactData data;
ParsedPatternInfo unsafePatternInfo;
MutablePatternModifier* unsafePatternModifier;
UBool safe;
/** Used by the safe code path */

View File

@ -384,11 +384,7 @@ NumberFormatterImpl::macrosToMicroGenerator(const MacroProps& macros, bool safe,
patternModifier->setSymbols(fMicros.symbols, currencySymbols, unitWidth, nullptr);
}
if (safe) {
fImmutablePatternModifier.adoptInstead(patternModifier->createImmutableAndChain(chain, status));
chain = fImmutablePatternModifier.getAlias();
} else {
patternModifier->addToChain(chain);
chain = patternModifier;
fImmutablePatternModifier.adoptInstead(patternModifier->createImmutable(status));
}
// Outer modifier (CLDR units and currency long names)
@ -418,8 +414,6 @@ NumberFormatterImpl::macrosToMicroGenerator(const MacroProps& macros, bool safe,
}
// Compact notation
// NOTE: Compact notation can (but might not) override the middle modifier and rounding.
// It therefore needs to go at the end of the chain.
if (macros.notation.fType == Notation::NTN_COMPACT) {
CompactType compactType = (isCurrency && unitWidth != UNUM_UNIT_WIDTH_FULL_NAME)
? CompactType::TYPE_CURRENCY : CompactType::TYPE_DECIMAL;
@ -429,7 +423,8 @@ NumberFormatterImpl::macrosToMicroGenerator(const MacroProps& macros, bool safe,
nsName,
compactType,
resolvePluralRules(macros.rules, macros.locale, status),
safe ? patternModifier : nullptr,
patternModifier,
safe,
chain,
status);
if (newCompactHandler == nullptr) {
@ -440,6 +435,15 @@ NumberFormatterImpl::macrosToMicroGenerator(const MacroProps& macros, bool safe,
chain = fCompactHandler.getAlias();
}
// Always add the pattern modifier as the last element of the chain.
if (safe) {
fImmutablePatternModifier->addToChain(chain);
chain = fImmutablePatternModifier.getAlias();
} else {
patternModifier->addToChain(chain);
chain = patternModifier;
}
return chain;
}

View File

@ -95,7 +95,7 @@ class NumberFormatterImpl : public UMemory {
LocalPointer<const ParsedPatternInfo> fPatternInfo;
LocalPointer<const ScientificHandler> fScientificHandler;
LocalPointer<MutablePatternModifier> fPatternModifier;
LocalPointer<const ImmutablePatternModifier> fImmutablePatternModifier;
LocalPointer<ImmutablePatternModifier> fImmutablePatternModifier;
LocalPointer<const LongNameHandler> fLongNameHandler;
LocalPointer<const CompactHandler> fCompactHandler;

View File

@ -37,7 +37,7 @@ struct MicroProps : public MicroPropsGenerator {
// Note: This struct has no direct ownership of the following pointers.
const DecimalFormatSymbols* symbols;
const Modifier* modOuter;
const Modifier* modMiddle;
const Modifier* modMiddle = nullptr;
const Modifier* modInner;
// The following "helper" fields may optionally be used during the MicroPropsGenerator.

View File

@ -55,12 +55,6 @@ bool MutablePatternModifier::needsPlurals() const {
}
ImmutablePatternModifier* MutablePatternModifier::createImmutable(UErrorCode& status) {
return createImmutableAndChain(nullptr, status);
}
ImmutablePatternModifier*
MutablePatternModifier::createImmutableAndChain(const MicroPropsGenerator* parent, UErrorCode& status) {
// TODO: Move StandardPlural VALUES to standardplural.h
static const StandardPlural::Form STANDARD_PLURAL_VALUES[] = {
StandardPlural::Form::ZERO,
@ -92,7 +86,7 @@ MutablePatternModifier::createImmutableAndChain(const MicroPropsGenerator* paren
delete pm;
return nullptr;
}
return new ImmutablePatternModifier(pm, fRules, parent); // adopts pm
return new ImmutablePatternModifier(pm, fRules); // adopts pm
} else {
// Faster path when plural keyword is not needed.
setNumberProperties(SIGNUM_POS, StandardPlural::Form::COUNT);
@ -107,7 +101,7 @@ MutablePatternModifier::createImmutableAndChain(const MicroPropsGenerator* paren
delete pm;
return nullptr;
}
return new ImmutablePatternModifier(pm, nullptr, parent); // adopts pm
return new ImmutablePatternModifier(pm, nullptr); // adopts pm
}
}
@ -124,13 +118,15 @@ ConstantMultiFieldModifier* MutablePatternModifier::createConstantModifier(UErro
}
}
ImmutablePatternModifier::ImmutablePatternModifier(AdoptingModifierStore* pm, const PluralRules* rules,
const MicroPropsGenerator* parent)
: pm(pm), rules(rules), parent(parent) {}
ImmutablePatternModifier::ImmutablePatternModifier(AdoptingModifierStore* pm, const PluralRules* rules)
: pm(pm), rules(rules), parent(nullptr) {}
void ImmutablePatternModifier::processQuantity(DecimalQuantity& quantity, MicroProps& micros,
UErrorCode& status) const {
parent->processQuantity(quantity, micros, status);
if (micros.modMiddle != nullptr) {
return;
}
applyToMicros(micros, quantity, status);
}
@ -152,6 +148,10 @@ const Modifier* ImmutablePatternModifier::getModifier(Signum signum, StandardPlu
}
}
void ImmutablePatternModifier::addToChain(const MicroPropsGenerator* parent) {
this->parent = parent;
}
/** Used by the unsafe code path. */
MicroPropsGenerator& MutablePatternModifier::addToChain(const MicroPropsGenerator* parent) {
@ -162,6 +162,9 @@ MicroPropsGenerator& MutablePatternModifier::addToChain(const MicroPropsGenerato
void MutablePatternModifier::processQuantity(DecimalQuantity& fq, MicroProps& micros,
UErrorCode& status) const {
fParent->processQuantity(fq, micros, status);
if (micros.modMiddle != nullptr) {
return;
}
// The unsafe code path performs self-mutation, so we need a const_cast.
// This method needs to be const because it overrides a const method in the parent class.
auto nonConstThis = const_cast<MutablePatternModifier*>(this);

View File

@ -50,9 +50,11 @@ class U_I18N_API ImmutablePatternModifier : public MicroPropsGenerator, public U
const Modifier* getModifier(Signum signum, StandardPlural::Form plural) const;
// Non-const method:
void addToChain(const MicroPropsGenerator* parent);
private:
ImmutablePatternModifier(AdoptingModifierStore* pm, const PluralRules* rules,
const MicroPropsGenerator* parent);
ImmutablePatternModifier(AdoptingModifierStore* pm, const PluralRules* rules);
const LocalPointer<AdoptingModifierStore> pm;
const PluralRules* rules;
@ -165,21 +167,6 @@ class U_I18N_API MutablePatternModifier
*/
ImmutablePatternModifier *createImmutable(UErrorCode &status);
/**
* Creates a new quantity-dependent Modifier that behaves the same as the current instance, but which is immutable
* and can be saved for future use. The number properties in the current instance are mutated; all other properties
* are left untouched.
*
* <p>
* CREATES A NEW HEAP OBJECT; THE CALLER GETS OWNERSHIP.
*
* @param parent
* The QuantityChain to which to chain this immutable.
* @return An immutable that supports both positive and negative numbers.
*/
ImmutablePatternModifier *
createImmutableAndChain(const MicroPropsGenerator *parent, UErrorCode &status);
MicroPropsGenerator &addToChain(const MicroPropsGenerator *parent);
void processQuantity(DecimalQuantity &, MicroProps &micros, UErrorCode &status) const U_OVERRIDE;

View File

@ -294,9 +294,7 @@ RoundingImpl::RoundingImpl(const Precision& precision, UNumberFormatRoundingMode
}
RoundingImpl RoundingImpl::passThrough() {
RoundingImpl retval;
retval.fPassThrough = true;
return retval;
return {};
}
bool RoundingImpl::isSignificantDigits() const {

View File

@ -150,7 +150,7 @@ digits_t doubleFractionLength(double input, int8_t* singleDigit);
*/
class RoundingImpl {
public:
RoundingImpl() = default; // default constructor: leaves object in undefined state
RoundingImpl() = default; // defaults to pass-through rounder
RoundingImpl(const Precision& precision, UNumberFormatRoundingMode roundingMode,
const CurrencyUnit& currency, UErrorCode& status);
@ -186,7 +186,7 @@ class RoundingImpl {
private:
Precision fPrecision;
UNumberFormatRoundingMode fRoundingMode;
bool fPassThrough;
bool fPassThrough = true; // default value
};

View File

@ -155,6 +155,8 @@ class DecNum;
class NumberRangeFormatterImpl;
struct RangeMacroProps;
struct UFormattedNumberImpl;
class MutablePatternModifier;
class ImmutablePatternModifier;
/**
* Used for NumberRangeFormatter and implemented in numrange_fluent.cpp.
@ -980,9 +982,13 @@ class U_I18N_API IntegerWidth : public UMemory {
friend struct impl::MacroProps;
friend struct impl::MicroProps;
// To allow NumberFormatterImpl to access isBogus() and perform other operations:
// To allow NumberFormatterImpl to access isBogus():
friend class impl::NumberFormatterImpl;
// To allow the use of this class when formatting:
friend class impl::MutablePatternModifier;
friend class impl::ImmutablePatternModifier;
// So that NumberPropertyMapper can create instances
friend class impl::NumberPropertyMapper;

View File

@ -155,19 +155,6 @@ public class MutablePatternModifier implements Modifier, SymbolProvider, MicroPr
* @return An immutable that supports both positive and negative numbers.
*/
public ImmutablePatternModifier createImmutable() {
return createImmutableAndChain(null);
}
/**
* Creates a new quantity-dependent Modifier that behaves the same as the current instance, but which
* is immutable and can be saved for future use. The number properties in the current instance are
* mutated; all other properties are left untouched.
*
* @param parent
* The QuantityChain to which to chain this immutable.
* @return An immutable that supports both positive and negative numbers.
*/
public ImmutablePatternModifier createImmutableAndChain(MicroPropsGenerator parent) {
FormattedStringBuilder a = new FormattedStringBuilder();
FormattedStringBuilder b = new FormattedStringBuilder();
if (needsPlurals()) {
@ -184,7 +171,7 @@ public class MutablePatternModifier implements Modifier, SymbolProvider, MicroPr
pm.setModifier(Signum.NEG, plural, createConstantModifier(a, b));
}
pm.freeze();
return new ImmutablePatternModifier(pm, rules, parent);
return new ImmutablePatternModifier(pm, rules);
} else {
// Faster path when plural keyword is not needed.
setNumberProperties(Signum.POS, null);
@ -196,7 +183,7 @@ public class MutablePatternModifier implements Modifier, SymbolProvider, MicroPr
setNumberProperties(Signum.NEG, null);
Modifier negative = createConstantModifier(a, b);
AdoptingModifierStore pm = new AdoptingModifierStore(positive, posZero, negZero, negative);
return new ImmutablePatternModifier(pm, null, parent);
return new ImmutablePatternModifier(pm, null);
}
}
@ -226,20 +213,27 @@ public class MutablePatternModifier implements Modifier, SymbolProvider, MicroPr
public static class ImmutablePatternModifier implements MicroPropsGenerator {
final AdoptingModifierStore pm;
final PluralRules rules;
final MicroPropsGenerator parent;
/* final */ MicroPropsGenerator parent;
ImmutablePatternModifier(
AdoptingModifierStore pm,
PluralRules rules,
MicroPropsGenerator parent) {
PluralRules rules) {
this.pm = pm;
this.rules = rules;
this.parent = null;
}
public ImmutablePatternModifier addToChain(MicroPropsGenerator parent) {
this.parent = parent;
return this;
}
@Override
public MicroProps processQuantity(DecimalQuantity quantity) {
MicroProps micros = parent.processQuantity(quantity);
if (micros.modMiddle != null) {
return micros;
}
applyToMicros(micros, quantity);
return micros;
}
@ -274,6 +268,9 @@ public class MutablePatternModifier implements Modifier, SymbolProvider, MicroPr
@Override
public MicroProps processQuantity(DecimalQuantity fq) {
MicroProps micros = parent.processQuantity(fq);
if (micros.modMiddle != null) {
return micros;
}
if (needsPlurals()) {
StandardPlural pluralForm = RoundingUtils.getPluralSafe(micros.rounder, rules, fq);
setNumberProperties(fq.signum(), pluralForm);

View File

@ -228,6 +228,9 @@ public class RoundingUtils {
*/
public static StandardPlural getPluralSafe(
Precision rounder, PluralRules rules, DecimalQuantity dq) {
if (rounder == null) {
return dq.getStandardPlural(rules);
}
// TODO(ICU-20500): Avoid the copy?
DecimalQuantity copy = dq.createCopy();
rounder.apply(copy);

View File

@ -66,9 +66,10 @@ public class CompactNotation extends Notation {
CompactType compactType,
PluralRules rules,
MutablePatternModifier buildReference,
boolean safe,
MicroPropsGenerator parent) {
// TODO: Add a data cache? It would be keyed by locale, nsName, compact type, and compact style.
return new CompactHandler(this, locale, nsName, compactType, rules, buildReference, parent);
return new CompactHandler(this, locale, nsName, compactType, rules, buildReference, safe, parent);
}
private static class CompactHandler implements MicroPropsGenerator {
@ -76,6 +77,7 @@ public class CompactNotation extends Notation {
final PluralRules rules;
final MicroPropsGenerator parent;
final Map<String, ImmutablePatternModifier> precomputedMods;
final MutablePatternModifier unsafePatternModifier;
final CompactData data;
private CompactHandler(
@ -85,6 +87,7 @@ public class CompactNotation extends Notation {
CompactType compactType,
PluralRules rules,
MutablePatternModifier buildReference,
boolean safe,
MicroPropsGenerator parent) {
this.rules = rules;
this.parent = parent;
@ -94,13 +97,15 @@ public class CompactNotation extends Notation {
} else {
data.populate(notation.compactCustomData);
}
if (buildReference != null) {
if (safe) {
// Safe code path
precomputedMods = new HashMap<>();
precomputeAllModifiers(buildReference);
unsafePatternModifier = null;
} else {
// Unsafe code path
precomputedMods = null;
unsafePatternModifier = buildReference;
}
}
@ -145,13 +150,14 @@ public class CompactNotation extends Notation {
} else {
// Unsafe code path.
// Overwrite the PatternInfo in the existing modMiddle.
assert micros.modMiddle instanceof MutablePatternModifier;
ParsedPatternInfo patternInfo = PatternStringParser.parseToPatternInfo(patternString);
((MutablePatternModifier) micros.modMiddle).setPatternInfo(patternInfo, NumberFormat.Field.COMPACT);
unsafePatternModifier.setPatternInfo(patternInfo, NumberFormat.Field.COMPACT);
unsafePatternModifier.setNumberProperties(quantity.signum(), null);
micros.modMiddle = unsafePatternModifier;
}
// We already performed rounding. Do not perform it again.
micros.rounder = Precision.constructPassThrough();
micros.rounder = null;
return micros;
}

View File

@ -17,6 +17,7 @@ import com.ibm.icu.impl.number.MicroProps;
import com.ibm.icu.impl.number.MicroPropsGenerator;
import com.ibm.icu.impl.number.MultiplierFormatHandler;
import com.ibm.icu.impl.number.MutablePatternModifier;
import com.ibm.icu.impl.number.MutablePatternModifier.ImmutablePatternModifier;
import com.ibm.icu.impl.number.Padder;
import com.ibm.icu.impl.number.PatternStringParser;
import com.ibm.icu.impl.number.PatternStringParser.ParsedPatternInfo;
@ -97,7 +98,9 @@ class NumberFormatterImpl {
*/
public MicroProps preProcess(DecimalQuantity inValue) {
MicroProps micros = microPropsGenerator.processQuantity(inValue);
micros.rounder.apply(inValue);
if (micros.rounder != null) {
micros.rounder.apply(inValue);
}
if (micros.integerWidth.maxInt == -1) {
inValue.setMinInteger(micros.integerWidth.minInt);
} else {
@ -111,7 +114,9 @@ class NumberFormatterImpl {
MicroProps micros = new MicroProps(false);
MicroPropsGenerator microPropsGenerator = macrosToMicroGenerator(macros, micros, false);
micros = microPropsGenerator.processQuantity(inValue);
micros.rounder.apply(inValue);
if (micros.rounder != null) {
micros.rounder.apply(inValue);
}
if (micros.integerWidth.maxInt == -1) {
inValue.setMinInteger(micros.integerWidth.minInt);
} else {
@ -341,10 +346,9 @@ class NumberFormatterImpl {
} else {
patternMod.setSymbols(micros.symbols, currency, unitWidth, null);
}
ImmutablePatternModifier immPatternMod = null;
if (safe) {
chain = patternMod.createImmutableAndChain(chain);
} else {
chain = patternMod.addToChain(chain);
immPatternMod = patternMod.createImmutable();
}
// Outer modifier (CLDR units and currency long names)
@ -367,8 +371,6 @@ class NumberFormatterImpl {
}
// Compact notation
// NOTE: Compact notation can (but might not) override the middle modifier and rounding.
// It therefore needs to go at the end of the chain.
if (macros.notation instanceof CompactNotation) {
if (rules == null) {
// Lazily create PluralRules
@ -381,10 +383,18 @@ class NumberFormatterImpl {
micros.nsName,
compactType,
rules,
safe ? patternMod : null,
patternMod,
safe,
chain);
}
// Always add the pattern modifier as the last element of the chain.
if (safe) {
chain = immPatternMod.addToChain(chain);
} else {
chain = patternMod.addToChain(chain);
}
return chain;
}

View File

@ -384,8 +384,6 @@ public abstract class Precision implements Cloneable {
static final CurrencyRounderImpl MONETARY_STANDARD = new CurrencyRounderImpl(CurrencyUsage.STANDARD);
static final CurrencyRounderImpl MONETARY_CASH = new CurrencyRounderImpl(CurrencyUsage.CASH);
static final PassThroughRounderImpl PASS_THROUGH = new PassThroughRounderImpl();
static Precision constructInfinite() {
return NONE;
}
@ -474,10 +472,6 @@ public abstract class Precision implements Cloneable {
return returnValue.withMode(base.mathContext);
}
static Precision constructPassThrough() {
return PASS_THROUGH;
}
/**
* Returns a valid working Rounder. If the Rounder is a CurrencyRounder, applies the given currency.
* Otherwise, simply passes through the argument.
@ -713,17 +707,6 @@ public abstract class Precision implements Cloneable {
}
}
static class PassThroughRounderImpl extends Precision {
public PassThroughRounderImpl() {
}
@Override
public void apply(DecimalQuantity value) {
// TODO: Assert that value has already been rounded
}
}
private static int getRoundingMagnitudeFraction(int maxFrac) {
if (maxFrac == -1) {
return Integer.MIN_VALUE;

View File

@ -196,7 +196,7 @@ public class ScientificNotation extends Notation implements Cloneable {
}
// We already performed rounding. Do not perform it again.
micros.rounder = Precision.constructPassThrough();
micros.rounder = null;
return micros;
}