From c940df09e781939d74fb83dcb299cc00c3d1647e Mon Sep 17 00:00:00 2001 From: Shane Carr Date: Sat, 17 Mar 2018 07:24:02 +0000 Subject: [PATCH] ICU-13644 Adds move operators and related boilerplate to NumberFormatter classes. Includes a handful of other changes made to these files on my branch for ICU-13634 . X-SVN-Rev: 41121 --- icu4c/source/i18n/number_fluent.cpp | 334 +++++++++++++-- icu4c/source/i18n/unicode/numberformatter.h | 395 ++++++++++++++++-- icu4c/source/test/intltest/numbertest.h | 1 + icu4c/source/test/intltest/numbertest_api.cpp | 76 ++++ 4 files changed, 743 insertions(+), 63 deletions(-) diff --git a/icu4c/source/i18n/number_fluent.cpp b/icu4c/source/i18n/number_fluent.cpp index b5ff4532a5b..372e6f18d83 100644 --- a/icu4c/source/i18n/number_fluent.cpp +++ b/icu4c/source/i18n/number_fluent.cpp @@ -1,6 +1,7 @@ // © 2017 and later: Unicode, Inc. and others. // License & terms of use: http://www.unicode.org/copyright.html +#include #include "unicode/utypes.h" #if !UCONFIG_NO_FORMATTING && !UPRV_INCOMPLETE_CPP11_SUPPORT @@ -16,7 +17,7 @@ using namespace icu::number; using namespace icu::number::impl; template -Derived NumberFormatterSettings::notation(const Notation& notation) const { +Derived NumberFormatterSettings::notation(const Notation& notation) const & { Derived copy(*this); // NOTE: Slicing is OK. copy.fMacros.notation = notation; @@ -24,7 +25,15 @@ Derived NumberFormatterSettings::notation(const Notation& notation) con } template -Derived NumberFormatterSettings::unit(const icu::MeasureUnit& unit) const { +Derived NumberFormatterSettings::notation(const Notation& notation) && { + Derived move(std::move(*this)); + // NOTE: Slicing is OK. + move.fMacros.notation = notation; + return move; +} + +template +Derived NumberFormatterSettings::unit(const icu::MeasureUnit& unit) const & { Derived copy(*this); // NOTE: Slicing occurs here. However, CurrencyUnit can be restored from MeasureUnit. // TimeUnit may be affected, but TimeUnit is not as relevant to number formatting. @@ -33,21 +42,41 @@ Derived NumberFormatterSettings::unit(const icu::MeasureUnit& unit) con } template -Derived NumberFormatterSettings::adoptUnit(icu::MeasureUnit* unit) const { +Derived NumberFormatterSettings::unit(const icu::MeasureUnit& unit) && { + Derived move(std::move(*this)); + // See comments above about slicing. + move.fMacros.unit = unit; + return move; +} + +template +Derived NumberFormatterSettings::adoptUnit(icu::MeasureUnit* unit) const & { Derived copy(*this); - // Just copy the unit into the MacroProps by value, and delete it since we have ownership. + // Just move the unit into the MacroProps by value, and delete it since we have ownership. // NOTE: Slicing occurs here. However, CurrencyUnit can be restored from MeasureUnit. // TimeUnit may be affected, but TimeUnit is not as relevant to number formatting. if (unit != nullptr) { // TODO: On nullptr, reset to default value? - copy.fMacros.unit = *unit; + copy.fMacros.unit = std::move(*unit); delete unit; } return copy; } template -Derived NumberFormatterSettings::perUnit(const icu::MeasureUnit& perUnit) const { +Derived NumberFormatterSettings::adoptUnit(icu::MeasureUnit* unit) && { + Derived move(std::move(*this)); + // See comments above about slicing and ownership. + if (unit != nullptr) { + // TODO: On nullptr, reset to default value? + move.fMacros.unit = std::move(*unit); + delete unit; + } + return move; +} + +template +Derived NumberFormatterSettings::perUnit(const icu::MeasureUnit& perUnit) const & { Derived copy(*this); // See comments above about slicing. copy.fMacros.perUnit = perUnit; @@ -55,19 +84,39 @@ Derived NumberFormatterSettings::perUnit(const icu::MeasureUnit& perUni } template -Derived NumberFormatterSettings::adoptPerUnit(icu::MeasureUnit* perUnit) const { +Derived NumberFormatterSettings::perUnit(const icu::MeasureUnit& perUnit) && { + Derived copy(*this); + // See comments above about slicing. + copy.fMacros.perUnit = perUnit; + return copy; +} + +template +Derived NumberFormatterSettings::adoptPerUnit(icu::MeasureUnit* perUnit) const & { + Derived move(std::move(*this)); + // See comments above about slicing and ownership. + if (perUnit != nullptr) { + // TODO: On nullptr, reset to default value? + move.fMacros.perUnit = std::move(*perUnit); + delete perUnit; + } + return move; +} + +template +Derived NumberFormatterSettings::adoptPerUnit(icu::MeasureUnit* perUnit) && { Derived copy(*this); // See comments above about slicing and ownership. if (perUnit != nullptr) { // TODO: On nullptr, reset to default value? - copy.fMacros.perUnit = *perUnit; + copy.fMacros.perUnit = std::move(*perUnit); delete perUnit; } return copy; } template -Derived NumberFormatterSettings::rounding(const Rounder& rounder) const { +Derived NumberFormatterSettings::rounding(const Rounder& rounder) const & { Derived copy(*this); // NOTE: Slicing is OK. copy.fMacros.rounder = rounder; @@ -75,7 +124,15 @@ Derived NumberFormatterSettings::rounding(const Rounder& rounder) const } template -Derived NumberFormatterSettings::grouping(const UGroupingStrategy& strategy) const { +Derived NumberFormatterSettings::rounding(const Rounder& rounder) && { + Derived move(std::move(*this)); + // NOTE: Slicing is OK. + move.fMacros.rounder = rounder; + return move; +} + +template +Derived NumberFormatterSettings::grouping(const UGroupingStrategy& strategy) const & { Derived copy(*this); // NOTE: This is slightly different than how the setting is stored in Java // because we want to put it on the stack. @@ -84,68 +141,152 @@ Derived NumberFormatterSettings::grouping(const UGroupingStrategy& stra } template -Derived NumberFormatterSettings::integerWidth(const IntegerWidth& style) const { +Derived NumberFormatterSettings::grouping(const UGroupingStrategy& strategy) && { + Derived move(std::move(*this)); + move.fMacros.grouper = Grouper::forStrategy(strategy); + return move; +} + +template +Derived NumberFormatterSettings::integerWidth(const IntegerWidth& style) const & { Derived copy(*this); copy.fMacros.integerWidth = style; return copy; } template -Derived NumberFormatterSettings::symbols(const DecimalFormatSymbols& symbols) const { +Derived NumberFormatterSettings::integerWidth(const IntegerWidth& style) && { + Derived move(std::move(*this)); + move.fMacros.integerWidth = style; + return move; +} + +template +Derived NumberFormatterSettings::symbols(const DecimalFormatSymbols& symbols) const & { Derived copy(*this); copy.fMacros.symbols.setTo(symbols); return copy; } template -Derived NumberFormatterSettings::adoptSymbols(NumberingSystem* ns) const { +Derived NumberFormatterSettings::symbols(const DecimalFormatSymbols& symbols) && { + Derived move(std::move(*this)); + move.fMacros.symbols.setTo(symbols); + return move; +} + +template +Derived NumberFormatterSettings::adoptSymbols(NumberingSystem* ns) const & { Derived copy(*this); copy.fMacros.symbols.setTo(ns); return copy; } template -Derived NumberFormatterSettings::unitWidth(const UNumberUnitWidth& width) const { +Derived NumberFormatterSettings::adoptSymbols(NumberingSystem* ns) && { + Derived move(std::move(*this)); + move.fMacros.symbols.setTo(ns); + return move; +} + +template +Derived NumberFormatterSettings::unitWidth(const UNumberUnitWidth& width) const & { Derived copy(*this); copy.fMacros.unitWidth = width; return copy; } template -Derived NumberFormatterSettings::sign(const UNumberSignDisplay& style) const { +Derived NumberFormatterSettings::unitWidth(const UNumberUnitWidth& width) && { + Derived move(std::move(*this)); + move.fMacros.unitWidth = width; + return move; +} + +template +Derived NumberFormatterSettings::sign(const UNumberSignDisplay& style) const & { Derived copy(*this); copy.fMacros.sign = style; return copy; } template -Derived NumberFormatterSettings::decimal(const UNumberDecimalSeparatorDisplay& style) const { +Derived NumberFormatterSettings::sign(const UNumberSignDisplay& style) && { + Derived move(std::move(*this)); + move.fMacros.sign = style; + return move; +} + +template +Derived NumberFormatterSettings::decimal(const UNumberDecimalSeparatorDisplay& style) const & { Derived copy(*this); copy.fMacros.decimal = style; return copy; } template -Derived NumberFormatterSettings::padding(const Padder& padder) const { +Derived NumberFormatterSettings::decimal(const UNumberDecimalSeparatorDisplay& style) && { + Derived move(std::move(*this)); + move.fMacros.decimal = style; + return move; +} + +template +Derived NumberFormatterSettings::padding(const Padder& padder) const & { Derived copy(*this); copy.fMacros.padder = padder; return copy; } template -Derived NumberFormatterSettings::threshold(int32_t threshold) const { +Derived NumberFormatterSettings::padding(const Padder& padder) && { + Derived move(std::move(*this)); + move.fMacros.padder = padder; + return move; +} + +template +Derived NumberFormatterSettings::threshold(int32_t threshold) const & { Derived copy(*this); copy.fMacros.threshold = threshold; return copy; } template -Derived NumberFormatterSettings::macros(impl::MacroProps& macros) const { +Derived NumberFormatterSettings::threshold(int32_t threshold) && { + Derived move(std::move(*this)); + move.fMacros.threshold = threshold; + return move; +} + +template +Derived NumberFormatterSettings::macros(const impl::MacroProps& macros) const & { Derived copy(*this); copy.fMacros = macros; return copy; } +template +Derived NumberFormatterSettings::macros(const impl::MacroProps& macros) && { + Derived move(std::move(*this)); + move.fMacros = macros; + return move; +} + +template +Derived NumberFormatterSettings::macros(impl::MacroProps&& macros) const & { + Derived copy(*this); + copy.fMacros = std::move(macros); + return copy; +} + +template +Derived NumberFormatterSettings::macros(impl::MacroProps&& macros) && { + Derived move(std::move(*this)); + move.fMacros = std::move(macros); + return move; +} + // Declare all classes that implement NumberFormatterSettings // See https://stackoverflow.com/a/495056/1407170 template @@ -163,18 +304,82 @@ LocalizedNumberFormatter NumberFormatter::withLocale(const Locale& locale) { return with().locale(locale); } -// Make the child class constructor that takes the parent class call the parent class's copy constructor -UnlocalizedNumberFormatter::UnlocalizedNumberFormatter( - const NumberFormatterSettings& other) - : NumberFormatterSettings(other) { + +template +using NFS = NumberFormatterSettings; +using LNF = LocalizedNumberFormatter; +using UNF = UnlocalizedNumberFormatter; + +UnlocalizedNumberFormatter::UnlocalizedNumberFormatter(const UNF& other) + : UNF(static_cast&>(other)) {} + +UnlocalizedNumberFormatter::UnlocalizedNumberFormatter(const NFS& other) + : NFS(other) { + // No additional fields to assign } -// Make the child class constructor that takes the parent class call the parent class's copy constructor -// For LocalizedNumberFormatter, also copy over the extra fields -LocalizedNumberFormatter::LocalizedNumberFormatter( - const NumberFormatterSettings& other) - : NumberFormatterSettings(other) { - // No additional copies required +UnlocalizedNumberFormatter::UnlocalizedNumberFormatter(UNF&& src) U_NOEXCEPT + : UNF(static_cast&&>(src)) {} + +UnlocalizedNumberFormatter::UnlocalizedNumberFormatter(NFS&& src) U_NOEXCEPT + : NFS(std::move(src)) { + // No additional fields to assign +} + +UnlocalizedNumberFormatter& UnlocalizedNumberFormatter::operator=(const UNF& other) { + NFS::operator=(static_cast&>(other)); + // No additional fields to assign + return *this; +} + +UnlocalizedNumberFormatter& UnlocalizedNumberFormatter::operator=(UNF&& src) U_NOEXCEPT { + NFS::operator=(static_cast&&>(src)); + // No additional fields to assign + return *this; +} + +LocalizedNumberFormatter::LocalizedNumberFormatter(const LNF& other) + : LNF(static_cast&>(other)) {} + +LocalizedNumberFormatter::LocalizedNumberFormatter(const NFS& other) + : NFS(other) { + // No additional fields to assign (let call count and compiled formatter reset to defaults) +} + +LocalizedNumberFormatter::LocalizedNumberFormatter(LocalizedNumberFormatter&& src) U_NOEXCEPT + : LNF(static_cast&&>(src)) {} + +LocalizedNumberFormatter::LocalizedNumberFormatter(NFS&& src) U_NOEXCEPT + : NFS(std::move(src)) { + // For the move operators, copy over the call count and compiled formatter. + auto&& srcAsLNF = static_cast(src); + fCompiled = srcAsLNF.fCompiled; + uprv_memcpy(fUnsafeCallCount, srcAsLNF.fUnsafeCallCount, sizeof(fUnsafeCallCount)); + // Reset the source object to leave it in a safe state. + srcAsLNF.fCompiled = nullptr; + uprv_memset(srcAsLNF.fUnsafeCallCount, 0, sizeof(fUnsafeCallCount)); +} + +LocalizedNumberFormatter& LocalizedNumberFormatter::operator=(const LNF& other) { + NFS::operator=(static_cast&>(other)); + // No additional fields to assign (let call count and compiled formatter reset to defaults) + return *this; +} + +LocalizedNumberFormatter& LocalizedNumberFormatter::operator=(LNF&& src) U_NOEXCEPT { + NFS::operator=(static_cast&&>(src)); + // For the move operators, copy over the call count and compiled formatter. + fCompiled = src.fCompiled; + uprv_memcpy(fUnsafeCallCount, src.fUnsafeCallCount, sizeof(fUnsafeCallCount)); + // Reset the source object to leave it in a safe state. + src.fCompiled = nullptr; + uprv_memset(src.fUnsafeCallCount, 0, sizeof(fUnsafeCallCount)); + return *this; +} + + +LocalizedNumberFormatter::~LocalizedNumberFormatter() { + delete fCompiled; } LocalizedNumberFormatter::LocalizedNumberFormatter(const MacroProps& macros, const Locale& locale) { @@ -182,14 +387,27 @@ LocalizedNumberFormatter::LocalizedNumberFormatter(const MacroProps& macros, con fMacros.locale = locale; } -LocalizedNumberFormatter UnlocalizedNumberFormatter::locale(const Locale& locale) const { +LocalizedNumberFormatter::LocalizedNumberFormatter(MacroProps&& macros, const Locale& locale) { + fMacros = std::move(macros); + fMacros.locale = locale; +} + +LocalizedNumberFormatter UnlocalizedNumberFormatter::locale(const Locale& locale) const & { return LocalizedNumberFormatter(fMacros, locale); } +LocalizedNumberFormatter UnlocalizedNumberFormatter::locale(const Locale& locale) && { + return LocalizedNumberFormatter(std::move(fMacros), locale); +} + SymbolsWrapper::SymbolsWrapper(const SymbolsWrapper& other) { doCopyFrom(other); } +SymbolsWrapper::SymbolsWrapper(SymbolsWrapper&& src) U_NOEXCEPT { + doMoveFrom(std::move(src)); +} + SymbolsWrapper& SymbolsWrapper::operator=(const SymbolsWrapper& other) { if (this == &other) { return *this; @@ -199,6 +417,15 @@ SymbolsWrapper& SymbolsWrapper::operator=(const SymbolsWrapper& other) { return *this; } +SymbolsWrapper& SymbolsWrapper::operator=(SymbolsWrapper&& src) U_NOEXCEPT { + if (this == &src) { + return *this; + } + doCleanup(); + doMoveFrom(std::move(src)); + return *this; +} + SymbolsWrapper::~SymbolsWrapper() { doCleanup(); } @@ -240,6 +467,23 @@ void SymbolsWrapper::doCopyFrom(const SymbolsWrapper& other) { } } +void SymbolsWrapper::doMoveFrom(SymbolsWrapper&& src) { + fType = src.fType; + switch (fType) { + case SYMPTR_NONE: + // No action necessary + break; + case SYMPTR_DFS: + fPtr.dfs = src.fPtr.dfs; + src.fPtr.dfs = nullptr; + break; + case SYMPTR_NS: + fPtr.ns = src.fPtr.ns; + src.fPtr.ns = nullptr; + break; + } +} + void SymbolsWrapper::doCleanup() { switch (fType) { case SYMPTR_NONE: @@ -272,8 +516,22 @@ const NumberingSystem* SymbolsWrapper::getNumberingSystem() const { return fPtr.ns; } -LocalizedNumberFormatter::~LocalizedNumberFormatter() { - delete fCompiled; + +FormattedNumber::FormattedNumber(FormattedNumber&& src) U_NOEXCEPT + : fResults(src.fResults), fErrorCode(src.fErrorCode) { + // Disown src.fResults to prevent double-deletion + src.fResults = nullptr; + src.fErrorCode = U_INVALID_STATE_ERROR; +} + +FormattedNumber& FormattedNumber::operator=(FormattedNumber&& src) U_NOEXCEPT { + delete fResults; + fResults = src.fResults; + fErrorCode = src.fErrorCode; + // Disown src.fResults to prevent double-deletion + src.fResults = nullptr; + src.fErrorCode = U_INVALID_STATE_ERROR; + return *this; } FormattedNumber LocalizedNumberFormatter::formatInt(int64_t value, UErrorCode& status) const { @@ -330,7 +588,7 @@ LocalizedNumberFormatter::formatImpl(impl::NumberFormatterResults* results, UErr static_assert( sizeof(u_atomic_int32_t) <= sizeof(fUnsafeCallCount), "Atomic integer size on this platform exceeds the size allocated by fUnsafeCallCount"); - u_atomic_int32_t* callCount = reinterpret_cast( + auto* callCount = reinterpret_cast( const_cast(this)->fUnsafeCallCount); // A positive value in the atomic int indicates that the data structure is not yet ready; @@ -368,6 +626,16 @@ LocalizedNumberFormatter::formatImpl(impl::NumberFormatterResults* results, UErr } } +const impl::NumberFormatterImpl* LocalizedNumberFormatter::getCompiled() const { + return fCompiled; +} + +int32_t LocalizedNumberFormatter::getCallCount() const { + auto* callCount = reinterpret_cast( + const_cast(this)->fUnsafeCallCount); + return umtx_loadAcquire(*callCount); +} + UnicodeString FormattedNumber::toString() const { if (fResults == nullptr) { // TODO: http://bugs.icu-project.org/trac/ticket/13437 diff --git a/icu4c/source/i18n/unicode/numberformatter.h b/icu4c/source/i18n/unicode/numberformatter.h index 56d67668e76..4f964fb1d6a 100644 --- a/icu4c/source/i18n/unicode/numberformatter.h +++ b/icu4c/source/i18n/unicode/numberformatter.h @@ -441,6 +441,11 @@ class ScientificHandler; class CompactHandler; class Modifier; class NumberStringBuilder; +class AffixPatternProvider; +class NumberPropertyMapper; +struct DecimalFormatProperties; +class MultiplierChain; +class CurrencySymbols; } // namespace impl @@ -691,7 +696,14 @@ class U_I18N_API ScientificNotation : public Notation { // Inherit constructor using Notation::Notation; + // Raw constructor for NumberPropertyMapper + ScientificNotation(int8_t fEngineeringInterval, bool fRequireMinInt, impl::digits_t fMinExponentDigits, + UNumberSignDisplay fExponentSignDisplay); + friend class Notation; + + // So that NumberPropertyMapper can create instances + friend class impl::NumberPropertyMapper; }; // Reserve extra names in case they are added as classes in the future: @@ -931,6 +943,7 @@ class U_I18N_API Rounder : public UMemory { struct IncrementSettings { double fIncrement; impl::digits_t fMinFrac; + impl::digits_t fMaxFrac; } increment; // For RND_INCREMENT UCurrencyUsage currencyUsage; // For RND_CURRENCY UErrorCode errorCode; // For RND_ERROR @@ -1011,6 +1024,9 @@ class U_I18N_API Rounder : public UMemory { // To allow NumberFormatterImpl to access isBogus() and other internal methods: friend class impl::NumberFormatterImpl; + // To allow NumberPropertyMapper to create instances from DecimalFormatProperties: + friend class impl::NumberPropertyMapper; + // To give access to apply() and chooseMultiplierAndApply(): friend class impl::MutablePatternModifier; friend class impl::LongNameHandler; @@ -1246,12 +1262,18 @@ class U_I18N_API SymbolsWrapper : public UMemory { /** @internal */ SymbolsWrapper(const SymbolsWrapper &other); + /** @internal */ + SymbolsWrapper(SymbolsWrapper&& src) U_NOEXCEPT; + /** @internal */ ~SymbolsWrapper(); /** @internal */ SymbolsWrapper &operator=(const SymbolsWrapper &other); + /** @internal */ + SymbolsWrapper &operator=(SymbolsWrapper&& src) U_NOEXCEPT; + /** * The provided object is copied, but we do not adopt it. * @internal @@ -1312,6 +1334,8 @@ class U_I18N_API SymbolsWrapper : public UMemory { void doCopyFrom(const SymbolsWrapper &other); + void doMoveFrom(SymbolsWrapper&& src); + void doCleanup(); }; @@ -1321,6 +1345,12 @@ class U_I18N_API Grouper : public UMemory { /** @internal */ static Grouper forStrategy(UGroupingStrategy grouping); + /** + * Resolve the values in Properties to a Grouper object. + * @internal + */ + static Grouper forProperties(const DecimalFormatProperties& properties); + // Future: static Grouper forProperties(DecimalFormatProperties& properties); /** @internal */ @@ -1385,6 +1415,9 @@ class U_I18N_API Padder : public UMemory { /** @internal */ static Padder codePoints(UChar32 cp, int32_t targetWidth, UNumberFormatPadPosition position); + /** @internal */ + static Padder forProperties(const DecimalFormatProperties& properties); + private: UChar32 fWidth; // -3 = error; -2 = bogus; -1 = no padding union { @@ -1433,6 +1466,38 @@ class U_I18N_API Padder : public UMemory { friend class impl::NumberFormatterImpl; }; +/** @internal */ +class U_I18N_API Multiplier : public UMemory { + public: + /** @internal */ + static Multiplier magnitude(int32_t magnitudeMultiplier); + + /** @internal */ + static Multiplier integer(int32_t multiplier); + + private: + int32_t magnitudeMultiplier; + int32_t multiplier; + + Multiplier(int32_t magnitudeMultiplier, int32_t multiplier); + + Multiplier() : magnitudeMultiplier(0), multiplier(1) {} + + bool isValid() const { + return magnitudeMultiplier != 0 || multiplier != 1; + } + + // To allow MacroProps/MicroProps to initialize empty instances: + friend struct MacroProps; + friend struct MicroProps; + + // To allow NumberFormatterImpl to access isBogus() and perform other operations: + friend class impl::NumberFormatterImpl; + + // To allow the helper class MultiplierChain access to private fields: + friend class impl::MultiplierChain; +}; + /** @internal */ struct U_I18N_API MacroProps : public UMemory { /** @internal */ @@ -1470,13 +1535,24 @@ struct U_I18N_API MacroProps : public UMemory { /** @internal */ UNumberDecimalSeparatorDisplay decimal = UNUM_DECIMAL_SEPARATOR_COUNT; + Multiplier multiplier; // = Multiplier(); (bogus) + + AffixPatternProvider* affixProvider = nullptr; // no ownership + /** @internal */ PluralRules *rules = nullptr; // no ownership + /** @internal */ + CurrencySymbols *currencySymbols = nullptr; // no ownership + /** @internal */ int32_t threshold = DEFAULT_THRESHOLD; + + /** @internal */ Locale locale; + // NOTE: Uses default copy and move constructors. + /** * Check all members for errors. * @internal @@ -1525,7 +1601,18 @@ class U_I18N_API NumberFormatterSettings { * @see Notation * @draft ICU 60 */ - Derived notation(const Notation ¬ation) const; + Derived notation(const Notation ¬ation) const &; + + /** + * Overload of notation() for use on an rvalue reference. + * + * @param notation + * The notation strategy to use. + * @return The fluent chain. + * @see #notation + * @draft ICU 62 + */ + Derived notation(const Notation ¬ation) &&; /** * Specifies the unit (unit of measure, currency, or percent) to associate with rendered numbers. @@ -1571,7 +1658,18 @@ class U_I18N_API NumberFormatterSettings { * @see #perUnit * @draft ICU 60 */ - Derived unit(const icu::MeasureUnit &unit) const; + Derived unit(const icu::MeasureUnit &unit) const &; + + /** + * Overload of unit() for use on an rvalue reference. + * + * @param unit + * The unit to render. + * @return The fluent chain. + * @see #unit + * @draft ICU 62 + */ + Derived unit(const icu::MeasureUnit &unit) &&; /** * Like unit(), but takes ownership of a pointer. Convenient for use with the MeasureFormat factory @@ -1588,7 +1686,18 @@ class U_I18N_API NumberFormatterSettings { * @see MeasureUnit * @draft ICU 60 */ - Derived adoptUnit(icu::MeasureUnit *unit) const; + Derived adoptUnit(icu::MeasureUnit *unit) const &; + + /** + * Overload of adoptUnit() for use on an rvalue reference. + * + * @param unit + * The unit to render. + * @return The fluent chain. + * @see #adoptUnit + * @draft ICU 62 + */ + Derived adoptUnit(icu::MeasureUnit *unit) &&; /** * Sets a unit to be used in the denominator. For example, to format "3 m/s", pass METER to the unit and SECOND to @@ -1607,7 +1716,18 @@ class U_I18N_API NumberFormatterSettings { * @see #unit * @draft ICU 61 */ - Derived perUnit(const icu::MeasureUnit &perUnit) const; + Derived perUnit(const icu::MeasureUnit &perUnit) const &; + + /** + * Overload of perUnit() for use on an rvalue reference. + * + * @param perUnit + * The unit to render in the denominator. + * @return The fluent chain. + * @see #perUnit + * @draft ICU 62 + */ + Derived perUnit(const icu::MeasureUnit &perUnit) &&; /** * Like perUnit(), but takes ownership of a pointer. Convenient for use with the MeasureFormat factory @@ -1626,7 +1746,18 @@ class U_I18N_API NumberFormatterSettings { * @see MeasureUnit * @draft ICU 61 */ - Derived adoptPerUnit(icu::MeasureUnit *perUnit) const; + Derived adoptPerUnit(icu::MeasureUnit *perUnit) const &; + + /** + * Overload of adoptPerUnit() for use on an rvalue reference. + * + * @param perUnit + * The unit to render in the denominator. + * @return The fluent chain. + * @see #adoptPerUnit + * @draft ICU 62 + */ + Derived adoptPerUnit(icu::MeasureUnit *perUnit) &&; /** * Specifies the rounding strategy to use when formatting numbers. @@ -1656,10 +1787,20 @@ class U_I18N_API NumberFormatterSettings { * The rounding strategy to use. * @return The fluent chain. * @see Rounder - * @provisional This API might change or be removed in a future release. * @draft ICU 60 */ - Derived rounding(const Rounder &rounder) const; + Derived rounding(const Rounder &rounder) const &; + + /** + * Overload of rounding() for use on an rvalue reference. + * + * @param rounder + * The rounding strategy to use. + * @return The fluent chain. + * @see #rounding + * @draft ICU 62 + */ + Derived rounding(const Rounder& rounder) &&; /** * Specifies the grouping strategy to use when formatting numbers. @@ -1688,7 +1829,19 @@ class U_I18N_API NumberFormatterSettings { * @return The fluent chain. * @draft ICU 61 */ - Derived grouping(const UGroupingStrategy &strategy) const; + Derived grouping(const UGroupingStrategy &strategy) const &; + + /** + * Overload of grouping() for use on an rvalue reference. + * + * @param rounder + * The grouping strategy to use. + * @return The fluent chain. + * @see #grouping + * @provisional This API might change or be removed in a future release. + * @draft ICU 62 + */ + Derived grouping(const UGroupingStrategy& rounder) &&; /** * Specifies the minimum and maximum number of digits to render before the decimal mark. @@ -1714,7 +1867,18 @@ class U_I18N_API NumberFormatterSettings { * @see IntegerWidth * @draft ICU 60 */ - Derived integerWidth(const IntegerWidth &style) const; + Derived integerWidth(const IntegerWidth &style) const &; + + /** + * Overload of integerWidth() for use on an rvalue reference. + * + * @param style + * The integer width to use. + * @return The fluent chain. + * @see #integerWidth + * @draft ICU 62 + */ + Derived integerWidth(const IntegerWidth &style) &&; /** * Specifies the symbols (decimal separator, grouping separator, percent sign, numerals, etc.) to use when rendering @@ -1756,7 +1920,18 @@ class U_I18N_API NumberFormatterSettings { * @see DecimalFormatSymbols * @draft ICU 60 */ - Derived symbols(const DecimalFormatSymbols &symbols) const; + Derived symbols(const DecimalFormatSymbols &symbols) const &; + + /** + * Overload of symbols() for use on an rvalue reference. + * + * @param symbols + * The DecimalFormatSymbols to use. + * @return The fluent chain. + * @see #symbols + * @draft ICU 62 + */ + Derived symbols(const DecimalFormatSymbols &symbols) &&; /** * Specifies that the given numbering system should be used when fetching symbols. @@ -1791,7 +1966,18 @@ class U_I18N_API NumberFormatterSettings { * @see NumberingSystem * @draft ICU 60 */ - Derived adoptSymbols(NumberingSystem *symbols) const; + Derived adoptSymbols(NumberingSystem *symbols) const &; + + /** + * Overload of adoptSymbols() for use on an rvalue reference. + * + * @param symbols + * The NumberingSystem to use. + * @return The fluent chain. + * @see #adoptSymbols + * @draft ICU 62 + */ + Derived adoptSymbols(NumberingSystem *symbols) &&; /** * Sets the width of the unit (measure unit or currency). Most common values: @@ -1818,7 +2004,18 @@ class U_I18N_API NumberFormatterSettings { * @see UNumberUnitWidth * @draft ICU 60 */ - Derived unitWidth(const UNumberUnitWidth &width) const; + Derived unitWidth(const UNumberUnitWidth &width) const &; + + /** + * Overload of unitWidth() for use on an rvalue reference. + * + * @param width + * The width to use when rendering numbers. + * @return The fluent chain. + * @see #unitWidth + * @draft ICU 62 + */ + Derived unitWidth(const UNumberUnitWidth &width) &&; /** * Sets the plus/minus sign display strategy. Most common values: @@ -1839,14 +2036,25 @@ class U_I18N_API NumberFormatterSettings { *

