From c07264a363dbdbaa28ecc15633ef05764b1d2afe Mon Sep 17 00:00:00 2001 From: younies Date: Mon, 30 Nov 2020 18:37:37 +0000 Subject: [PATCH] ICU-21349 Make appendSingleUnit behaviour in C++ comply with Java See #1486 --- icu4c/source/i18n/measunit.cpp | 4 +- icu4c/source/i18n/measunit_extra.cpp | 253 +++++++++--------- icu4c/source/i18n/measunit_impl.h | 6 +- icu4c/source/i18n/number_formatimpl.cpp | 4 +- icu4c/source/i18n/number_longnames.cpp | 12 +- icu4c/source/i18n/number_usageprefs.cpp | 4 +- icu4c/source/i18n/units_converter.cpp | 18 +- icu4c/source/test/intltest/measfmttest.cpp | 47 +++- .../ibm/icu/impl/units/MeasureUnitImpl.java | 10 +- 9 files changed, 193 insertions(+), 165 deletions(-) diff --git a/icu4c/source/i18n/measunit.cpp b/icu4c/source/i18n/measunit.cpp index c3e39b9c660..5e55fde4ca1 100644 --- a/icu4c/source/i18n/measunit.cpp +++ b/icu4c/source/i18n/measunit.cpp @@ -2321,8 +2321,8 @@ MeasureUnitImpl MeasureUnitImpl::copy(UErrorCode &status) const { MeasureUnitImpl result; result.complexity = complexity; result.identifier.append(identifier, status); - for (int32_t i = 0; i < units.length(); i++) { - SingleUnitImpl *item = result.units.emplaceBack(*units[i]); + for (int32_t i = 0; i < singleUnits.length(); i++) { + SingleUnitImpl *item = result.singleUnits.emplaceBack(*singleUnits[i]); if (!item) { status = U_MEMORY_ALLOCATION_ERROR; return result; diff --git a/icu4c/source/i18n/measunit_extra.cpp b/icu4c/source/i18n/measunit_extra.cpp index 2eb3f066142..1edc77212fa 100644 --- a/icu4c/source/i18n/measunit_extra.cpp +++ b/icu4c/source/i18n/measunit_extra.cpp @@ -380,7 +380,53 @@ public: MeasureUnitImpl parse(UErrorCode& status) { MeasureUnitImpl result; - parseImpl(result, status); + + if (U_FAILURE(status)) { + return result; + } + if (fSource.empty()) { + // The dimenionless unit: nothing to parse. leave result as is. + return result; + } + + while (hasNext()) { + bool sawAnd = false; + + SingleUnitImpl singleUnit = nextSingleUnit(sawAnd, status); + if (U_FAILURE(status)) { + return result; + } + + bool added = result.appendSingleUnit(singleUnit, status); + if (U_FAILURE(status)) { + return result; + } + + if (sawAnd && !added) { + // Two similar units are not allowed in a mixed unit. + status = kUnitIdentifierSyntaxError; + return result; + } + + if (result.singleUnits.length() >= 2) { + // nextSingleUnit fails appropriately for "per" and "and" in the + // same identifier. It doesn't fail for other compound units + // (COMPOUND_PART_TIMES). Consequently we take care of that + // here. + UMeasureUnitComplexity complexity = + sawAnd ? UMEASURE_UNIT_MIXED : UMEASURE_UNIT_COMPOUND; + if (result.singleUnits.length() == 2) { + // After appending two singleUnits, the complexity will be `UMEASURE_UNIT_COMPOUND` + U_ASSERT(result.complexity == UMEASURE_UNIT_COMPOUND); + result.complexity = complexity; + } else if (result.complexity != complexity) { + // Can't have mixed compound units + status = kUnitIdentifierSyntaxError; + return result; + } + } + } + return result; } @@ -457,9 +503,10 @@ private: * unit", sawAnd is set to true. If not, it is left as is. * @param status ICU error code. */ - void nextSingleUnit(SingleUnitImpl& result, bool& sawAnd, UErrorCode& status) { + SingleUnitImpl nextSingleUnit(bool &sawAnd, UErrorCode &status) { + SingleUnitImpl result; if (U_FAILURE(status)) { - return; + return result; } // state: @@ -470,7 +517,9 @@ private: bool atStart = fIndex == 0; Token token = nextToken(status); - if (U_FAILURE(status)) { return; } + if (U_FAILURE(status)) { + return result; + } if (atStart) { // Identifiers optionally start with "per-". @@ -480,14 +529,16 @@ private: result.dimensionality = -1; token = nextToken(status); - if (U_FAILURE(status)) { return; } + if (U_FAILURE(status)) { + return result; + } } } else { // All other SingleUnit's are separated from previous SingleUnit's // via a compound part: if (token.getType() != Token::TYPE_COMPOUND_PART) { status = kUnitIdentifierSyntaxError; - return; + return result; } switch (token.getMatch()) { @@ -496,7 +547,7 @@ private: // Mixed compound units not yet supported, // TODO(CLDR-13700). status = kUnitIdentifierSyntaxError; - return; + return result; } fAfterPer = true; result.dimensionality = -1; @@ -513,14 +564,16 @@ private: // Can't start with "-and-", and mixed compound units // not yet supported, TODO(CLDR-13700). status = kUnitIdentifierSyntaxError; - return; + return result; } sawAnd = true; break; } token = nextToken(status); - if (U_FAILURE(status)) { return; } + if (U_FAILURE(status)) { + return result; + } } // Read tokens until we have a complete SingleUnit or we reach the end. @@ -529,7 +582,7 @@ private: case Token::TYPE_POWER_PART: if (state > 0) { status = kUnitIdentifierSyntaxError; - return; + return result; } result.dimensionality *= token.getPower(); state = 1; @@ -538,7 +591,7 @@ private: case Token::TYPE_SI_PREFIX: if (state > 1) { status = kUnitIdentifierSyntaxError; - return; + return result; } result.siPrefix = token.getSIPrefix(); state = 2; @@ -546,67 +599,25 @@ private: case Token::TYPE_SIMPLE_UNIT: result.index = token.getSimpleUnitIndex(); - return; + return result; default: status = kUnitIdentifierSyntaxError; - return; + return result; } if (!hasNext()) { // We ran out of tokens before finding a complete single unit. status = kUnitIdentifierSyntaxError; - return; + return result; } token = nextToken(status); if (U_FAILURE(status)) { - return; + return result; } } - } - /// @param result is modified, not overridden. Caller must pass in a - /// default-constructed (empty) MeasureUnitImpl instance. - void parseImpl(MeasureUnitImpl& result, UErrorCode& status) { - if (U_FAILURE(status)) { - return; - } - if (fSource.empty()) { - // The dimenionless unit: nothing to parse. leave result as is. - return; - } - int32_t unitNum = 0; - while (hasNext()) { - bool sawAnd = false; - SingleUnitImpl singleUnit; - nextSingleUnit(singleUnit, sawAnd, status); - if (U_FAILURE(status)) { - return; - } - U_ASSERT(!singleUnit.isDimensionless()); - bool added = result.append(singleUnit, status); - if (sawAnd && !added) { - // Two similar units are not allowed in a mixed unit - status = kUnitIdentifierSyntaxError; - return; - } - if ((++unitNum) >= 2) { - // nextSingleUnit fails appropriately for "per" and "and" in the - // same identifier. It doesn't fail for other compound units - // (COMPOUND_PART_TIMES). Consequently we take care of that - // here. - UMeasureUnitComplexity complexity = - sawAnd ? UMEASURE_UNIT_MIXED : UMEASURE_UNIT_COMPOUND; - if (unitNum == 2) { - U_ASSERT(result.complexity == UMEASURE_UNIT_SINGLE); - result.complexity = complexity; - } else if (result.complexity != complexity) { - // Can't have mixed compound units - status = kUnitIdentifierSyntaxError; - return; - } - } - } + return result; } }; @@ -684,32 +695,26 @@ void serialize(MeasureUnitImpl& impl, UErrorCode& status) { return; } U_ASSERT(impl.identifier.isEmpty()); - if (impl.units.length() == 0) { + if (impl.singleUnits.length() == 0) { // Dimensionless, constructed by the default constructor: no appending // to impl.identifier, we wish it to contain the zero-length string. return; } if (impl.complexity == UMEASURE_UNIT_COMPOUND) { // Note: don't sort a MIXED unit - uprv_sortArray( - impl.units.getAlias(), - impl.units.length(), - sizeof(impl.units[0]), - compareSingleUnits, - nullptr, - false, - &status); + uprv_sortArray(impl.singleUnits.getAlias(), impl.singleUnits.length(), + sizeof(impl.singleUnits[0]), compareSingleUnits, nullptr, false, &status); if (U_FAILURE(status)) { return; } } - serializeSingle(*impl.units[0], true, impl.identifier, status); - if (impl.units.length() == 1) { + serializeSingle(*impl.singleUnits[0], true, impl.identifier, status); + if (impl.singleUnits.length() == 1) { return; } - for (int32_t i = 1; i < impl.units.length(); i++) { - const SingleUnitImpl& prev = *impl.units[i-1]; - const SingleUnitImpl& curr = *impl.units[i]; + for (int32_t i = 1; i < impl.singleUnits.length(); i++) { + const SingleUnitImpl &prev = *impl.singleUnits[i - 1]; + const SingleUnitImpl &curr = *impl.singleUnits[i]; if (impl.complexity == UMEASURE_UNIT_MIXED) { impl.identifier.append("-and-", status); serializeSingle(curr, true, impl.identifier, status); @@ -722,41 +727,6 @@ void serialize(MeasureUnitImpl& impl, UErrorCode& status) { serializeSingle(curr, false, impl.identifier, status); } } - -} - -/** - * Appends a SingleUnitImpl to a MeasureUnitImpl. - * - * @return true if a new item was added. If unit is the dimensionless unit, it - * is never added: the return value will always be false. - */ -bool appendImpl(MeasureUnitImpl& impl, const SingleUnitImpl& unit, UErrorCode& status) { - if (unit.isDimensionless()) { - // We don't append dimensionless units. - return false; - } - // Find a similar unit that already exists, to attempt to coalesce - SingleUnitImpl* oldUnit = nullptr; - for (int32_t i = 0; i < impl.units.length(); i++) { - auto* candidate = impl.units[i]; - if (candidate->isCompatibleWith(unit)) { - oldUnit = candidate; - } - } - if (oldUnit) { - // Both dimensionalities will be positive, or both will be negative, by - // virtue of isCompatibleWith(). - oldUnit->dimensionality += unit.dimensionality; - } else { - SingleUnitImpl* destination = impl.units.emplaceBack(); - if (!destination) { - status = U_MEMORY_ALLOCATION_ERROR; - return false; - } - *destination = unit; - } - return (oldUnit == nullptr); } } // namespace @@ -768,11 +738,11 @@ SingleUnitImpl SingleUnitImpl::forMeasureUnit(const MeasureUnit& measureUnit, UE if (U_FAILURE(status)) { return {}; } - if (impl.units.length() == 0) { + if (impl.singleUnits.length() == 0) { return {}; } - if (impl.units.length() == 1) { - return *impl.units[0]; + if (impl.singleUnits.length() == 1) { + return *impl.singleUnits[0]; } status = U_ILLEGAL_ARGUMENT_ERROR; return {}; @@ -780,7 +750,7 @@ SingleUnitImpl SingleUnitImpl::forMeasureUnit(const MeasureUnit& measureUnit, UE MeasureUnit SingleUnitImpl::build(UErrorCode& status) const { MeasureUnitImpl temp; - temp.append(*this, status); + temp.appendSingleUnit(*this, status); return std::move(temp).build(status); } @@ -793,7 +763,7 @@ MeasureUnitImpl::MeasureUnitImpl(const MeasureUnitImpl &other, UErrorCode &statu } MeasureUnitImpl::MeasureUnitImpl(const SingleUnitImpl &singleUnit, UErrorCode &status) { - this->append(singleUnit, status); + this->appendSingleUnit(singleUnit, status); } MeasureUnitImpl MeasureUnitImpl::forIdentifier(StringPiece identifier, UErrorCode& status) { @@ -821,14 +791,51 @@ MeasureUnitImpl MeasureUnitImpl::forMeasureUnitMaybeCopy( void MeasureUnitImpl::takeReciprocal(UErrorCode& /*status*/) { identifier.clear(); - for (int32_t i = 0; i < units.length(); i++) { - units[i]->dimensionality *= -1; + for (int32_t i = 0; i < singleUnits.length(); i++) { + singleUnits[i]->dimensionality *= -1; } } -bool MeasureUnitImpl::append(const SingleUnitImpl& singleUnit, UErrorCode& status) { +bool MeasureUnitImpl::appendSingleUnit(const SingleUnitImpl &singleUnit, UErrorCode &status) { identifier.clear(); - return appendImpl(*this, singleUnit, status); + + if (singleUnit.isDimensionless()) { + // Do not append dimensionless units. + return false; + } + + // Find a similar unit that already exists, to attempt to coalesce + SingleUnitImpl *oldUnit = nullptr; + for (int32_t i = 0; i < this->singleUnits.length(); i++) { + auto *candidate = this->singleUnits[i]; + if (candidate->isCompatibleWith(singleUnit)) { + oldUnit = candidate; + } + } + + if (oldUnit) { + // Both dimensionalities will be positive, or both will be negative, by + // virtue of isCompatibleWith(). + oldUnit->dimensionality += singleUnit.dimensionality; + + return false; + } + + // Add a copy of singleUnit + // NOTE: MaybeStackVector::emplaceBackAndCheckErrorCode creates new copy of singleUnit. + this->singleUnits.emplaceBackAndCheckErrorCode(status, singleUnit); + if (U_FAILURE(status)) { + return false; + } + + // If the MeasureUnitImpl is `UMEASURE_UNIT_SINGLE` and after the appending a unit, the `singleUnits` + // contains more than one. thus means the complexity should be `UMEASURE_UNIT_COMPOUND` + if (this->singleUnits.length() > 1 && + this->complexity == UMeasureUnitComplexity::UMEASURE_UNIT_SINGLE) { + this->complexity = UMeasureUnitComplexity::UMEASURE_UNIT_COMPOUND; + } + + return true; } MaybeStackVector MeasureUnitImpl::extractIndividualUnits(UErrorCode &status) const { @@ -839,8 +846,8 @@ MaybeStackVector MeasureUnitImpl::extractIndividualUnits(UError return result; } - for (int32_t i = 0; i < units.length(); i++) { - result.emplaceBackAndCheckErrorCode(status, *units[i], status); + for (int32_t i = 0; i < singleUnits.length(); i++) { + result.emplaceBackAndCheckErrorCode(status, *singleUnits[i], status); } return result; @@ -899,10 +906,10 @@ MeasureUnit MeasureUnit::product(const MeasureUnit& other, UErrorCode& status) c status = U_ILLEGAL_ARGUMENT_ERROR; return {}; } - for (int32_t i = 0; i < otherImpl.units.length(); i++) { - impl.append(*otherImpl.units[i], status); + for (int32_t i = 0; i < otherImpl.singleUnits.length(); i++) { + impl.appendSingleUnit(*otherImpl.singleUnits[i], status); } - if (impl.units.length() > 1) { + if (impl.singleUnits.length() > 1) { impl.complexity = UMEASURE_UNIT_COMPOUND; } return std::move(impl).build(status); @@ -911,14 +918,14 @@ MeasureUnit MeasureUnit::product(const MeasureUnit& other, UErrorCode& status) c LocalArray MeasureUnit::splitToSingleUnitsImpl(int32_t& outCount, UErrorCode& status) const { MeasureUnitImpl temp; const MeasureUnitImpl& impl = MeasureUnitImpl::forMeasureUnit(*this, temp, status); - outCount = impl.units.length(); + outCount = impl.singleUnits.length(); MeasureUnit* arr = new MeasureUnit[outCount]; if (arr == nullptr) { status = U_MEMORY_ALLOCATION_ERROR; return LocalArray(); } for (int32_t i = 0; i < outCount; i++) { - arr[i] = impl.units[i]->build(status); + arr[i] = impl.singleUnits[i]->build(status); } return LocalArray(arr, status); } diff --git a/icu4c/source/i18n/measunit_impl.h b/icu4c/source/i18n/measunit_impl.h index 3cc2cd0476f..a4f97cffe73 100644 --- a/icu4c/source/i18n/measunit_impl.h +++ b/icu4c/source/i18n/measunit_impl.h @@ -215,19 +215,19 @@ struct U_I18N_API MeasureUnitImpl : public UMemory { * @return true if a new item was added. If unit is the dimensionless unit, * it is never added: the return value will always be false. */ - bool append(const SingleUnitImpl& singleUnit, UErrorCode& status); + bool appendSingleUnit(const SingleUnitImpl& singleUnit, UErrorCode& status); /** The complexity, either SINGLE, COMPOUND, or MIXED. */ UMeasureUnitComplexity complexity = UMEASURE_UNIT_SINGLE; /** - * The list of simple units. These may be summed or multiplied, based on the + * The list of single units. These may be summed or multiplied, based on the * value of the complexity field. * * The "dimensionless" unit (SingleUnitImpl default constructor) must not be * added to this list. */ - MaybeStackVector units; + MaybeStackVector singleUnits; /** * The full unit identifier. Owned by the MeasureUnitImpl. Empty if not computed. diff --git a/icu4c/source/i18n/number_formatimpl.cpp b/icu4c/source/i18n/number_formatimpl.cpp index f7e1fcbb4e6..14091314769 100644 --- a/icu4c/source/i18n/number_formatimpl.cpp +++ b/icu4c/source/i18n/number_formatimpl.cpp @@ -252,8 +252,8 @@ NumberFormatterImpl::macrosToMicroGenerator(const MacroProps& macros, bool safe, } else if (isMixedUnit) { MeasureUnitImpl temp; const MeasureUnitImpl &outputUnit = MeasureUnitImpl::forMeasureUnit(macros.unit, temp, status); - auto unitConversionHandler = - new UnitConversionHandler(outputUnit.units[0]->build(status), macros.unit, chain, status); + auto unitConversionHandler = new UnitConversionHandler(outputUnit.singleUnits[0]->build(status), + macros.unit, chain, status); fUnitConversionHandler.adoptInsteadAndCheckErrorCode(unitConversionHandler, status); chain = fUnitConversionHandler.getAlias(); } diff --git a/icu4c/source/i18n/number_longnames.cpp b/icu4c/source/i18n/number_longnames.cpp index 0fa56c70c15..bf89a7a0b9e 100644 --- a/icu4c/source/i18n/number_longnames.cpp +++ b/icu4c/source/i18n/number_longnames.cpp @@ -234,13 +234,13 @@ void LongNameHandler::forMeasureUnit(const Locale &loc, const MeasureUnit &unitR MeasureUnitImpl fullUnit = MeasureUnitImpl::forMeasureUnitMaybeCopy(unitRef, status); MeasureUnitImpl unit; MeasureUnitImpl perUnit; - for (int32_t i = 0; i < fullUnit.units.length(); i++) { - SingleUnitImpl *subUnit = fullUnit.units[i]; + for (int32_t i = 0; i < fullUnit.singleUnits.length(); i++) { + SingleUnitImpl *subUnit = fullUnit.singleUnits[i]; if (subUnit->dimensionality > 0) { - unit.append(*subUnit, status); + unit.appendSingleUnit(*subUnit, status); } else { subUnit->dimensionality *= -1; - perUnit.append(*subUnit, status); + perUnit.appendSingleUnit(*subUnit, status); } } forCompoundUnit(loc, std::move(unit).build(status), std::move(perUnit).build(status), width, @@ -420,12 +420,12 @@ void MixedUnitLongNameHandler::forMeasureUnit(const Locale &loc, const MeasureUn MeasureUnitImpl temp; const MeasureUnitImpl& impl = MeasureUnitImpl::forMeasureUnit(mixedUnit, temp, status); - fillIn->fMixedUnitCount = impl.units.length(); + fillIn->fMixedUnitCount = impl.singleUnits.length(); fillIn->fMixedUnitData.adoptInstead(new UnicodeString[fillIn->fMixedUnitCount * ARRAY_LENGTH]); for (int32_t i = 0; i < fillIn->fMixedUnitCount; i++) { // Grab data for each of the components. UnicodeString *unitData = &fillIn->fMixedUnitData[i * ARRAY_LENGTH]; - getMeasureData(loc, impl.units[i]->build(status), width, unitData, status); + getMeasureData(loc, impl.singleUnits[i]->build(status), width, unitData, status); } UListFormatterWidth listWidth = ULISTFMT_WIDTH_SHORT; diff --git a/icu4c/source/i18n/number_usageprefs.cpp b/icu4c/source/i18n/number_usageprefs.cpp index 0d9cb06c50a..ea684bad870 100644 --- a/icu4c/source/i18n/number_usageprefs.cpp +++ b/icu4c/source/i18n/number_usageprefs.cpp @@ -116,9 +116,9 @@ void mixedMeasuresToMicros(const MaybeStackVector &measures, DecimalQua MeasureUnitImpl temp; const MeasureUnitImpl& impl = MeasureUnitImpl::forMeasureUnit(micros->outputUnit, temp, status); U_ASSERT(U_SUCCESS(status)); - U_ASSERT(measures.length() == impl.units.length()); + U_ASSERT(measures.length() == impl.singleUnits.length()); for (int32_t i = 0; i < measures.length(); i++) { - U_ASSERT(measures[i]->getUnit() == impl.units[i]->build(status)); + U_ASSERT(measures[i]->getUnit() == impl.singleUnits[i]->build(status)); } (void)impl; #endif diff --git a/icu4c/source/i18n/units_converter.cpp b/icu4c/source/i18n/units_converter.cpp index bed40dd4e13..085789abc30 100644 --- a/icu4c/source/i18n/units_converter.cpp +++ b/icu4c/source/i18n/units_converter.cpp @@ -217,8 +217,8 @@ Factor loadCompoundFactor(const MeasureUnitImpl &source, const ConversionRates & UErrorCode &status) { Factor result; - for (int32_t i = 0, n = source.units.length(); i < n; i++) { - SingleUnitImpl singleUnit = *source.units[i]; + for (int32_t i = 0, n = source.singleUnits.length(); i < n; i++) { + SingleUnitImpl singleUnit = *source.singleUnits[i]; Factor singleFactor = loadSingleFactor(singleUnit.getSimpleUnitID(), ratesInfo, status); if (U_FAILURE(status)) return result; @@ -248,12 +248,12 @@ UBool checkSimpleUnit(const MeasureUnitImpl &unit, UErrorCode &status) { if (unit.complexity != UMEASURE_UNIT_SINGLE) { return false; } - if (unit.units.length() == 0) { + if (unit.singleUnits.length() == 0) { // Empty units means simple unit. return true; } - auto singleUnit = *(unit.units[0]); + auto singleUnit = *(unit.singleUnits[0]); if (singleUnit.dimensionality != 1 || singleUnit.siPrefix != UMEASURE_SI_PREFIX_ONE) { return false; @@ -329,8 +329,8 @@ void mergeSingleUnitWithDimension(MaybeStackVector &unitI void mergeUnitsAndDimensions(MaybeStackVector &unitIndicesWithDimension, const MeasureUnitImpl &shouldBeMerged, int32_t multiplier) { - for (int32_t unit_i = 0; unit_i < shouldBeMerged.units.length(); unit_i++) { - auto singleUnit = *shouldBeMerged.units[unit_i]; + for (int32_t unit_i = 0; unit_i < shouldBeMerged.singleUnits.length(); unit_i++) { + auto singleUnit = *shouldBeMerged.singleUnits[unit_i]; mergeSingleUnitWithDimension(unitIndicesWithDimension, singleUnit, multiplier); } } @@ -396,7 +396,7 @@ MeasureUnitImpl U_I18N_API extractCompoundBaseUnit(const MeasureUnitImpl &source MeasureUnitImpl result; if (U_FAILURE(status)) return result; - const auto &singleUnits = source.units; + const auto &singleUnits = source.singleUnits; for (int i = 0, count = singleUnits.length(); i < count; ++i) { const auto &singleUnit = *singleUnits[i]; // Extract `ConversionRateInfo` using the absolute unit. For example: in case of `square-meter`, @@ -414,11 +414,11 @@ MeasureUnitImpl U_I18N_API extractCompoundBaseUnit(const MeasureUnitImpl &source // Multiply the power of the singleUnit by the power of the baseUnit. For example, square-hectare // must be pow4-meter. (NOTE: hectare --> square-meter) auto baseUnits = - MeasureUnitImpl::forIdentifier(rateInfo->baseUnit.toStringPiece(), status).units; + MeasureUnitImpl::forIdentifier(rateInfo->baseUnit.toStringPiece(), status).singleUnits; for (int32_t i = 0, baseUnitsCount = baseUnits.length(); i < baseUnitsCount; i++) { baseUnits[i]->dimensionality *= singleUnit.dimensionality; // TODO: Deal with SI-prefix - result.append(*baseUnits[i], status); + result.appendSingleUnit(*baseUnits[i], status); if (U_FAILURE(status)) { return result; diff --git a/icu4c/source/test/intltest/measfmttest.cpp b/icu4c/source/test/intltest/measfmttest.cpp index 7d5330f6657..3f95358fb00 100644 --- a/icu4c/source/test/intltest/measfmttest.cpp +++ b/icu4c/source/test/intltest/measfmttest.cpp @@ -4046,8 +4046,8 @@ void MeasureFormatTest::TestInternalMeasureUnitImpl() { status.assertSuccess(); assertEquals("mu1 initial identifier", "", mu1.identifier.data()); assertEquals("mu1 initial complexity", UMEASURE_UNIT_SINGLE, mu1.complexity); - assertEquals("mu1 initial units length", 1, mu1.units.length()); - assertEquals("mu1 initial units[0]", "meter", mu1.units[0]->getSimpleUnitID()); + assertEquals("mu1 initial units length", 1, mu1.singleUnits.length()); + assertEquals("mu1 initial units[0]", "meter", mu1.singleUnits[0]->getSimpleUnitID()); // Producing identifier via build(): the std::move() means mu1 gets modified // while it also gets assigned to tmp's internal fImpl. @@ -4055,8 +4055,8 @@ void MeasureFormatTest::TestInternalMeasureUnitImpl() { status.assertSuccess(); assertEquals("mu1 post-move-build identifier", "meter", mu1.identifier.data()); assertEquals("mu1 post-move-build complexity", UMEASURE_UNIT_SINGLE, mu1.complexity); - assertEquals("mu1 post-move-build units length", 1, mu1.units.length()); - assertEquals("mu1 post-move-build units[0]", "meter", mu1.units[0]->getSimpleUnitID()); + assertEquals("mu1 post-move-build units length", 1, mu1.singleUnits.length()); + assertEquals("mu1 post-move-build units[0]", "meter", mu1.singleUnits[0]->getSimpleUnitID()); assertEquals("MeasureUnit tmp identifier", "meter", tmp.getIdentifier()); // This temporary variable is used when forMeasureUnit's first parameter @@ -4066,8 +4066,8 @@ void MeasureFormatTest::TestInternalMeasureUnitImpl() { status.assertSuccess(); assertEquals("tmpMemory identifier", "", tmpMemory.identifier.data()); assertEquals("tmpMemory complexity", UMEASURE_UNIT_SINGLE, tmpMemory.complexity); - assertEquals("tmpMemory units length", 1, tmpMemory.units.length()); - assertEquals("tmpMemory units[0]", "meter", tmpMemory.units[0]->getSimpleUnitID()); + assertEquals("tmpMemory units length", 1, tmpMemory.singleUnits.length()); + assertEquals("tmpMemory units[0]", "meter", tmpMemory.singleUnits[0]->getSimpleUnitID()); assertEquals("tmpImplRef identifier", "", tmpImplRef.identifier.data()); assertEquals("tmpImplRef complexity", UMEASURE_UNIT_SINGLE, tmpImplRef.complexity); @@ -4076,18 +4076,39 @@ void MeasureFormatTest::TestInternalMeasureUnitImpl() { mu1 = std::move(mu2); assertEquals("mu1 = move(mu2): identifier", "", mu1.identifier.data()); assertEquals("mu1 = move(mu2): complexity", UMEASURE_UNIT_COMPOUND, mu1.complexity); - assertEquals("mu1 = move(mu2): units length", 2, mu1.units.length()); - assertEquals("mu1 = move(mu2): units[0]", "newton", mu1.units[0]->getSimpleUnitID()); - assertEquals("mu1 = move(mu2): units[1]", "meter", mu1.units[1]->getSimpleUnitID()); + assertEquals("mu1 = move(mu2): units length", 2, mu1.singleUnits.length()); + assertEquals("mu1 = move(mu2): units[0]", "newton", mu1.singleUnits[0]->getSimpleUnitID()); + assertEquals("mu1 = move(mu2): units[1]", "meter", mu1.singleUnits[1]->getSimpleUnitID()); mu1 = MeasureUnitImpl::forIdentifier("hour-and-minute-and-second", status); status.assertSuccess(); assertEquals("mu1 = HMS: identifier", "", mu1.identifier.data()); assertEquals("mu1 = HMS: complexity", UMEASURE_UNIT_MIXED, mu1.complexity); - assertEquals("mu1 = HMS: units length", 3, mu1.units.length()); - assertEquals("mu1 = HMS: units[0]", "hour", mu1.units[0]->getSimpleUnitID()); - assertEquals("mu1 = HMS: units[1]", "minute", mu1.units[1]->getSimpleUnitID()); - assertEquals("mu1 = HMS: units[2]", "second", mu1.units[2]->getSimpleUnitID()); + assertEquals("mu1 = HMS: units length", 3, mu1.singleUnits.length()); + assertEquals("mu1 = HMS: units[0]", "hour", mu1.singleUnits[0]->getSimpleUnitID()); + assertEquals("mu1 = HMS: units[1]", "minute", mu1.singleUnits[1]->getSimpleUnitID()); + assertEquals("mu1 = HMS: units[2]", "second", mu1.singleUnits[2]->getSimpleUnitID()); + + MeasureUnitImpl m2 = MeasureUnitImpl::forIdentifier("", status); + m2.appendSingleUnit(SingleUnitImpl::forMeasureUnit(MeasureUnit::getMeter(), status), status); + m2.appendSingleUnit(SingleUnitImpl::forMeasureUnit(MeasureUnit::getMeter(), status), status); + status.assertSuccess(); + assertEquals("append meter twice: complexity", UMEASURE_UNIT_SINGLE, m2.complexity); + assertEquals("append meter twice: units length", 1, m2.singleUnits.length()); + assertEquals("append meter twice: units[0]", "meter", m2.singleUnits[0]->getSimpleUnitID()); + assertEquals("append meter twice: identifier", "square-meter", + std::move(m2).build(status).getIdentifier()); + + MeasureUnitImpl mcm = MeasureUnitImpl::forIdentifier("", status); + mcm.appendSingleUnit(SingleUnitImpl::forMeasureUnit(MeasureUnit::getMeter(), status), status); + mcm.appendSingleUnit(SingleUnitImpl::forMeasureUnit(MeasureUnit::getCentimeter(), status), status); + status.assertSuccess(); + assertEquals("append meter & centimeter: complexity", UMEASURE_UNIT_COMPOUND, mcm.complexity); + assertEquals("append meter & centimeter: units length", 2, mcm.singleUnits.length()); + assertEquals("append meter & centimeter: units[0]", "meter", mcm.singleUnits[0]->getSimpleUnitID()); + assertEquals("append meter & centimeter: units[1]", "meter", mcm.singleUnits[1]->getSimpleUnitID()); + assertEquals("append meter & centimeter: identifier", "centimeter-meter", + std::move(mcm).build(status).getIdentifier()); } diff --git a/icu4j/main/classes/core/src/com/ibm/icu/impl/units/MeasureUnitImpl.java b/icu4j/main/classes/core/src/com/ibm/icu/impl/units/MeasureUnitImpl.java index cdaca18febd..0821aa24588 100644 --- a/icu4j/main/classes/core/src/com/ibm/icu/impl/units/MeasureUnitImpl.java +++ b/icu4j/main/classes/core/src/com/ibm/icu/impl/units/MeasureUnitImpl.java @@ -27,7 +27,7 @@ public class MeasureUnitImpl { private MeasureUnit.Complexity complexity = MeasureUnit.Complexity.SINGLE; /** - * The list of simple units. These may be summed or multiplied, based on the + * The list of single units. These may be summed or multiplied, based on the * value of the complexity field. *

