ICU-21311 Fix code generation in MeasureUnitTest.java and use it

icu4c/source/i18n/measunit.cpp

@@ -33,8 +33,7 @@ UOBJECT_DEFINE_RTTI_IMPLEMENTATION(MeasureUnit)
 // update this code, refer to:
 // http://site.icu-project.org/design/formatting/measureformat/updating-measure-unit
 //
-// Start generated code
+// Start generated code for measunit.cpp
-// TODO(ICU-21076): improve how this generated code is produced.
 
 // Maps from Type ID to offset in gSubTypes.
 static const int32_t gOffsets[] = {
@@ -55,44 +54,15 @@ static const int32_t gOffsets[] = {
     404,
     408,
     423,
-    426,
+    424,
-    432,
+    430,
-    442,
+    440,
-    446,
+    444,
+    448,
     450,
-    452,
+    484
-    486
 };
 
-// TODO: FIX CODE GENERATION - leaving this here but commented-out to make it
-// clear that we no longer want this array. We needed it for only one thing: efficient checking of "currency".
-//
-// static const int32_t gIndexes[] = {
-//     0,
-//     2,
-//     7,
-//     17,
-//     25,
-//     29,
-//     29,
-//     40,
-//     56,
-//     60,
-//     69,
-//     71,
-//     75,
-//     83,
-//     105,
-//     109,
-//     124,
-//     127,
-//     133,
-//     143,
-//     147,
-//     151,
-//     153,
-//     187
-// };
 static const int32_t kCurrencyOffset = 5;
 
 // Must be sorted alphabetically.
@@ -547,9 +517,7 @@ static const char * const gSubTypes[] = {
     "solar-mass",
     "stone",
     "ton",
-    "", // TODO(ICU-21076): manual edit of what should have been generated by Java.
+    "",
-    "percent", // TODO(ICU-21076): regenerate, deal with duplication.
-    "permille", // TODO(ICU-21076): regenerate, deal with duplication.
     "gigawatt",
     "horsepower",
     "kilowatt",
@@ -612,8 +580,6 @@ static const char * const gSubTypes[] = {
     "teaspoon"
 };
 
-// unitPerUnitToSingleUnit no longer in use! TODO: remove from code-generation code.
-
 // Shortcuts to the base unit in order to make the default constructor fast
 static const int32_t kBaseTypeIdx = 16;
 static const int32_t kBaseSubTypeIdx = 0;
@@ -2090,7 +2056,7 @@ MeasureUnit MeasureUnit::getTeaspoon() {
     return MeasureUnit(22, 33);
 }
 
-// End generated code
+// End generated code for measunit.cpp
 
 static int32_t binarySearch(
         const char * const * array, int32_t start, int32_t end, StringPiece key) {
@@ -2271,9 +2237,11 @@ StringEnumeration* MeasureUnit::getAvailableTypes(UErrorCode &errorCode) {
 }
 
 bool MeasureUnit::findBySubType(StringPiece subType, MeasureUnit* output) {
+    // Sanity checking kCurrencyOffset and final entry in gOffsets
+    U_ASSERT(uprv_strcmp(gTypes[kCurrencyOffset], "currency") == 0);
+    U_ASSERT(gOffsets[UPRV_LENGTHOF(gOffsets) - 1] == UPRV_LENGTHOF(gSubTypes));
+
     for (int32_t t = 0; t < UPRV_LENGTHOF(gOffsets) - 1; t++) {
-        // Ensure kCurrencyOffset is set correctly
-        U_ASSERT(uprv_strcmp(gTypes[kCurrencyOffset], "currency") == 0);
         // Skip currency units
         if (t == kCurrencyOffset) {
             continue;

icu4c/source/i18n/unicode/measunit.h

@@ -3519,7 +3519,6 @@ class U_I18N_API MeasureUnit: public UObject {
      */
     static MeasureUnit getTeaspoon();
 
-
 // End generated createXXX methods
 
  protected:

icu4j/main/classes/core/src/com/ibm/icu/util/MeasureUnit.java

@@ -1306,7 +1306,7 @@ public class MeasureUnit implements Serializable {
      * Constant for unit of graphics: dot
      * @draft ICU 68
      * @provisional This API might change or be removed in a future release.
-    */
+     */
     public static final MeasureUnit DOT = MeasureUnit.internalGetInstance("graphics", "dot");
 
     /**
@@ -1373,7 +1373,7 @@ public class MeasureUnit implements Serializable {
      * Constant for unit of length: earth-radius
      * @draft ICU 68
      * @provisional This API might change or be removed in a future release.
-    */
+     */
     public static final MeasureUnit EARTH_RADIUS = MeasureUnit.internalGetInstance("length", "earth-radius");
 
     /**
@@ -1488,14 +1488,14 @@ public class MeasureUnit implements Serializable {
      * Constant for unit of light: candela
      * @draft ICU 68
      * @provisional This API might change or be removed in a future release.
-    */
+     */
     public static final MeasureUnit CANDELA = MeasureUnit.internalGetInstance("light", "candela");
 
     /**
      * Constant for unit of light: lumen
      * @draft ICU 68
      * @provisional This API might change or be removed in a future release.
-    */
+     */
     public static final MeasureUnit LUMEN = MeasureUnit.internalGetInstance("light", "lumen");
 
     /**
@@ -1532,7 +1532,7 @@ public class MeasureUnit implements Serializable {
      * Constant for unit of mass: grain
      * @draft ICU 68
      * @provisional This API might change or be removed in a future release.
-    */
+     */
     public static final MeasureUnit GRAIN = MeasureUnit.internalGetInstance("mass", "grain");
 
     /**
@@ -1845,28 +1845,28 @@ public class MeasureUnit implements Serializable {
      * Constant for unit of volume: dessert-spoon
      * @draft ICU 68
      * @provisional This API might change or be removed in a future release.
-    */
+     */
     public static final MeasureUnit DESSERT_SPOON = MeasureUnit.internalGetInstance("volume", "dessert-spoon");
 
     /**
      * Constant for unit of volume: dessert-spoon-imperial
      * @draft ICU 68
      * @provisional This API might change or be removed in a future release.
-    */
+     */
     public static final MeasureUnit DESSERT_SPOON_IMPERIAL = MeasureUnit.internalGetInstance("volume", "dessert-spoon-imperial");
 
     /**
      * Constant for unit of volume: dram
      * @draft ICU 68
      * @provisional This API might change or be removed in a future release.
-    */
+     */
     public static final MeasureUnit DRAM = MeasureUnit.internalGetInstance("volume", "dram");
 
     /**
      * Constant for unit of volume: drop
      * @draft ICU 68
      * @provisional This API might change or be removed in a future release.
-    */
+     */
     public static final MeasureUnit DROP = MeasureUnit.internalGetInstance("volume", "drop");
 
     /**
@@ -1903,7 +1903,7 @@ public class MeasureUnit implements Serializable {
      * Constant for unit of volume: jigger
      * @draft ICU 68
      * @provisional This API might change or be removed in a future release.
-    */
+     */
     public static final MeasureUnit JIGGER = MeasureUnit.internalGetInstance("volume", "jigger");
 
     /**
@@ -1928,7 +1928,7 @@ public class MeasureUnit implements Serializable {
      * Constant for unit of volume: pinch
      * @draft ICU 68
      * @provisional This API might change or be removed in a future release.
-    */
+     */
     public static final MeasureUnit PINCH = MeasureUnit.internalGetInstance("volume", "pinch");
 
     /**
@@ -1953,7 +1953,7 @@ public class MeasureUnit implements Serializable {
      * Constant for unit of volume: quart-imperial
      * @draft ICU 68
      * @provisional This API might change or be removed in a future release.
-    */
+     */
     public static final MeasureUnit QUART_IMPERIAL = MeasureUnit.internalGetInstance("volume", "quart-imperial");
 
     /**
@@ -1968,9 +1968,8 @@ public class MeasureUnit implements Serializable {
      */
     public static final MeasureUnit TEASPOON = MeasureUnit.internalGetInstance("volume", "teaspoon");
 
-    // unitPerUnitToSingleUnit no longer in use! TODO: remove from code-generation code.
-
     // End generated MeasureUnit constants
+
     /* Private */
 
     private Object writeReplace() throws ObjectStreamException {

icu4j/main/tests/core/src/com/ibm/icu/dev/test/format/MeasureUnitTest.java

@@ -2898,6 +2898,7 @@ public class MeasureUnitTest extends TestFmwk {
     // for MeasureFormat during the release process.
     static void generateCXXHConstants(String thisVersion) {
         Map<String, MeasureUnit> seen = new HashMap<>();
+        System.out.println("// Start generated createXXX methods");
         System.out.println();
         TreeMap<String, List<MeasureUnit>> allUnits = getAllUnits();
         for (Map.Entry<String, List<MeasureUnit>> entry : allUnits.entrySet()) {
@@ -2929,13 +2930,15 @@ public class MeasureUnitTest extends TestFmwk {
                 System.out.println("    /**");
                 System.out.println("     * Returns by value, unit of " + type + ": " + code + ".");
                 System.out.printf("     * Also see {@link #create%s()}.\n", name);
-                // TODO: When the get* methods become stable in ICU 66, update their
-                // @draft code to be more like that for the create* methods above.
                 String getterVersion = getVersion(javaName, thisVersion);
                 if (Integer.valueOf(getterVersion) < 64) {
                     getterVersion = "64";
                 }
-                System.out.println("     * @draft ICU " + getterVersion);
+                if (isDraft(javaName)) {
+                    System.out.println("     * @draft ICU " + getterVersion);
+                } else {
+                    System.out.println("     * @stable ICU " + getterVersion);
+                }
                 System.out.println("     */");
                 System.out.printf("    static MeasureUnit get%s();\n", name);
                 if (isDraft(javaName)) {
@@ -2944,6 +2947,7 @@ public class MeasureUnitTest extends TestFmwk {
                 System.out.println("");
             }
         }
+        System.out.println("// End generated createXXX methods");
     }
 
     private static void checkForDup(
@@ -2998,34 +3002,34 @@ public class MeasureUnitTest extends TestFmwk {
     // DO NOT DELETE THIS FUNCTION! It may appear as dead code, but we use this to generate code
     // for MeasureFormat during the release process.
     static void generateCXXConstants() {
+        System.out.println("// Start generated code for measunit.cpp");
         System.out.println("");
         TreeMap<String, List<MeasureUnit>> allUnits = getAllUnits();
 
-        // Hack: for C++, add NoUnits here, but ignore them when printing the create methods.
+        // Hack: for C++, add base unit here, but ignore them when printing the create methods.
-        // ALso keep track of the base unit offset to make the C++ default constructor faster.
+        // Also keep track of the base unit offset to make the C++ default constructor faster.
-        allUnits.put("none", Arrays.asList(new MeasureUnit[]{NoUnit.BASE, NoUnit.PERCENT, NoUnit.PERMILLE}));
+        allUnits.put("none", Arrays.asList(new MeasureUnit[] {NoUnit.BASE}));
         int baseTypeIdx = -1;
         int baseSubTypeIdx = -1;
 
+        System.out.println("// Maps from Type ID to offset in gSubTypes.");
         System.out.println("static const int32_t gOffsets[] = {");
         int index = 0;
+        int typeCount = 0;
+        int currencyIndex = -1;
         for (Map.Entry<String, List<MeasureUnit>> entry : allUnits.entrySet()) {
             System.out.printf("    %d,\n", index);
+            if (entry.getKey() == "currency") {
+                currencyIndex = typeCount;
+            }
+            typeCount++;
             index += entry.getValue().size();
         }
+        assertTrue("currency present", currencyIndex >= 0);
         System.out.printf("    %d\n", index);
         System.out.println("};");
         System.out.println();
-        System.out.println("static const int32_t gIndexes[] = {");
+        System.out.println("static const int32_t kCurrencyOffset = " + currencyIndex + ";");
-        index = 0;
-        for (Map.Entry<String, List<MeasureUnit>> entry : allUnits.entrySet()) {
-            System.out.printf("    %d,\n", index);
-            if (!entry.getKey().equals("currency")) {
-                index += entry.getValue().size();
-            }
-        }
-        System.out.printf("    %d\n", index);
-        System.out.println("};");
         System.out.println();
         System.out.println("// Must be sorted alphabetically.");
         System.out.println("static const char * const gTypes[] = {");
@@ -3054,7 +3058,12 @@ public class MeasureUnitTest extends TestFmwk {
                 if (!first) {
                     System.out.println(",");
                 }
-                System.out.print("    \"" + unit.getSubtype() + "\"");
+                if (unit != null) {
+                    System.out.print("    \"" + unit.getSubtype() + "\"");
+                } else {
+                    assertEquals("unit only null for \"none\" type", "none", entry.getKey());
+                    System.out.print("    \"\"");
+                }
                 first = false;
                 measureUnitToOffset.put(unit, offset);
                 measureUnitToTypeSubType.put(unit, Pair.of(typeIdx, subTypeIdx));
@@ -3085,27 +3094,6 @@ public class MeasureUnitTest extends TestFmwk {
                     measureUnitToTypeSubType.get(entry.getKey()));
         }
 
-        System.out.println("// Must be sorted by first value and then second value.");
-        System.out.println("static int32_t unitPerUnitToSingleUnit[][4] = {");
-        first = true;
-        for (Map.Entry<OrderedPair<Integer, Integer>, Pair<Integer, Integer>> entry
-                : unitPerUnitOffsetsToTypeSubType.entrySet()) {
-            if (!first) {
-                System.out.println(",");
-            }
-            first = false;
-            OrderedPair<Integer, Integer> unitPerUnitOffsets = entry.getKey();
-            Pair<Integer, Integer> typeSubType = entry.getValue();
-            System.out.printf("        {%d, %d, %d, %d}",
-                    unitPerUnitOffsets.first,
-                    unitPerUnitOffsets.second,
-                    typeSubType.first,
-                    typeSubType.second);
-        }
-        System.out.println();
-        System.out.println("};");
-        System.out.println();
-
         // Print out the fast-path for the default constructor
         System.out.println("// Shortcuts to the base unit in order to make the default constructor fast");
         System.out.println("static const int32_t kBaseTypeIdx = " + baseTypeIdx + ";");
@@ -3138,6 +3126,7 @@ public class MeasureUnitTest extends TestFmwk {
                 System.out.println();
             }
         }
+        System.out.println("// End generated code for measunit.cpp");
     }
 
     private static String toCamelCase(MeasureUnit unit) {
@@ -3243,6 +3232,7 @@ public class MeasureUnitTest extends TestFmwk {
     // DO NOT DELETE THIS FUNCTION! It may appear as dead code, but we use this to generate code
     // for MeasureFormat during the release process.
     static void generateConstants(String thisVersion) {
+        System.out.println("    // Start generated MeasureUnit constants");
         System.out.println();
         Map<String, MeasureUnit> seen = new HashMap<>();
         TreeMap<String, List<MeasureUnit>> allUnits = getAllUnits();
@@ -3269,7 +3259,7 @@ public class MeasureUnitTest extends TestFmwk {
                 } else {
                     System.out.println("     * @stable ICU " + getVersion(name, thisVersion));
                 }
-                System.out.println("    */");
+                System.out.println("     */");
                 if ("duration".equals(type) && TIME_CODES.contains(code)) {
                     System.out.println("    public static final TimeUnit " + name + " = (TimeUnit) MeasureUnit.internalGetInstance(\"" +
                             type +
@@ -3286,16 +3276,7 @@ public class MeasureUnitTest extends TestFmwk {
                 System.out.println();
             }
         }
-        System.out.println("    private static HashMap<Pair<MeasureUnit, MeasureUnit>, MeasureUnit>unitPerUnitToSingleUnit =");
+        System.out.println("    // End generated MeasureUnit constants");
-        System.out.println("            new HashMap<Pair<MeasureUnit, MeasureUnit>, MeasureUnit>();");
-        System.out.println();
-        System.out.println("    static {");
-        for (Map.Entry<MeasureUnit, Pair<MeasureUnit, MeasureUnit>> unitPerUnitEntry
-                : getUnitsToPerParts().entrySet()) {
-            Pair<MeasureUnit, MeasureUnit> unitPerUnit = unitPerUnitEntry.getValue();
-            System.out.println("        unitPerUnitToSingleUnit.put(Pair.<MeasureUnit, MeasureUnit>of(MeasureUnit." + toJAVAName(unitPerUnit.first) + ", MeasureUnit." + toJAVAName(unitPerUnit.second) + "), MeasureUnit." + toJAVAName(unitPerUnitEntry.getKey()) + ");");
-        }
-        System.out.println("    }");
     }
 
     private static String getVersion(String javaName, String thisVersion) {