* The default is AUTO sign display. * - * @param width + * @param style * The sign display strategy to use when rendering numbers. * @return The fluent chain * @see UNumberSignDisplay * @provisional This API might change or be removed in a future release. * @draft ICU 60 */ - Derived sign(const UNumberSignDisplay &width) const; + Derived sign(const UNumberSignDisplay &style) const &; + + /** + * Overload of sign() for use on an rvalue reference. + * + * @param style + * The sign display strategy to use when rendering numbers. + * @return The fluent chain. + * @see #sign + * @draft ICU 62 + */ + Derived sign(const UNumberSignDisplay &style) &&; /** * Sets the decimal separator display strategy. This affects integer numbers with no fraction part. Most common @@ -1867,23 +2075,37 @@ class U_I18N_API NumberFormatterSettings { *

* The default is AUTO decimal separator display. * - * @param width + * @param style * The decimal separator display strategy to use when rendering numbers. * @return The fluent chain * @see UNumberDecimalSeparatorDisplay * @provisional This API might change or be removed in a future release. * @draft ICU 60 */ - Derived decimal(const UNumberDecimalSeparatorDisplay &width) const; + Derived decimal(const UNumberDecimalSeparatorDisplay &style) const &; + + /** + * Overload of decimal() for use on an rvalue reference. + * + * @param style + * The decimal separator display strategy to use when rendering numbers. + * @return The fluent chain. + * @see #sign + * @draft ICU 62 + */ + Derived decimal(const UNumberDecimalSeparatorDisplay &style) &&; #ifndef U_HIDE_INTERNAL_API /** - * Set the padding strategy. May be added to ICU 61; see #13338. + * Set the padding strategy. May be added in the future; see #13338. * * @internal ICU 60: This API is ICU internal only. */ - Derived padding(const impl::Padder &padder) const; + Derived padding(const impl::Padder &padder) const &; + + /** @internal */ + Derived padding(const impl::Padder &padder) &&; /** * Internal fluent setter to support a custom regulation threshold. A threshold of 1 causes the data structures to @@ -1891,14 +2113,26 @@ class U_I18N_API NumberFormatterSettings { * * @internal ICU 60: This API is ICU internal only. */ - Derived threshold(int32_t threshold) const; + Derived threshold(int32_t threshold) const &; + + /** @internal */ + Derived threshold(int32_t threshold) &&; /** * Internal fluent setter to overwrite the entire macros object. * * @internal ICU 60: This API is ICU internal only. */ - Derived macros(impl::MacroProps& macros) const; + Derived macros(const impl::MacroProps& macros) const &; + + /** @internal */ + Derived macros(const impl::MacroProps& macros) &&; + + /** @internal */ + Derived macros(impl::MacroProps&& macros) const &; + + /** @internal */ + Derived macros(impl::MacroProps&& macros) &&; #endif /* U_HIDE_INTERNAL_API */ @@ -1917,6 +2151,8 @@ class U_I18N_API NumberFormatterSettings { return U_FAILURE(outErrorCode); } + // NOTE: Uses default copy and move constructors. + protected: impl::MacroProps fMacros; @@ -1954,21 +2190,58 @@ class U_I18N_API UnlocalizedNumberFormatter * @return The fluent chain. * @draft ICU 60 */ - LocalizedNumberFormatter locale(const icu::Locale &locale) const; + LocalizedNumberFormatter locale(const icu::Locale &locale) const &; + + /** + * Overload of locale() for use on an rvalue reference. + * + * @param locale + * The locale to use when loading data for number formatting. + * @return The fluent chain. + * @see #locale + * @draft ICU 62 + */ + LocalizedNumberFormatter locale(const icu::Locale &locale) &&; + + /** + * Default constructor: puts the formatter into a valid but undefined state. + * + * @draft ICU 62 + */ + UnlocalizedNumberFormatter() = default; // Make default copy constructor call the NumberFormatterSettings copy constructor. /** * Returns a copy of this UnlocalizedNumberFormatter. * @draft ICU 60 */ - UnlocalizedNumberFormatter(const UnlocalizedNumberFormatter &other) : UnlocalizedNumberFormatter( - static_cast &>(other)) {} + UnlocalizedNumberFormatter(const UnlocalizedNumberFormatter &other); + + /** + * Move constructor: + * The source UnlocalizedNumberFormatter will be left in a valid but undefined state. + * @draft ICU 62 + */ + UnlocalizedNumberFormatter(UnlocalizedNumberFormatter&& src) U_NOEXCEPT; + + /** + * Copy assignment operator. + * @draft ICU 62 + */ + UnlocalizedNumberFormatter& operator=(const UnlocalizedNumberFormatter& other); + + /** + * Move assignment operator: + * The source UnlocalizedNumberFormatter will be left in a valid but undefined state. + * @draft ICU 62 + */ + UnlocalizedNumberFormatter& operator=(UnlocalizedNumberFormatter&& src) U_NOEXCEPT; private: - UnlocalizedNumberFormatter() = default; + explicit UnlocalizedNumberFormatter(const NumberFormatterSettings& other); explicit UnlocalizedNumberFormatter( - const NumberFormatterSettings &other); + NumberFormatterSettings&& src) U_NOEXCEPT; // To give the fluent setters access to this class's constructor: friend class NumberFormatterSettings; @@ -2026,22 +2299,62 @@ class U_I18N_API LocalizedNumberFormatter * @return A FormattedNumber object; call .toString() to get the string. * @draft ICU 60 */ - FormattedNumber formatDecimal(StringPiece value, UErrorCode &status) const; + FormattedNumber formatDecimal(StringPiece value, UErrorCode& status) const; #ifndef U_HIDE_INTERNAL_API + /** Internal method. * @internal */ FormattedNumber formatDecimalQuantity(const impl::DecimalQuantity& dq, UErrorCode& status) const; + + /** + * Internal method for testing. + * @internal + */ + const impl::NumberFormatterImpl* getCompiled() const; + + /** + * Internal method for testing. + * @internal + */ + int32_t getCallCount() const; + #endif + /** + * Default constructor: puts the formatter into a valid but undefined state. + * + * @draft ICU 62 + */ + LocalizedNumberFormatter() = default; + // Make default copy constructor call the NumberFormatterSettings copy constructor. /** * Returns a copy of this LocalizedNumberFormatter. * @draft ICU 60 */ - LocalizedNumberFormatter(const LocalizedNumberFormatter &other) : LocalizedNumberFormatter( - static_cast &>(other)) {} + LocalizedNumberFormatter(const LocalizedNumberFormatter &other); + + /** + * Move constructor: + * The source LocalizedNumberFormatter will be left in a valid but undefined state. + * @draft ICU 62 + */ + LocalizedNumberFormatter(LocalizedNumberFormatter&& src) U_NOEXCEPT; + + /** + * Copy assignment operator. + * @draft ICU 62 + */ + LocalizedNumberFormatter& operator=(const LocalizedNumberFormatter& other); + + /** + * Move assignment operator: + * The source LocalizedNumberFormatter will be left in a valid but undefined state. + * @draft ICU 62 + */ + LocalizedNumberFormatter& operator=(LocalizedNumberFormatter&& src) U_NOEXCEPT; /** * Destruct this LocalizedNumberFormatter, cleaning up any memory it might own. @@ -2050,15 +2363,19 @@ class U_I18N_API LocalizedNumberFormatter ~LocalizedNumberFormatter(); private: + // Note: fCompiled can't be a LocalPointer because impl::NumberFormatterImpl is defined in an internal + // header, and LocalPointer needs the full class definition in order to delete the instance. const impl::NumberFormatterImpl* fCompiled {nullptr}; char fUnsafeCallCount[8] {}; // internally cast to u_atomic_int32_t - LocalizedNumberFormatter() = default; + explicit LocalizedNumberFormatter(const NumberFormatterSettings& other); - explicit LocalizedNumberFormatter(const NumberFormatterSettings &other); + explicit LocalizedNumberFormatter(NumberFormatterSettings&& src) U_NOEXCEPT; LocalizedNumberFormatter(const impl::MacroProps ¯os, const Locale &locale); + LocalizedNumberFormatter(impl::MacroProps &¯os, const Locale &locale); + /** * This is the core entrypoint to the number formatting pipeline. It performs self-regulation: a static code path * for the first few calls, and compiling a more efficient data structure if called repeatedly. @@ -2160,6 +2477,24 @@ class U_I18N_API FormattedNumber : public UMemory { #endif + // Don't allow copying of FormattedNumber, but moving is okay. + FormattedNumber(const FormattedNumber&) = delete; + FormattedNumber& operator=(const FormattedNumber&) = delete; + + /** + * Move constructor: + * Leaves the source FormattedNumber in an undefined state. + * @draft ICU 62 + */ + FormattedNumber(FormattedNumber&& src) U_NOEXCEPT; + + /** + * Move assignment: + * Leaves the source FormattedNumber in an undefined state. + * @draft ICU 62 + */ + FormattedNumber& operator=(FormattedNumber&& src) U_NOEXCEPT; + /** * Destruct an instance of FormattedNumber, cleaning up any memory it might own. * @draft ICU 60 diff --git a/icu4c/source/test/intltest/numbertest.h b/icu4c/source/test/intltest/numbertest.h index 3c2a62a805f..410d9ba316b 100644 --- a/icu4c/source/test/intltest/numbertest.h +++ b/icu4c/source/test/intltest/numbertest.h @@ -68,6 +68,7 @@ class NumberFormatterApiTest : public IntlTest { void formatTypes(); void errors(); void validRanges(); + void copyMove(); void runIndexedTest(int32_t index, UBool exec, const char *&name, char *par = 0); diff --git a/icu4c/source/test/intltest/numbertest_api.cpp b/icu4c/source/test/intltest/numbertest_api.cpp index 5b1c46f1e5e..e158e7a8c77 100644 --- a/icu4c/source/test/intltest/numbertest_api.cpp +++ b/icu4c/source/test/intltest/numbertest_api.cpp @@ -11,6 +11,7 @@ #include "unicode/numberformatter.h" #include "number_types.h" #include "numbertest.h" +#include "unicode/utypes.h" // Horrible workaround for the lack of a status code in the constructor... UErrorCode globalNumberFormatterApiTestStatus = U_ZERO_ERROR; @@ -77,6 +78,7 @@ void NumberFormatterApiTest::runIndexedTest(int32_t index, UBool exec, const cha TESTCASE_AUTO(formatTypes); TESTCASE_AUTO(errors); TESTCASE_AUTO(validRanges); + TESTCASE_AUTO(copyMove); TESTCASE_AUTO_END; } @@ -1840,6 +1842,80 @@ void NumberFormatterApiTest::validRanges() { VALID_RANGE_ONEARG(integerWidth, IntegerWidth::zeroFillTo(0).truncateAt, -1); } +void NumberFormatterApiTest::copyMove() { + IcuTestErrorCode status(*this, "copyMove"); + + // Default constructors + LocalizedNumberFormatter l1; + assertEquals("Initial behavior", u"10", l1.formatInt(10, status).toString()); + assertEquals("Initial call count", 1, l1.getCallCount()); + assertTrue("Initial compiled", l1.getCompiled() == nullptr); + + // Setup + l1 = NumberFormatter::withLocale("en").unit(NoUnit::percent()).threshold(3); + assertEquals("Initial behavior", u"10%", l1.formatInt(10, status).toString()); + assertEquals("Initial call count", 1, l1.getCallCount()); + assertTrue("Initial compiled", l1.getCompiled() == nullptr); + l1.formatInt(123, status); + assertEquals("Still not compiled", 2, l1.getCallCount()); + assertTrue("Still not compiled", l1.getCompiled() == nullptr); + l1.formatInt(123, status); + assertEquals("Compiled", u"10%", l1.formatInt(10, status).toString()); + assertEquals("Compiled", INT32_MIN, l1.getCallCount()); + assertTrue("Compiled", l1.getCompiled() != nullptr); + + // Copy constructor + LocalizedNumberFormatter l2 = l1; + assertEquals("[constructor] Copy behavior", u"10%", l2.formatInt(10, status).toString()); + assertEquals("[constructor] Copy should not have compiled state", 1, l2.getCallCount()); + assertTrue("[constructor] Copy should not have compiled state", l2.getCompiled() == nullptr); + + // Move constructor + LocalizedNumberFormatter l3 = std::move(l1); + assertEquals("[constructor] Move behavior", u"10%", l3.formatInt(10, status).toString()); + assertEquals("[constructor] Move *should* have compiled state", INT32_MIN, l3.getCallCount()); + assertTrue("[constructor] Move *should* have compiled state", l3.getCompiled() != nullptr); + assertEquals("[constructor] Source should be reset after move", 0, l1.getCallCount()); + assertTrue("[constructor] Source should be reset after move", l1.getCompiled() == nullptr); + + // Reset l1 and l2 to check for macro-props copying for behavior testing + l1 = NumberFormatter::withLocale("en"); + l2 = NumberFormatter::withLocale("en"); + + // Copy assignment + l1 = l3; + assertEquals("[assignment] Copy behavior", u"10%", l1.formatInt(10, status).toString()); + assertEquals("[assignment] Copy should not have compiled state", 1, l1.getCallCount()); + assertTrue("[assignment] Copy should not have compiled state", l1.getCompiled() == nullptr); + + // Move assignment + l2 = std::move(l3); + assertEquals("[assignment] Move behavior", u"10%", l2.formatInt(10, status).toString()); + assertEquals("[assignment] Move *should* have compiled state", INT32_MIN, l2.getCallCount()); + assertTrue("[assignment] Move *should* have compiled state", l2.getCompiled() != nullptr); + assertEquals("[assignment] Source should be reset after move", 0, l3.getCallCount()); + assertTrue("[assignment] Source should be reset after move", l3.getCompiled() == nullptr); + + // Coverage tests for UnlocalizedNumberFormatter + UnlocalizedNumberFormatter u1; + assertEquals("Default behavior", u"10", u1.locale("en").formatInt(10, status).toString()); + u1 = u1.unit(NoUnit::percent()); + assertEquals("Copy assignment", u"10%", u1.locale("en").formatInt(10, status).toString()); + UnlocalizedNumberFormatter u2 = u1; + assertEquals("Copy constructor", u"10%", u2.locale("en").formatInt(10, status).toString()); + UnlocalizedNumberFormatter u3 = std::move(u1); + assertEquals("Move constructor", u"10%", u3.locale("en").formatInt(10, status).toString()); + u1 = NumberFormatter::with(); + u1 = std::move(u2); + assertEquals("Move assignment", u"10%", u1.locale("en").formatInt(10, status).toString()); + + // FormattedNumber move operators + FormattedNumber result = l1.formatInt(10, status); + assertEquals("FormattedNumber move constructor", u"10%", result.toString()); + result = l1.formatInt(20, status); + assertEquals("FormattedNumber move assignment", u"20%", result.toString()); +} + void NumberFormatterApiTest::assertFormatDescending(const UnicodeString &message, const UnlocalizedNumberFormatter &f,