Rip out compound base unit calculation.

diff --git a/icu4c/source/i18n/unitsdata.cpp b/icu4c/source/i18n/unitsdata.cpp
index 6651a905768d2256c3b0ec6c190a80683b03db9f..1fe9076001571eb53e6fe38c63f1fc53fd7c6a97 100644 (file)
--- a/icu4c/source/i18n/unitsdata.cpp
+++ b/icu4c/source/i18n/unitsdata.cpp
@@ -151,14 +151,10 @@ class ConversionRateDataSink : public ResourceSink {
  * resource.
  * @param convertSink The ConversionRateDataSink through which
  * ConversionRateInfo instances are to be collected.
- * @param baseSingleUnit Output parameter: if not NULL, the base unit through
- * which conversion rates pivot to other similar units will be returned through
- * this pointer.
  * @param status The standard ICU error code output parameter.
  */
 void processSingleUnit(const MeasureUnit &unit, const UResourceBundle *convertUnitsBundle,
-                       ConversionRateDataSink &convertSink, MeasureUnit *baseSingleUnit,
-                       UErrorCode &status) {
+                       ConversionRateDataSink &convertSink, UErrorCode &status) {
     if (U_FAILURE(status)) return;
     int32_t dimensionality = unit.getDimensionality(status);
 
@@ -168,32 +164,12 @@ void processSingleUnit(const MeasureUnit &unit, const UResourceBundle *convertUn
         simple = unit.withDimensionality(1, status).withSIPrefix(UMEASURE_SI_PREFIX_ONE, status);
     }
     ures_getAllItemsWithFallback(convertUnitsBundle, simple.getIdentifier(), convertSink, status);
-
-    if (baseSingleUnit != NULL) {
-        MeasureUnit baseUnit = convertSink.getLastBaseUnit(status);
-
-        if (dimensionality == 1) {
-            *baseSingleUnit = baseUnit;
-        } else if (baseUnit.getComplexity(status) == UMEASURE_UNIT_SINGLE) {
-            // The baseUnit is a single unit, so can be raised to the
-            // dimensionality of the input unit.
-            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.
-            status = U_ILLEGAL_ARGUMENT_ERROR;
-            return;
-        }
-    }
 }
 
 } // namespace
 