* The "dimensionless" unit (SingleUnitImpl default constructor) must not be @@ -141,7 +141,7 @@ public class MeasureUnitImpl { identifier = null; if (singleUnit == null) { - // We don't append dimensionless units. + // Do not append dimensionless units. return false; } @@ -165,8 +165,8 @@ public class MeasureUnitImpl { // Add a copy of singleUnit this.singleUnits.add(singleUnit.copy()); - // If the MeasureUnitImpl is `UMEASURE_UNIT_SINGLE` and after the appending a unit, the singleUnits are more - // than one singleUnit. thus means the complexity should be `UMEASURE_UNIT_COMPOUND` + // If the MeasureUnitImpl is `UMEASURE_UNIT_SINGLE` and after the appending a unit, the singleUnits contains + // more than one. thus means the complexity should be `UMEASURE_UNIT_COMPOUND` if (this.singleUnits.size() > 1 && this.complexity == MeasureUnit.Complexity.SINGLE) { this.setComplexity(MeasureUnit.Complexity.COMPOUND); } @@ -495,7 +495,7 @@ public class MeasureUnitImpl { MeasureUnit.Complexity complexity = fSawAnd ? MeasureUnit.Complexity.MIXED : MeasureUnit.Complexity.COMPOUND; if (result.getSingleUnits().size() == 2) { - // After appending two singleUnits, the complexity will be `UMEASURE_UNIT_COMPOUND` + // After appending two singleUnits, the complexity will be MeasureUnit.Complexity.COMPOUND assert result.getComplexity() == MeasureUnit.Complexity.COMPOUND; result.setComplexity(complexity); } else if (result.getComplexity() != complexity) {