From 864682f5dd1b86c5524d1709b6c39af73d52e010 Mon Sep 17 00:00:00 2001 From: Hugo van der Merwe <17109322+hugovdm@users.noreply.github.com> Date: Sat, 6 Mar 2021 18:41:21 +0100 Subject: [PATCH] ICU-21349 Delete MeasureUnitImpl copy constructor, drop an indirection --- icu4c/source/i18n/measunit_extra.cpp | 8 ++---- icu4c/source/i18n/measunit_impl.h | 26 +++++++++++++------- icu4c/source/i18n/units_complexconverter.cpp | 26 ++++++++++---------- 3 files changed, 32 insertions(+), 28 deletions(-) diff --git a/icu4c/source/i18n/measunit_extra.cpp b/icu4c/source/i18n/measunit_extra.cpp index 676ff886eca..6c82ffbc8e0 100644 --- a/icu4c/source/i18n/measunit_extra.cpp +++ b/icu4c/source/i18n/measunit_extra.cpp @@ -955,10 +955,6 @@ int32_t SingleUnitImpl::getUnitCategoryIndex() const { return gSimpleUnitCategories[index]; } -MeasureUnitImpl::MeasureUnitImpl(const MeasureUnitImpl &other, UErrorCode &status) { - *this = other.copy(status); -} - MeasureUnitImpl::MeasureUnitImpl(const SingleUnitImpl &singleUnit, UErrorCode &status) { this->appendSingleUnit(singleUnit, status); } @@ -1040,12 +1036,12 @@ MeasureUnitImpl::extractIndividualUnitsWithIndices(UErrorCode &status) const { MaybeStackVector result; if (this->complexity != UMeasureUnitComplexity::UMEASURE_UNIT_MIXED) { - result.emplaceBackAndCheckErrorCode(status, 0, new MeasureUnitImpl(*this, status)); + result.emplaceBackAndCheckErrorCode(status, 0, *this, status); return result; } for (int32_t i = 0; i < singleUnits.length(); ++i) { - result.emplaceBackAndCheckErrorCode(status, i, new MeasureUnitImpl(*singleUnits[i], status)); + result.emplaceBackAndCheckErrorCode(status, i, *singleUnits[i], status); if (U_FAILURE(status)) { return result; } diff --git a/icu4c/source/i18n/measunit_impl.h b/icu4c/source/i18n/measunit_impl.h index 6c5a5acc039..f88d9000883 100644 --- a/icu4c/source/i18n/measunit_impl.h +++ b/icu4c/source/i18n/measunit_impl.h @@ -39,14 +39,6 @@ template class U_I18N_API LocalPointer; static const char16_t kDefaultCurrency[] = u"XXX"; static const char kDefaultCurrency8[] = "XXX"; -struct U_I18N_API MeasureUnitImplWithIndex : public UMemory { - const int32_t index; - LocalPointer unitImpl; - // Takes ownership of unitImpl. - MeasureUnitImplWithIndex(int32_t index, MeasureUnitImpl *unitImpl) - : index(index), unitImpl(unitImpl) {} -}; - /** * Looks up the "unitQuantity" (aka "type" or "category") of a base unit * identifier. The category is returned via `result`, which must initially be @@ -192,6 +184,9 @@ struct U_I18N_API SingleUnitImpl : public UMemory { int32_t dimensionality = 1; }; +// Forward declaration +struct MeasureUnitImplWithIndex; + // Export explicit template instantiations of MaybeStackArray, MemoryPool and // MaybeStackVector. This is required when building DLLs for Windows. (See // datefmt.h, collationiterator.h, erarules.h and others for similar examples.) @@ -212,7 +207,8 @@ class U_I18N_API MeasureUnitImpl : public UMemory { public: MeasureUnitImpl() = default; MeasureUnitImpl(MeasureUnitImpl &&other) = default; - MeasureUnitImpl(const MeasureUnitImpl &other, UErrorCode &status); + // No copy constructor, use MeasureUnitImpl::copy() to make it explicit. + MeasureUnitImpl(const MeasureUnitImpl &other, UErrorCode &status) = delete; MeasureUnitImpl(const SingleUnitImpl &singleUnit, UErrorCode &status); MeasureUnitImpl &operator=(MeasureUnitImpl &&other) noexcept = default; @@ -322,6 +318,18 @@ class U_I18N_API MeasureUnitImpl : public UMemory { friend class number::impl::LongNameHandler; }; +struct U_I18N_API MeasureUnitImplWithIndex : public UMemory { + const int32_t index; + MeasureUnitImpl unitImpl; + // Makes a copy of unitImpl. + MeasureUnitImplWithIndex(int32_t index, const MeasureUnitImpl &unitImpl, UErrorCode &status) + : index(index), unitImpl(unitImpl.copy(status)) { + } + MeasureUnitImplWithIndex(int32_t index, const SingleUnitImpl &singleUnitImpl, UErrorCode &status) + : index(index), unitImpl(MeasureUnitImpl(singleUnitImpl, status)) { + } +}; + U_NAMESPACE_END #endif /* #if !UCONFIG_NO_FORMATTING */ diff --git a/icu4c/source/i18n/units_complexconverter.cpp b/icu4c/source/i18n/units_complexconverter.cpp index 8ecb1b38531..89a60e4aa37 100644 --- a/icu4c/source/i18n/units_complexconverter.cpp +++ b/icu4c/source/i18n/units_complexconverter.cpp @@ -31,11 +31,11 @@ ComplexUnitsConverter::ComplexUnitsConverter(const MeasureUnitImpl &targetUnit, U_ASSERT(units_.length() != 0); // Just borrowing a pointer to the instance - MeasureUnitImpl *biggestUnit = units_[0]->unitImpl.getAlias(); + MeasureUnitImpl *biggestUnit = &units_[0]->unitImpl; for (int32_t i = 1; i < units_.length(); i++) { - if (UnitsConverter::compareTwoUnits(*units_[i]->unitImpl, *biggestUnit, ratesInfo, status) > 0 && + if (UnitsConverter::compareTwoUnits(units_[i]->unitImpl, *biggestUnit, ratesInfo, status) > 0 && U_SUCCESS(status)) { - biggestUnit = units_[i]->unitImpl.getAlias(); + biggestUnit = &units_[i]->unitImpl; } if (U_FAILURE(status)) { @@ -85,10 +85,10 @@ void ComplexUnitsConverter::init(const MeasureUnitImpl &inputUnit, const auto *rightPointer = static_cast(right); // Multiply by -1 to sort in descending order - return (-1) * UnitsConverter::compareTwoUnits(*((**leftPointer).unitImpl) /* left unit*/, // - *((**rightPointer).unitImpl) /* right unit */, // - *static_cast(context), // - status); + return (-1) * UnitsConverter::compareTwoUnits((**leftPointer).unitImpl, // + (**rightPointer).unitImpl, // + *static_cast(context), // + status); }; uprv_sortArray(units_.getAlias(), // @@ -116,11 +116,11 @@ void ComplexUnitsConverter::init(const MeasureUnitImpl &inputUnit, // 3. then, the final result will be (6 feet and 6.74016 inches) for (int i = 0, n = units_.length(); i < n; i++) { if (i == 0) { // first element - unitsConverters_.emplaceBackAndCheckErrorCode(status, inputUnit, *(units_[i]->unitImpl), - ratesInfo, status); + unitsConverters_.emplaceBackAndCheckErrorCode(status, inputUnit, units_[i]->unitImpl, + ratesInfo, status); } else { - unitsConverters_.emplaceBackAndCheckErrorCode(status, *(units_[i - 1]->unitImpl), - *(units_[i]->unitImpl), ratesInfo, status); + unitsConverters_.emplaceBackAndCheckErrorCode(status, units_[i - 1]->unitImpl, + units_[i]->unitImpl, ratesInfo, status); } if (U_FAILURE(status)) { @@ -200,12 +200,12 @@ MaybeStackVector ComplexUnitsConverter::convert(double quantity, if (i < n - 1) { Formattable formattableQuantity(intValues[i] * sign); // Measure takes ownership of the MeasureUnit* - MeasureUnit *type = new MeasureUnit(units_[i]->unitImpl->copy(status).build(status)); + MeasureUnit *type = new MeasureUnit(units_[i]->unitImpl.copy(status).build(status)); tmpResult[units_[i]->index] = new Measure(formattableQuantity, type, status); } else { // LAST ELEMENT Formattable formattableQuantity(quantity * sign); // Measure takes ownership of the MeasureUnit* - MeasureUnit *type = new MeasureUnit(units_[i]->unitImpl->copy(status).build(status)); + MeasureUnit *type = new MeasureUnit(units_[i]->unitImpl.copy(status).build(status)); tmpResult[units_[i]->index] = new Measure(formattableQuantity, type, status); } }