-MaybeStackVector<ConversionRateInfo> getConversionRatesInfo(const MeasureUnit source,
-                                                            const MeasureUnit target,
-                                                            MeasureUnit *baseUnit, UErrorCode &status) {
+MaybeStackVector<ConversionRateInfo>
+getConversionRatesInfo(const MeasureUnit source, const MeasureUnit target, UErrorCode &status) {
     MaybeStackVector<ConversionRateInfo> result;
     if (U_FAILURE(status)) return result;
 
@@ -206,61 +182,13 @@ MaybeStackVector<ConversionRateInfo> getConversionRatesInfo(const MeasureUnit so
     ures_getByKey(unitsBundle.getAlias(), "convertUnits", convertUnitsBundle.getAlias(), &status);
     ConversionRateDataSink convertSink(result);
 
-    MeasureUnit sourceBaseUnit;
     for (int i = 0; i < sourceUnitsLength; i++) {
-        MeasureUnit baseUnit;
-        processSingleUnit(sourceUnits[i], convertUnitsBundle.getAlias(), convertSink, &baseUnit, status);
-        if (source.getComplexity(status) == UMEASURE_UNIT_SEQUENCE) {
-            if (i == 0) {
-                sourceBaseUnit = baseUnit;
-            } else {
-                if (baseUnit != sourceBaseUnit) {
-                    status = U_ILLEGAL_ARGUMENT_ERROR;
-                    return result;
-                }
-            }
-        } else {
-            sourceBaseUnit = sourceBaseUnit.product(baseUnit, status);
-        }
+        processSingleUnit(sourceUnits[i], convertUnitsBundle.getAlias(), convertSink, status);
     }
 
-    MeasureUnit targetBaseUnit;
     for (int i = 0; i < targetUnitsLength; i++) {
-        MeasureUnit baseUnit;
-        processSingleUnit(targetUnits[i], convertUnitsBundle.getAlias(), convertSink, &baseUnit, status);
-        if (target.getComplexity(status) == UMEASURE_UNIT_SEQUENCE) {
-            if (baseUnit != sourceBaseUnit) {
-                status = U_ILLEGAL_ARGUMENT_ERROR;
-                return result;
-            }
-            targetBaseUnit = baseUnit;
-        } else {
-            // WIP/FIXME(hugovdm): ensure this gets fixed, then remove this
-            // comment: I think I found a bug in targetBaseUnit.product(). First
-            // symptom was an unexpected product, further exploration resulted
-            // in AddressSanitizer errors.
-            //
-            // The product was:
-            //
-            // <kilogram-square-meter-per-square-second> * <one-per-meter> => <meter>
-            //
-            // as output by a printf:
-            //
-            // fprintf(stderr, "<%s> x <%s> => ",
-            //         targetBaseUnit.getIdentifier(),
-            //         baseUnit.getIdentifier());
-            //
-            // Expected: <kilogram-square-meter-per-meter-square-second>
-            targetBaseUnit = targetBaseUnit.product(baseUnit, status);
-            // fprintf(stderr, "<%s> - Status: %s\n",
-            //         targetBaseUnit.getIdentifier(), u_errorName(status));
-        }
-    }
-    if (targetBaseUnit != sourceBaseUnit) {
-        status = U_ILLEGAL_ARGUMENT_ERROR;
-        return result;
+        processSingleUnit(targetUnits[i], convertUnitsBundle.getAlias(), convertSink, status);
     }
-    if (baseUnit != NULL) { *baseUnit = sourceBaseUnit; }
     return result;
 }
 

diff --git a/icu4c/source/i18n/unitsdata.h b/icu4c/source/i18n/unitsdata.h
index 9893c23f09e5135bebf60791d35dd3e78a29c4ec..58aa989cdf58816c0d96d2a9843371004f7ed9e1 100644 (file)
--- a/icu4c/source/i18n/unitsdata.h
+++ b/icu4c/source/i18n/unitsdata.h
@@ -38,22 +38,18 @@ class U_I18N_API ConversionRateInfo {
 };
 
 /**
- * Collects and returns ConversionRateInfo needed to convert from source to
- * baseUnit and from target to baseUnit.
+ * Collects and returns ConversionRateInfo needed for conversions from source
+ * and to target.
  *
  * If source and target are not compatible for conversion, status will be set to
  * U_ILLEGAL_ARGUMENT_ERROR.
  *
  * @param source The source unit (the unit type converted from).
  * @param target The target unit (the unit type converted to).
- * @param baseUnit Output parameter: if not NULL, it will be set to the base
- * unit type used as pivot for converting from source to target. This may be a
- * compound unit (a combination of base units).
  * @param status Receives status.
  */
 MaybeStackVector<ConversionRateInfo> U_I18N_API getConversionRatesInfo(MeasureUnit source,
                                                                        MeasureUnit target,
-                                                                       MeasureUnit *baseUnit,
                                                                        UErrorCode &status);
 
 U_NAMESPACE_END

diff --git a/icu4c/source/test/intltest/unitsdatatest.cpp b/icu4c/source/test/intltest/unitsdatatest.cpp
index 3ca72db4e8ce47ebbfbf31a1d1995e99edfdf3c9..cd3e213fd760923ebe091cce2eea217a8cb9c728 100644 (file)
--- a/icu4c/source/test/intltest/unitsdatatest.cpp
+++ b/icu4c/source/test/intltest/unitsdatatest.cpp
@@ -33,74 +33,42 @@ void UnitsDataTest::testGetConversionRateInfo() {
         const char *targetUnit;
         // Expected: units whose conversion rates are expected in the results.
         const char *expectedOutputs[MAX_NUM_RATES];
-        // Expected "base unit", to serve as pivot between source and target.
-        const char *expectedBaseUnit;
     } testCases[]{
         {"centimeter-per-square-milligram",
          "inch-per-square-ounce",
-         {"meter", "gram", "inch", "ounce", NULL},
-         "meter-per-square-kilogram"},
+         {"meter", "gram", "inch", "ounce", NULL}},
 
-        {"liter", "gallon", {"liter", "gallon", NULL, NULL, NULL}, "cubic-meter"},
+        {"liter", "gallon", {"liter", "gallon", NULL, NULL, NULL}},
 
         // Sequence
-        {"stone-and-pound", "ton", {"pound", "stone", "ton", NULL, NULL}, "kilogram"},
+        {"stone-and-pound", "ton", {"pound", "stone", "ton", NULL, NULL}},
 
-        {"mile-per-hour",
-         "dekameter-per-hour",
-         {"mile", "hour", "meter", NULL, NULL},
-         "meter-per-second"},
+        {"mile-per-hour", "dekameter-per-hour", {"mile", "hour", "meter", NULL, NULL}},
 
         // Power: watt
-        {"watt",
-         "horsepower",
-         {"watt", "horsepower", NULL, NULL, NULL},
-         "kilogram-square-meter-per-cubic-second"},
+        {"watt", "horsepower", {"watt", "horsepower", NULL, NULL, NULL}},
 
         // Energy: joule
         {"therm-us",
          "kilogram-square-meter-per-square-second",
-         {"therm-us", "kilogram", "meter", "second", NULL},
-         "kilogram-square-meter-per-square-second"},
+         {"therm-us", "kilogram", "meter", "second", NULL}},
 
         // Add "reciprocal" example: consumption and consumption-inverse
-        //
-        // WIP/FIXME: failing example which should pass. Thus we will remove the
-        // base unit calculation.
-        {"liter-per-100-kilometer",
-         "mile-per-gallon",
-         {"meter", "mile", "gallon", NULL, NULL},
-         "cubic-meter-per-meter"},
-
-        // WIP/FIXME(hugovdm): I think I found a bug in targetBaseUnit.product():
-        // Target Base: <kilogram-square-meter-per-square-second> x <one-per-meter> => <meter>
-        //
-        // // Joule-per-meter
-        // {"therm-us-per-meter",
-        //  "joule-per-meter",
-        //  {"therm-us", "joule", "meter", NULL, NULL},
-        //  "kilogram-square-meter-per-meter-square-second"},
-
-        // TODO: include capacitance test case with base unit:
-        // pow4-second-square-ampere-per-kilogram-square-meter;
+        {"liter-per-100-kilometer", "mile-per-gallon", {"liter", "100-kilometer", "mile", "gallon", NULL}},
     };
     for (const auto &t : testCases) {
-        logln("---testing: source=\"%s\", target=\"%s\", expectedBaseUnit=\"%s\"", t.sourceUnit,
-              t.targetUnit, t.expectedBaseUnit);
+        logln("---testing: source=\"%s\", target=\"%s\"", t.sourceUnit, t.targetUnit);
         IcuTestErrorCode status(*this, "testGetConversionRateInfo");
 
-        MeasureUnit baseCompoundUnit;
         MeasureUnit sourceUnit = MeasureUnit::forIdentifier(t.sourceUnit, status);
         MeasureUnit targetUnit = MeasureUnit::forIdentifier(t.targetUnit, status);
         MaybeStackVector<ConversionRateInfo> conversionInfo =
-            getConversionRatesInfo(sourceUnit, targetUnit, &baseCompoundUnit, status);
+            getConversionRatesInfo(sourceUnit, targetUnit, status);
         if (status.errIfFailureAndReset("getConversionRatesInfo(<%s>, <%s>, ...)",
                                         sourceUnit.getIdentifier(), targetUnit.getIdentifier())) {
             continue;
         }
 
-        assertEquals("baseCompoundUnit returned by getConversionRatesInfo", t.expectedBaseUnit,
-                     baseCompoundUnit.getIdentifier());
         int countExpected;
         for (countExpected = 0; countExpected < MAX_NUM_RATES; countExpected++) {
             auto expected = t.expectedOutputs[countExpected];