From 8fa27836959bbdaa06a5cf69a2153aa81fbcaede Mon Sep 17 00:00:00 2001 From: "Shane F. Carr" Date: Wed, 15 Jan 2020 21:05:27 +0100 Subject: [PATCH] Checkpoint --- icu4c/source/common/cmemory.h | 77 ++++++++++++++------ icu4c/source/common/unicode/localpointer.h | 13 ++++ icu4c/source/i18n/measunit_extra.cpp | 57 ++++++++++++--- icu4c/source/i18n/unicode/measunit.h | 5 ++ icu4c/source/test/intltest/measfmttest.cpp | 83 ++++++++++++++-------- 5 files changed, 175 insertions(+), 60 deletions(-) diff --git a/icu4c/source/common/cmemory.h b/icu4c/source/common/cmemory.h index 8f9be605396..da663918cfd 100644 --- a/icu4c/source/common/cmemory.h +++ b/icu4c/source/common/cmemory.h @@ -684,26 +684,26 @@ inline H *MaybeStackHeaderAndArray::orphanOrClone(int32_t l template class MemoryPool : public UMemory { public: - MemoryPool() : count(0), pool() {} + MemoryPool() : fCount(0), fPool() {} ~MemoryPool() { - for (int32_t i = 0; i < count; ++i) { - delete pool[i]; + for (int32_t i = 0; i < fCount; ++i) { + delete fPool[i]; } } MemoryPool(const MemoryPool&) = delete; MemoryPool& operator=(const MemoryPool&) = delete; - MemoryPool(MemoryPool&& other) U_NOEXCEPT : count(other.count), - pool(std::move(other.pool)) { - other.count = 0; + MemoryPool(MemoryPool&& other) U_NOEXCEPT : fCount(other.fCount), + fPool(std::move(other.fPool)) { + other.fCount = 0; } MemoryPool& operator=(MemoryPool&& other) U_NOEXCEPT { - count = other.count; - pool = std::move(other.pool); - other.count = 0; + fCount = other.fCount; + fPool = std::move(other.fPool); + other.fCount = 0; return *this; } @@ -716,20 +716,58 @@ public: */ template T* create(Args&&... args) { - int32_t capacity = pool.getCapacity(); - if (count == capacity && - pool.resize(capacity == stackCapacity ? 4 * capacity : 2 * capacity, - capacity) == nullptr) { + int32_t capacity = fPool.getCapacity(); + if (fCount == capacity && + fPool.resize(capacity == stackCapacity ? 4 * capacity : 2 * capacity, + capacity) == nullptr) { return nullptr; } - return pool[count++] = new T(std::forward(args)...); + return fPool[fCount++] = new T(std::forward(args)...); } /** * @return Number of elements that have been allocated. */ - int32_t size() const { - return count; + int32_t count() const { + return fCount; + } + +protected: + int32_t fCount; + MaybeStackArray fPool; +}; + +/** + * An internal Vector-like implementation based on MemoryPool. + * + * To append an item to the vector, use emplaceBack. + * + * MaybeStackVector vector; + * MyType* element = vector.emplaceBack(); + * if (!element) { + * status = U_MEMORY_ALLOCATION_ERROR; + * } + * // do stuff with element + * + * To loop over the vector, use a for loop with indices: + * + * for (int32_t i = 0; i < vector.length(); i++) { + * MyType* element = vector[i]; + * } + */ +template +class MaybeStackVector : protected MemoryPool { +public: + using MemoryPool::MemoryPool; + using MemoryPool::operator=; + + template + T* emplaceBack(Args&&... args) { + return this->create(args...); + } + + int32_t length() const { + return this->count(); } /** @@ -739,14 +777,11 @@ public: * @return reference to the array item */ T *operator[](ptrdiff_t i) const { - return pool[i]; + return this->fPool[i]; } - -private: - int32_t count; - MaybeStackArray pool; }; + U_NAMESPACE_END #endif /* __cplusplus */ diff --git a/icu4c/source/common/unicode/localpointer.h b/icu4c/source/common/unicode/localpointer.h index e011688b1a5..01f4205f99b 100644 --- a/icu4c/source/common/unicode/localpointer.h +++ b/icu4c/source/common/unicode/localpointer.h @@ -406,6 +406,10 @@ public: src.ptr=NULL; } + static LocalArray withLength(T *p, int32_t length) { + return LocalArray(p, length); + } + #ifndef U_HIDE_DRAFT_API /** * Constructs a LocalArray from a C++11 std::unique_ptr of an array type. @@ -537,6 +541,15 @@ public: return std::unique_ptr(LocalPointerBase::orphan()); } #endif /* U_HIDE_DRAFT_API */ + + int32_t length() const { return fLength; } + +private: + int32_t fLength = -1; + + LocalArray(T *p, int32_t length) : LocalArray(p) { + fLength = length; + } }; /** diff --git a/icu4c/source/i18n/measunit_extra.cpp b/icu4c/source/i18n/measunit_extra.cpp index eec204ed0e7..090cb803ac0 100644 --- a/icu4c/source/i18n/measunit_extra.cpp +++ b/icu4c/source/i18n/measunit_extra.cpp @@ -17,6 +17,7 @@ #include "ucln_in.h" #include "umutex.h" #include "unicode/errorcode.h" +#include "unicode/localpointer.h" #include "unicode/measunit.h" #include "unicode/ucharstrie.h" #include "unicode/ucharstriebuilder.h" @@ -356,6 +357,8 @@ struct PowerUnit { class CompoundUnit { public: + typedef MaybeStackVector PowerUnitList; + void append(PowerUnit&& powerUnit, UErrorCode& status) { if (powerUnit.power >= 0) { appendImpl(numerator, std::move(powerUnit), status); @@ -371,19 +374,26 @@ public: } void appendTo(CharString& builder, UErrorCode& status) { - if (numerator.size() == 0) { + if (numerator.length() == 0) { builder.append("one", status); } else { - appendToImpl(numerator, numerator.size(), builder, status); + appendToImpl(numerator, numerator.length(), builder, status); } - if (denominator.size() > 0) { + if (denominator.length() > 0) { builder.append("-per-", status); - appendToImpl(denominator, denominator.size(), builder, status); + appendToImpl(denominator, denominator.length(), builder, status); } } + const PowerUnitList& getNumeratorUnits() { + return numerator; + } + + const PowerUnitList& getDenominatorUnits() { + return denominator; + } + private: - typedef MemoryPool PowerUnitList; PowerUnitList numerator; PowerUnitList denominator; @@ -401,7 +411,7 @@ private: void appendImpl(PowerUnitList& unitList, PowerUnit&& powerUnit, UErrorCode& status) { // Check that the same simple unit doesn't already exist - for (int32_t i = 0; i < unitList.size(); i++) { + for (int32_t i = 0; i < unitList.length(); i++) { PowerUnit* candidate = unitList[i]; if (candidate->simpleUnitIndex == powerUnit.simpleUnitIndex && candidate->siPrefix == powerUnit.siPrefix) { @@ -410,7 +420,7 @@ private: } } // Add a new unit - PowerUnit* destination = unitList.create(); + PowerUnit* destination = unitList.emplaceBack(); if (!destination) { status = U_MEMORY_ALLOCATION_ERROR; return; @@ -555,6 +565,7 @@ private: goto fail; } fAfterPer = true; + result.power = -1; break; case COMPOUND_PART_TIMES: @@ -580,10 +591,7 @@ private: if (state > 0) { goto fail; } - result.power = token.getPower(); - if (fAfterPer) { - result.power *= -1; - } + result.power *= token.getPower(); previ = fIndex; state = 1; break; @@ -721,6 +729,33 @@ MeasureUnit MeasureUnit::product(const MeasureUnit& other, UErrorCode& status) c return MeasureUnit(builder.cloneData(status)); } +LocalArray MeasureUnit::getSimpleUnits(UErrorCode& status) const { + const char* id = getIdentifier(); + CompoundUnit compoundUnit = UnitIdentifierParser::from(id, status).getOnlyCompoundUnit(status); + if (U_FAILURE(status)) { + return LocalArray::withLength(nullptr, 0); + } + + const CompoundUnit::PowerUnitList& numerator = compoundUnit.getNumeratorUnits(); + const CompoundUnit::PowerUnitList& denominator = compoundUnit.getDenominatorUnits(); + int32_t count = numerator.length() + denominator.length(); + MeasureUnit* arr = new MeasureUnit[count]; + + CharString builder; + int32_t i = 0; + for (int32_t j = 0; j < numerator.length(); j++) { + numerator[j]->appendTo(builder.clear(), status); + arr[i++] = MeasureUnit(builder.cloneData(status)); + } + for (int32_t j = 0; j < denominator.length(); j++) { + builder.clear().append("one-per-", status); + denominator[j]->appendTo(builder, status); + arr[i++] = MeasureUnit(builder.cloneData(status)); + } + + return LocalArray::withLength(arr, count); +} + U_NAMESPACE_END diff --git a/icu4c/source/i18n/unicode/measunit.h b/icu4c/source/i18n/unicode/measunit.h index 8ed893190ab..c9947d3edff 100644 --- a/icu4c/source/i18n/unicode/measunit.h +++ b/icu4c/source/i18n/unicode/measunit.h @@ -20,6 +20,7 @@ #if !UCONFIG_NO_FORMATTING #include "unicode/unistr.h" +#include "unicode/localpointer.h" /** * \file @@ -377,6 +378,10 @@ class U_I18N_API MeasureUnit: public UObject { */ size_t getSimpleUnitCount(UErrorCode& status) const; + LocalArray getSimpleUnits(UErrorCode& status) const; + + LocalArray getSequenceUnits(UErrorCode& status) const; + /** * Gets the constituent unit at the given index. * diff --git a/icu4c/source/test/intltest/measfmttest.cpp b/icu4c/source/test/intltest/measfmttest.cpp index 01f118f4d36..5af03b77e64 100644 --- a/icu4c/source/test/intltest/measfmttest.cpp +++ b/icu4c/source/test/intltest/measfmttest.cpp @@ -139,14 +139,16 @@ private: NumberFormat::EAlignmentFields field, int32_t start, int32_t end); - void verifyUnitParts( + void verifyPowerUnit( const MeasureUnit& unit, UMeasureSIPrefix siPrefix, int8_t power, const char* identifier); - void verifyUnitIdentifierOnly( + void verifyCompoundUnit( const MeasureUnit& unit, - const char* identifier); + const char* identifier, + const char** subIdentifiers, + int32_t subIdentifierCount); }; void MeasureFormatTest::runIndexedTest( @@ -3237,11 +3239,11 @@ void MeasureFormatTest::TestCompoundUnitOperations() { MeasureUnit centimeter2 = meter.withSIPrefix(UMEASURE_SI_PREFIX_CENTI, status); MeasureUnit cubicDecimeter = cubicMeter.withSIPrefix(UMEASURE_SI_PREFIX_DECI, status); - verifyUnitParts(kilometer, UMEASURE_SI_PREFIX_KILO, 1, "kilometer"); - verifyUnitParts(meter, UMEASURE_SI_PREFIX_ONE, 1, "meter"); - verifyUnitParts(centimeter1, UMEASURE_SI_PREFIX_CENTI, 1, "centimeter"); - verifyUnitParts(centimeter2, UMEASURE_SI_PREFIX_CENTI, 1, "centimeter"); - verifyUnitParts(cubicDecimeter, UMEASURE_SI_PREFIX_DECI, 3, "cubic-decimeter"); + verifyPowerUnit(kilometer, UMEASURE_SI_PREFIX_KILO, 1, "kilometer"); + verifyPowerUnit(meter, UMEASURE_SI_PREFIX_ONE, 1, "meter"); + verifyPowerUnit(centimeter1, UMEASURE_SI_PREFIX_CENTI, 1, "centimeter"); + verifyPowerUnit(centimeter2, UMEASURE_SI_PREFIX_CENTI, 1, "centimeter"); + verifyPowerUnit(cubicDecimeter, UMEASURE_SI_PREFIX_DECI, 3, "cubic-decimeter"); assertTrue("centimeter equality", centimeter1 == centimeter2); assertTrue("kilometer inequality", centimeter1 != kilometer); @@ -3251,10 +3253,10 @@ void MeasureFormatTest::TestCompoundUnitOperations() { MeasureUnit quarticKilometer = kilometer.withPower(4, status); MeasureUnit overQuarticKilometer1 = kilometer.withPower(-4, status); - verifyUnitParts(squareMeter, UMEASURE_SI_PREFIX_ONE, 2, "square-meter"); - verifyUnitParts(overCubicCentimeter, UMEASURE_SI_PREFIX_CENTI, -3, "one-per-cubic-centimeter"); - verifyUnitParts(quarticKilometer, UMEASURE_SI_PREFIX_KILO, 4, "p4-kilometer"); - verifyUnitParts(overQuarticKilometer1, UMEASURE_SI_PREFIX_KILO, -4, "one-per-p4-kilometer"); + verifyPowerUnit(squareMeter, UMEASURE_SI_PREFIX_ONE, 2, "square-meter"); + verifyPowerUnit(overCubicCentimeter, UMEASURE_SI_PREFIX_CENTI, -3, "one-per-cubic-centimeter"); + verifyPowerUnit(quarticKilometer, UMEASURE_SI_PREFIX_KILO, 4, "p4-kilometer"); + verifyPowerUnit(overQuarticKilometer1, UMEASURE_SI_PREFIX_KILO, -4, "one-per-p4-kilometer"); assertTrue("power inequality", quarticKilometer != overQuarticKilometer1); @@ -3264,8 +3266,8 @@ void MeasureFormatTest::TestCompoundUnitOperations() { .product(kilometer, status) .reciprocal(status); - verifyUnitParts(overQuarticKilometer2, UMEASURE_SI_PREFIX_KILO, -4, "one-per-p4-kilometer"); - verifyUnitParts(overQuarticKilometer3, UMEASURE_SI_PREFIX_KILO, -4, "one-per-p4-kilometer"); + verifyPowerUnit(overQuarticKilometer2, UMEASURE_SI_PREFIX_KILO, -4, "one-per-p4-kilometer"); + verifyPowerUnit(overQuarticKilometer3, UMEASURE_SI_PREFIX_KILO, -4, "one-per-p4-kilometer"); assertTrue("reciprocal equality", overQuarticKilometer1 == overQuarticKilometer2); assertTrue("reciprocal equality", overQuarticKilometer1 == overQuarticKilometer3); @@ -3277,13 +3279,30 @@ void MeasureFormatTest::TestCompoundUnitOperations() { MeasureUnit centimeterSecond1 = meter.withSIPrefix(UMEASURE_SI_PREFIX_CENTI, status).product(kiloSquareSecond, status); MeasureUnit secondCubicMeter = kiloSquareSecond.product(meter.withPower(3, status), status); MeasureUnit secondCentimeter = kiloSquareSecond.product(meter.withSIPrefix(UMEASURE_SI_PREFIX_CENTI, status), status); + MeasureUnit secondCentimeterPerKilometer = secondCentimeter.product(kilometer.reciprocal(status), status); + + verifyPowerUnit(kiloSquareSecond, UMEASURE_SI_PREFIX_KILO, 2, "square-kilosecond"); + const char* meterSecondSub[] = {"meter", "square-kilosecond"}; + verifyCompoundUnit(meterSecond, "meter-square-kilosecond", + meterSecondSub, UPRV_LENGTHOF(meterSecondSub)); + const char* cubicMeterSecond1Sub[] = {"cubic-meter", "square-kilosecond"}; + verifyCompoundUnit(cubicMeterSecond1, "cubic-meter-square-kilosecond", + cubicMeterSecond1Sub, UPRV_LENGTHOF(cubicMeterSecond1Sub)); + const char* centimeterSecond1Sub[] = {"centimeter", "square-kilosecond"}; + verifyCompoundUnit(centimeterSecond1, "centimeter-square-kilosecond", + centimeterSecond1Sub, UPRV_LENGTHOF(centimeterSecond1Sub)); + const char* secondCubicMeterSub[] = {"square-kilosecond", "cubic-meter"}; + verifyCompoundUnit(secondCubicMeter, "square-kilosecond-cubic-meter", + secondCubicMeterSub, UPRV_LENGTHOF(secondCubicMeterSub)); + const char* secondCentimeterSub[] = {"square-kilosecond", "centimeter"}; + verifyCompoundUnit(secondCentimeter, "square-kilosecond-centimeter", + secondCentimeterSub, UPRV_LENGTHOF(secondCentimeterSub)); + const char* secondCentimeterPerKilometerSub[] = {"square-kilosecond", "centimeter", "one-per-kilometer"}; + verifyCompoundUnit(secondCentimeterPerKilometer, "square-kilosecond-centimeter-per-kilometer", + secondCentimeterPerKilometerSub, UPRV_LENGTHOF(secondCentimeterPerKilometerSub)); - verifyUnitParts(kiloSquareSecond, UMEASURE_SI_PREFIX_KILO, 2, "square-kilosecond"); - verifyUnitIdentifierOnly(meterSecond, "meter-square-kilosecond"); - verifyUnitIdentifierOnly(cubicMeterSecond1, "cubic-meter-square-kilosecond"); - verifyUnitIdentifierOnly(centimeterSecond1, "centimeter-square-kilosecond"); - verifyUnitIdentifierOnly(secondCubicMeter, "square-kilosecond-cubic-meter"); - verifyUnitIdentifierOnly(secondCentimeter, "square-kilosecond-centimeter"); + assertTrue("order matters inequality", cubicMeterSecond1 != secondCubicMeter); + assertTrue("additional simple units inequality", secondCubicMeter != secondCentimeter); // Don't allow get/set power or SI prefix on compound units status.errIfFailureAndReset(); @@ -3295,9 +3314,6 @@ void MeasureFormatTest::TestCompoundUnitOperations() { status.expectErrorAndReset(U_ILLEGAL_ARGUMENT_ERROR); meterSecond.withSIPrefix(UMEASURE_SI_PREFIX_CENTI, status); status.expectErrorAndReset(U_ILLEGAL_ARGUMENT_ERROR); - - assertTrue("order matters inequality", cubicMeterSecond1 != secondCubicMeter); - assertTrue("additional simple units inequality", secondCubicMeter != secondCentimeter); } @@ -3371,12 +3387,12 @@ void MeasureFormatTest::verifyFormat( } } -void MeasureFormatTest::verifyUnitParts( +void MeasureFormatTest::verifyPowerUnit( const MeasureUnit& unit, UMeasureSIPrefix siPrefix, int8_t power, const char* identifier) { - IcuTestErrorCode status(*this, "verifyUnitParts"); + IcuTestErrorCode status(*this, "verifyPowerUnit"); UnicodeString uid(identifier, -1, US_INV); assertEquals(uid + ": SI prefix", siPrefix, @@ -3395,10 +3411,12 @@ void MeasureFormatTest::verifyUnitParts( status.errIfFailureAndReset("%s: Constructor", identifier); } -void MeasureFormatTest::verifyUnitIdentifierOnly( +void MeasureFormatTest::verifyCompoundUnit( const MeasureUnit& unit, - const char* identifier) { - IcuTestErrorCode status(*this, "verifyUnitIdentifierOnly"); + const char* identifier, + const char** subIdentifiers, + int32_t subIdentifierCount) { + IcuTestErrorCode status(*this, "verifyCompoundUnit"); UnicodeString uid(identifier, -1, US_INV); assertEquals(uid + ": Identifier", identifier, @@ -3407,6 +3425,15 @@ void MeasureFormatTest::verifyUnitIdentifierOnly( assertTrue(uid + ": Constructor", unit == MeasureUnit::forIdentifier(identifier, status)); status.errIfFailureAndReset("%s: Constructor", identifier); + + LocalArray subUnits = unit.getSimpleUnits(status); + assertEquals(uid + ": Length", subIdentifierCount, subUnits.length()); + for (int32_t i = 0;; i++) { + if (i >= subIdentifierCount || i >= subUnits.length()) break; + assertEquals(uid + ": Sub-unit #" + Int64ToUnicodeString(i), + subIdentifiers[i], + subUnits[i].getIdentifier()); + } } extern IntlTest *createMeasureFormatTest() { -- 2.40.0