From: Hugo van der Merwe <17109322+hugovdm@users.noreply.github.com> Date: Fri, 16 Oct 2020 17:06:28 +0000 (+0200) Subject: ICU-21311 Fix code generation in MeasureUnitTest.java and use it X-Git-Tag: cldr/2020-12-03b~14 X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=0439cc5f7beceb3bdf95815e8a8b75f21bd11e05;p=icu ICU-21311 Fix code generation in MeasureUnitTest.java and use it --- diff --git a/icu4c/source/i18n/measunit.cpp b/icu4c/source/i18n/measunit.cpp index 02624f824e1..dab3abb5e21 100644 --- a/icu4c/source/i18n/measunit.cpp +++ b/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 -// TODO(ICU-21076): improve how this generated code is produced. +// Start generated code for measunit.cpp // 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, - 432, - 442, - 446, + 424, + 430, + 440, + 444, + 448, 450, - 452, - 486 + 484 }; -// 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; @@ -2304,7 +2272,7 @@ void MeasureUnit::initTime(const char *timeId) { fTypeId = result; result = binarySearch(gSubTypes, gOffsets[fTypeId], gOffsets[fTypeId + 1], timeId); U_ASSERT(result != -1); - fSubTypeId = result - gOffsets[fTypeId]; + fSubTypeId = result - gOffsets[fTypeId]; } void MeasureUnit::initCurrency(StringPiece isoCurrency) { diff --git a/icu4c/source/i18n/unicode/measunit.h b/icu4c/source/i18n/unicode/measunit.h index d86bab39095..b9f732ae990 100644 --- a/icu4c/source/i18n/unicode/measunit.h +++ b/icu4c/source/i18n/unicode/measunit.h @@ -23,10 +23,10 @@ #include "unicode/localpointer.h" /** - * \file + * \file * \brief C++ API: A unit for measuring a quantity. */ - + U_NAMESPACE_BEGIN class StringEnumeration; @@ -35,7 +35,7 @@ struct MeasureUnitImpl; #ifndef U_HIDE_DRAFT_API /** * Enumeration for unit complexity. There are three levels: - * + * * - SINGLE: A single unit, optionally with a power and/or SI prefix. Examples: hectare, * square-kilometer, kilojoule, per-second. * - COMPOUND: A unit composed of the product of multiple single units. Examples: @@ -58,7 +58,7 @@ enum UMeasureUnitComplexity { /** * A compound unit, like meter-per-second. - * + * * @draft ICU 67 */ UMEASURE_UNIT_COMPOUND, @@ -243,7 +243,7 @@ class U_I18N_API MeasureUnit: public UObject { * @stable ICU 3.0 */ MeasureUnit(); - + /** * Copy constructor. * @stable ICU 3.0 @@ -3519,7 +3519,6 @@ class U_I18N_API MeasureUnit: public UObject { */ static MeasureUnit getTeaspoon(); - // End generated createXXX methods protected: diff --git a/icu4j/main/classes/core/src/com/ibm/icu/util/MeasureUnit.java b/icu4j/main/classes/core/src/com/ibm/icu/util/MeasureUnit.java index e3de81bdb10..69a81f65cb2 100644 --- a/icu4j/main/classes/core/src/com/ibm/icu/util/MeasureUnit.java +++ b/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 { diff --git a/icu4j/main/tests/core/src/com/ibm/icu/dev/test/format/MeasureUnitTest.java b/icu4j/main/tests/core/src/com/ibm/icu/dev/test/format/MeasureUnitTest.java index a7f706cbef9..3033f856f48 100644 --- a/icu4j/main/tests/core/src/com/ibm/icu/dev/test/format/MeasureUnitTest.java +++ b/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 seen = new HashMap<>(); + System.out.println("// Start generated createXXX methods"); System.out.println(); TreeMap> allUnits = getAllUnits(); for (Map.Entry> 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> allUnits = getAllUnits(); - // Hack: for C++, add NoUnits here, but ignore them when printing the create methods. - // 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})); + // 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. + 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> 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[] = {"); - index = 0; - for (Map.Entry> 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("static const int32_t kCurrencyOffset = " + currencyIndex + ";"); 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, Pair> entry - : unitPerUnitOffsetsToTypeSubType.entrySet()) { - if (!first) { - System.out.println(","); - } - first = false; - OrderedPair unitPerUnitOffsets = entry.getKey(); - Pair 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 seen = new HashMap<>(); TreeMap> 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, MeasureUnit>unitPerUnitToSingleUnit ="); - System.out.println(" new HashMap, MeasureUnit>();"); - System.out.println(); - System.out.println(" static {"); - for (Map.Entry> unitPerUnitEntry - : getUnitsToPerParts().entrySet()) { - Pair unitPerUnit = unitPerUnitEntry.getValue(); - System.out.println(" unitPerUnitToSingleUnit.put(Pair.of(MeasureUnit." + toJAVAName(unitPerUnit.first) + ", MeasureUnit." + toJAVAName(unitPerUnit.second) + "), MeasureUnit." + toJAVAName(unitPerUnitEntry.getKey()) + ");"); - } - System.out.println(" }"); + System.out.println(" // End generated MeasureUnit constants"); } private static String getVersion(String javaName, String thisVersion) {