Fix for base unit complexity != 1. Eg: volume -> cubic-meter.

diff --git a/icu4c/source/i18n/getunitsdata.cpp b/icu4c/source/i18n/getunitsdata.cpp
index b98d119e1719b9905583370cdc230eacb5642f4a..ffb0415807ed8d188e770665626eecc73614d5a3 100644 (file)
--- a/icu4c/source/i18n/getunitsdata.cpp
+++ b/icu4c/source/i18n/getunitsdata.cpp
@@ -242,6 +242,7 @@ void collectUnitPrefs(UResourceBundle *usageData, MaybeStackVector<UnitPreferenc
     }
 }
 
+// The input unit needs to be simple, but can have dimensionality != 1.
 void processSingleUnit(const MeasureUnit &unit, const UResourceBundle *convertUnitsBundle,
                        ConversionRateDataSink &convertSink, MeasureUnit *baseSingleUnit,
                        UErrorCode &status) {
@@ -254,7 +255,32 @@ void processSingleUnit(const MeasureUnit &unit, const UResourceBundle *convertUn
     ures_getAllItemsWithFallback(convertUnitsBundle, simple.getIdentifier(), convertSink, status);
 
     if (baseSingleUnit != NULL) {
-        *baseSingleUnit = convertSink.getLastBaseUnit(status).withDimensionality(dimensionality, status);
+        MeasureUnit baseUnit = convertSink.getLastBaseUnit(status);
+
+        if (dimensionality == 1) {
+            *baseSingleUnit = baseUnit;
+        } else if (baseUnit.getComplexity(status) == UMEASURE_UNIT_SINGLE) {
+            // TODO(hugovdm): find examples where we're converting a *-per-* to
+            // a square-*? Does one ever square frequency? What about
+            // squared-speed in the case of mv^2? Or F=ma^2?
+            //
+            // baseUnit might also have dimensionality, e.g. cubic-meter -
+            // retain this instead of overriding with input unit dimensionality:
+            dimensionality *= baseUnit.getDimensionality(status);
+            *baseSingleUnit = baseUnit.withDimensionality(dimensionality, status);
+        } else {
+            // We only support higher dimensionality input units if they map to
+            // simple base units, such that that base unit can have the
+            // dimensionality easily applied.
+            //
+            // TODO(hugovdm): produce succeeding examples of simple input unit
+            // mapped to a different simple target/base unit.
+            //
+            // TODO(hugovdm): produce failing examples of higher-dimensionality
+            // or inverted input units that map to compound output units.
+            status = U_ILLEGAL_ARGUMENT_ERROR;
+            return;
+        }
     }
 }
 
@@ -290,7 +316,6 @@ MaybeStackVector<ConversionRateInfo> getConversionRatesInfo(const MeasureUnit so
         }
     }
     if (baseCompoundUnit != NULL) {
-        fprintf(stderr, "source base: %s\n", baseCompoundUnit->getIdentifier());
         *baseCompoundUnit = MeasureUnit();
     }
     for (int i = 0; i < targetUnitsLength; i++) {

diff --git a/icu4c/source/test/intltest/unitstest.cpp b/icu4c/source/test/intltest/unitstest.cpp
index d53e6e8539a04878196cfe17ff3af1ce63436b22..b5a5e83f0a5f11e1b93377443ac5ef2dce69a0fb 100644 (file)
--- a/icu4c/source/test/intltest/unitstest.cpp
+++ b/icu4c/source/test/intltest/unitstest.cpp
@@ -590,10 +590,16 @@ void UnitsTest::testGetConversionRateInfo() {
         {"centimeter-per-square-milligram",
          "inch-per-square-ounce",
          {"pound", "stone", "ton"},
-         "kilogram"},
+         "meter-per-square-kilogram"},
         {"liter", "gallon", {"liter", "gallon", NULL}, "cubic-meter"},
         {"stone-and-pound", "ton", {"pound", "stone", "ton"}, "kilogram"},
         {"mile-per-hour", "dekameter-per-hour", {"mile", "hour", "meter"}, "meter-per-second"},
+        {"kilovolt-ampere",
+         "horsepower",
+         {"volt", "ampere", "horsepower"},
+         "kilogram-square-meter-per-cubic-second"}, // watt
+        // TODO: include capacitance test case with base unit:
+        // pow4-second-square-ampere-per-kilogram-square-meter;
     };
     for (const auto &t : testCases) {
         logln("---testing: source=\"%s\", target=\"%s\", expectedBaseUnit=\"%s\"", t.sourceUnit,
@@ -605,8 +611,13 @@ void UnitsTest::testGetConversionRateInfo() {
         MeasureUnit targetUnit = MeasureUnit::forIdentifier(t.targetUnit, status);
         MaybeStackVector<ConversionRateInfo> conversionInfo =
             getConversionRatesInfo(sourceUnit, targetUnit, &baseCompoundUnit, status);
+        if (status.errIfFailureAndReset("getConversionRatesInfo(<%s>, <%s>, ...)",
+                                        sourceUnit.getIdentifier(), targetUnit.getIdentifier())) {
+            continue;
+        }
 
-        logln("---found BaseUnit=\"%s\"", baseCompoundUnit.getIdentifier());
+        assertEquals("baseCompoundUnit returned by getConversionRatesInfo", t.baseUnit,
+                     baseCompoundUnit.getIdentifier());
         for (int i = 0; i < conversionInfo.length(); i++) {
             ConversionRateInfo *cri;
             cri = conversionInfo[i];