From: Shane Carr Date: Fri, 30 Mar 2018 04:28:53 +0000 (+0000) Subject: ICU-13634 Various fixes to fix remaining compatibility issues in data-driven test... X-Git-Tag: release-62-rc~200^2~62 X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=e5bda1eb0e45dad5d56b4d793786999f188ce182;p=icu ICU-13634 Various fixes to fix remaining compatibility issues in data-driven test. Includes fix for a memory sanitizer issue. X-SVN-Rev: 41174 --- diff --git a/icu4c/source/i18n/fmtable.cpp b/icu4c/source/i18n/fmtable.cpp index c65c5c2e095..862b5f912e5 100644 --- a/icu4c/source/i18n/fmtable.cpp +++ b/icu4c/source/i18n/fmtable.cpp @@ -771,7 +771,7 @@ Formattable::adoptDecimalQuantity(DecimalQuantity *dq) { // Cannot use the set() functions because they would delete the fDecimalNum value, // TODO: fDecimalQuantity->fitsInInt() to kLong type. /* - if (fDecimalQuantity->fitsIntoLong(FALSE)) { + if (fDecimalQuantity->fitsInInt()) { fType = kLong; fValue.fInt64 = fDecimalNum->getLong(); } else @@ -794,7 +794,7 @@ Formattable::setDecimalNumber(StringPiece numberString, UErrorCode &status) { } dispose(); - DecimalQuantity* dq = new DecimalQuantity(); + auto* dq = new DecimalQuantity(); dq->setToDecNumber(numberString, status); adoptDecimalQuantity(dq); diff --git a/icu4c/source/i18n/number_currencysymbols.cpp b/icu4c/source/i18n/number_currencysymbols.cpp index 051212fb6b9..8f05da78c4e 100644 --- a/icu4c/source/i18n/number_currencysymbols.cpp +++ b/icu4c/source/i18n/number_currencysymbols.cpp @@ -61,7 +61,7 @@ UnicodeString CurrencySymbols::loadSymbol(UCurrNameStyle selector, UErrorCode& s &ignoredIsChoiceFormatFillIn, &symbolLen, &status); - // Readonly-aliasing char16_t* constructor: + // Readonly-aliasing char16_t* constructor, which points to a resource bundle: return UnicodeString(TRUE, symbol, symbolLen); } @@ -69,8 +69,9 @@ UnicodeString CurrencySymbols::getIntlCurrencySymbol(UErrorCode&) const { if (!fIntlCurrencySymbol.isBogus()) { return fIntlCurrencySymbol; } - // Readonly-aliasing char16_t* constructor: - return UnicodeString(TRUE, fCurrency.getISOCurrency(), 3); + // Note: Not safe to use readonly-aliasing constructor here because the buffer belongs to this object, + // which could be destructed or moved during the lifetime of the return value. + return UnicodeString(fCurrency.getISOCurrency(), 3); } UnicodeString CurrencySymbols::getPluralName(StandardPlural::Form plural, UErrorCode& status) const { @@ -83,7 +84,7 @@ UnicodeString CurrencySymbols::getPluralName(StandardPlural::Form plural, UError StandardPlural::getKeyword(plural), &symbolLen, &status); - // Readonly-aliasing char16_t* constructor: + // Readonly-aliasing char16_t* constructor, which points to a resource bundle: return UnicodeString(TRUE, symbol, symbolLen); } diff --git a/icu4c/source/i18n/number_decimalquantity.cpp b/icu4c/source/i18n/number_decimalquantity.cpp index 172dd23f428..538cf81f640 100644 --- a/icu4c/source/i18n/number_decimalquantity.cpp +++ b/icu4c/source/i18n/number_decimalquantity.cpp @@ -56,6 +56,9 @@ void stringToDecNumber(StringPiece n, DecNumberWithStorage &dn, UErrorCode& stat // Check for invalid syntax and set the corresponding error code. if ((set.status & DEC_Conversion_syntax) != 0) { status = U_DECIMAL_NUMBER_SYNTAX_ERROR; + } else if (set.status != 0) { + // Not a syntax error, but some other error, like an exponent that is too large. + status = U_UNSUPPORTED_ERROR; } } @@ -535,12 +538,26 @@ double DecimalQuantity::toDouble() const { if (_scale >= 0) { // 1e22 is the largest exact double. int32_t i = _scale; - for (; i >= 22; i -= 22) result *= 1e22; + for (; i >= 22; i -= 22) { + result *= 1e22; + if (uprv_isInfinite(result)) { + // Further multiplications will not be productive. + i = 0; + break; + } + } result *= DOUBLE_MULTIPLIERS[i]; } else { // 1e22 is the largest exact double. int32_t i = _scale; - for (; i <= -22; i += 22) result /= 1e22; + for (; i <= -22; i += 22) { + result /= 1e22; + if (result == 0.0) { + // Further divisions will not be productive. + i = 0; + break; + } + } result /= DOUBLE_MULTIPLIERS[-i]; } if (isNegative()) { result = -result; } @@ -1078,11 +1095,12 @@ UnicodeString DecimalQuantity::toString() const { } UnicodeString DecimalQuantity::toNumberString() const { - MaybeStackArray digits(precision + 11); + // 13 should hold both the largest and the smallest int32_t plus exponent separator and NUL + MaybeStackArray digits(precision + 13); for (int32_t i = 0; i < precision; i++) { digits[i] = getDigitPos(precision - i - 1) + '0'; } - snprintf(digits.getAlias() + precision, 11, "E%d", scale); + snprintf(digits.getAlias() + precision, 13, "E%d", scale); return UnicodeString(digits.getAlias(), -1, US_INV); } diff --git a/icu4c/source/i18n/numparse_currency.cpp b/icu4c/source/i18n/numparse_currency.cpp index 504df74e369..6dee3d28d4e 100644 --- a/icu4c/source/i18n/numparse_currency.cpp +++ b/icu4c/source/i18n/numparse_currency.cpp @@ -99,7 +99,8 @@ bool CurrencyCustomMatcher::match(StringSegment& segment, ParsedNumber& result, } bool CurrencyCustomMatcher::smokeTest(const StringSegment& segment) const { - return segment.startsWith(fCurrency1) || segment.startsWith(fCurrency2); + return segment.startsWith(fCurrency1) + || segment.startsWith(fCurrency2); } UnicodeString CurrencyCustomMatcher::toString() const { diff --git a/icu4c/source/i18n/numparse_impl.cpp b/icu4c/source/i18n/numparse_impl.cpp index d8338ae0cf9..c528d8efa82 100644 --- a/icu4c/source/i18n/numparse_impl.cpp +++ b/icu4c/source/i18n/numparse_impl.cpp @@ -223,7 +223,7 @@ void NumberParserImpl::parse(const UnicodeString& input, int32_t start, bool gre UErrorCode& status) const { U_ASSERT(fFrozen); // TODO: Check start >= 0 and start < input.length() - StringSegment segment(input, fParseFlags); + StringSegment segment(input, 0 != (fParseFlags & PARSE_FLAG_IGNORE_CASE)); segment.adjustOffset(start); if (greedy) { parseGreedyRecursive(segment, result, status); diff --git a/icu4c/source/test/intltest/numbertest_parse.cpp b/icu4c/source/test/intltest/numbertest_parse.cpp index d97141a489a..11d6d64cdf7 100644 --- a/icu4c/source/test/intltest/numbertest_parse.cpp +++ b/icu4c/source/test/intltest/numbertest_parse.cpp @@ -201,7 +201,7 @@ void NumberParserTest::testSeriesMatcher() { for (auto& cas : cases) { UnicodeString input(cas.input); - StringSegment segment(input, 0); + StringSegment segment(input, false); ParsedNumber result; bool actualMaybeMore = series.match(segment, result, status); int actualOffset = segment.getOffset(); @@ -242,7 +242,7 @@ void NumberParserTest::testCurrencyAnyMatcher() { for (auto& cas : cases) { UnicodeString input(cas.input); - StringSegment segment(input, 0); + StringSegment segment(input, false); ParsedNumber result; matcher.match(segment, result, status); assertEquals("Parsing " + input, cas.expectedCurrencyCode, result.currencyCode); @@ -293,7 +293,7 @@ void NumberParserTest::testAffixPatternMatcher() { assertEquals(affixPattern + " " + cas.exactMatch, cas.expectedMatcherLength, matcher.length()); // Check that the matcher works on a sample string - StringSegment segment(sampleParseableString, 0); + StringSegment segment(sampleParseableString, false); ParsedNumber result; matcher.match(segment, result, status); assertEquals(affixPattern + " " + cas.exactMatch, sampleParseableString.length(), result.charEnd); diff --git a/icu4c/source/test/intltest/numbertest_stringsegment.cpp b/icu4c/source/test/intltest/numbertest_stringsegment.cpp index 6c11df8f96b..b174828e1f0 100644 --- a/icu4c/source/test/intltest/numbertest_stringsegment.cpp +++ b/icu4c/source/test/intltest/numbertest_stringsegment.cpp @@ -24,7 +24,7 @@ void StringSegmentTest::runIndexedTest(int32_t index, UBool exec, const char*&na } void StringSegmentTest::testOffset() { - StringSegment segment(SAMPLE_STRING, 0); + StringSegment segment(SAMPLE_STRING, false); assertEquals("Initial Offset", 0, segment.getOffset()); segment.adjustOffset(3); assertEquals("Adjust A", 3, segment.getOffset()); @@ -35,7 +35,7 @@ void StringSegmentTest::testOffset() { } void StringSegmentTest::testLength() { - StringSegment segment(SAMPLE_STRING, 0); + StringSegment segment(SAMPLE_STRING, false); assertEquals("Initial length", 11, segment.length()); segment.adjustOffset(3); assertEquals("Adjust", 8, segment.length()); @@ -48,7 +48,7 @@ void StringSegmentTest::testLength() { } void StringSegmentTest::testCharAt() { - StringSegment segment(SAMPLE_STRING, 0); + StringSegment segment(SAMPLE_STRING, false); assertEquals("Initial", SAMPLE_STRING, segment.toUnicodeString()); assertEquals("Initial", SAMPLE_STRING, segment.toTempUnicodeString()); segment.adjustOffset(3); @@ -60,7 +60,7 @@ void StringSegmentTest::testCharAt() { } void StringSegmentTest::testGetCodePoint() { - StringSegment segment(SAMPLE_STRING, 0); + StringSegment segment(SAMPLE_STRING, false); assertEquals("Double-width code point", 0x1F4FB, segment.getCodePoint()); segment.setLength(1); assertEquals("Inalid A", -1, segment.getCodePoint()); @@ -72,7 +72,7 @@ void StringSegmentTest::testGetCodePoint() { } void StringSegmentTest::testCommonPrefixLength() { - StringSegment segment(SAMPLE_STRING, 0); + StringSegment segment(SAMPLE_STRING, false); assertEquals("", 11, segment.getCommonPrefixLength(SAMPLE_STRING)); assertEquals("", 4, segment.getCommonPrefixLength(u"📻 r")); assertEquals("", 3, segment.getCommonPrefixLength(u"📻 x")); diff --git a/icu4c/source/test/intltest/numfmtst.cpp b/icu4c/source/test/intltest/numfmtst.cpp index 29d9379173a..ee638ea36aa 100644 --- a/icu4c/source/test/intltest/numfmtst.cpp +++ b/icu4c/source/test/intltest/numfmtst.cpp @@ -105,7 +105,7 @@ static DecimalQuantity &strToDigitList( } CharString formatValue; formatValue.appendInvariantChars(str, status); - digitList.setToDecNumber(StringPiece(formatValue.data()), status); + digitList.setToDecNumber({formatValue.data(), formatValue.length()}, status); return digitList; } @@ -332,7 +332,9 @@ UBool NumberFormatTestDataDriven::isFormatPass( } } double doubleVal = digitList.toDouble(); - { + DecimalQuantity doubleCheck; + doubleCheck.setToDouble(doubleVal); + if (digitList == doubleCheck) { // skip cases where the double does not round-trip UnicodeString appendTo; format(*fmtPtr, doubleVal, appendTo, status); if (U_FAILURE(status)) { @@ -345,7 +347,7 @@ UBool NumberFormatTestDataDriven::isFormatPass( return FALSE; } } - if (!uprv_isNaN(doubleVal) && !uprv_isInfinite(doubleVal) && doubleVal == uprv_floor(doubleVal)) { + if (!uprv_isNaN(doubleVal) && !uprv_isInfinite(doubleVal) && digitList.fitsInLong()) { int64_t intVal = digitList.toLong(); { UnicodeString appendTo; @@ -423,40 +425,55 @@ UBool NumberFormatTestDataDriven::isParsePass( appendErrorMessage = appendErrorMessage + ppos.getErrorIndex(); return FALSE; } - UnicodeString resultStr(UnicodeString::fromUTF8(result.getDecimalNumber(status))); if (tuple.output == "fail") { - appendErrorMessage.append(UnicodeString("Parse succeeded: ") + resultStr + ", but was expected to fail."); + appendErrorMessage.append(UnicodeString("Parse succeeded: ") + result.getDouble() + ", but was expected to fail."); return TRUE; // TRUE because failure handling is in the test suite } if (tuple.output == "NaN") { if (!uprv_isNaN(result.getDouble())) { - appendErrorMessage.append("Expected NaN, but got: " + resultStr); + appendErrorMessage.append(UnicodeString("Expected NaN, but got: ") + result.getDouble()); return FALSE; } return TRUE; } else if (tuple.output == "Inf") { if (!uprv_isInfinite(result.getDouble()) || result.getDouble() < 0) { - appendErrorMessage.append("Expected Inf, but got: " + resultStr); + appendErrorMessage.append(UnicodeString("Expected Inf, but got: ") + result.getDouble()); return FALSE; } return TRUE; } else if (tuple.output == "-Inf") { if (!uprv_isInfinite(result.getDouble()) || result.getDouble() > 0) { - appendErrorMessage.append("Expected -Inf, but got: " + resultStr); + appendErrorMessage.append(UnicodeString("Expected -Inf, but got: ") + result.getDouble()); + return FALSE; + } + return TRUE; + } else if (tuple.output == "-0.0") { + if (std::signbit(result.getDouble()) == 0 || result.getDouble() != 0) { + appendErrorMessage.append(UnicodeString("Expected -0.0, but got: ") + result.getDouble()); return FALSE; } return TRUE; } - DecimalQuantity expected; - strToDigitList(tuple.output, expected, status); + // All other cases parse to a DecimalQuantity, not a double. + + DecimalQuantity expectedQuantity; + strToDigitList(tuple.output, expectedQuantity, status); + UnicodeString expectedString = expectedQuantity.toNumberString(); if (U_FAILURE(status)) { - appendErrorMessage.append("Error parsing."); - return FALSE; + appendErrorMessage.append("[Error parsing decnumber] "); + // If this happens, assume that tuple.output is exactly the same format as + // DecimalQuantity.toNumberString() + expectedString = tuple.output; + status = U_ZERO_ERROR; } - if (expected != *result.getDecimalQuantity()) { - appendErrorMessage.append(UnicodeString("Expected: ") + tuple.output + ", but got: " + resultStr + " (" + ppos.getIndex() + ":" + ppos.getErrorIndex() + ")"); + UnicodeString actualString = result.getDecimalQuantity()->toNumberString(); + if (expectedString != actualString) { + appendErrorMessage.append( + UnicodeString("Expected: ") + tuple.output + " (i.e., " + expectedString + "), but got: " + + actualString + " (" + ppos.getIndex() + ":" + ppos.getErrorIndex() + ")"); return FALSE; } + return TRUE; } @@ -485,22 +502,30 @@ UBool NumberFormatTestDataDriven::isParseCurrencyPass( return FALSE; } UnicodeString currStr(currAmt->getISOCurrency()); - Formattable resultFormattable(currAmt->getNumber()); - UnicodeString resultStr(UnicodeString::fromUTF8(resultFormattable.getDecimalNumber(status))); + U_ASSERT(currAmt->getNumber().getDecimalQuantity() != nullptr); // no doubles in currency tests + UnicodeString resultStr = currAmt->getNumber().getDecimalQuantity()->toNumberString(); if (tuple.output == "fail") { appendErrorMessage.append(UnicodeString("Parse succeeded: ") + resultStr + ", but was expected to fail."); return TRUE; // TRUE because failure handling is in the test suite } - DecimalQuantity expected; - strToDigitList(tuple.output, expected, status); + + DecimalQuantity expectedQuantity; + strToDigitList(tuple.output, expectedQuantity, status); + UnicodeString expectedString = expectedQuantity.toNumberString(); if (U_FAILURE(status)) { - appendErrorMessage.append("Error parsing."); - return FALSE; + appendErrorMessage.append("Error parsing decnumber"); + // If this happens, assume that tuple.output is exactly the same format as + // DecimalQuantity.toNumberString() + expectedString = tuple.output; + status = U_ZERO_ERROR; } - if (expected != *currAmt->getNumber().getDecimalQuantity()) { - appendErrorMessage.append(UnicodeString("Expected: ") + tuple.output + ", but got: " + resultStr + " (" + ppos.getIndex() + ":" + ppos.getErrorIndex() + ")"); + if (expectedString != resultStr) { + appendErrorMessage.append( + UnicodeString("Expected: ") + tuple.output + " (i.e., " + expectedString + "), but got: " + + resultStr + " (" + ppos.getIndex() + ":" + ppos.getErrorIndex() + ")"); return FALSE; } + if (currStr != tuple.outputCurrency) { appendErrorMessage.append(UnicodeString( "Expected currency: ") + tuple.outputCurrency + ", got: " + currStr + ". "); diff --git a/icu4c/source/test/testdata/numberformattestspecification.txt b/icu4c/source/test/testdata/numberformattestspecification.txt index 8aa1bb44ea2..509da32c8d8 100644 --- a/icu4c/source/test/testdata/numberformattestspecification.txt +++ b/icu4c/source/test/testdata/numberformattestspecification.txt @@ -548,7 +548,6 @@ set locale en begin currency currencyUsage toPattern breaks // These work in J, but it prepends an extra hash sign to the pattern. -// C does not print the currency rounding information in the pattern. // K does not support this feature. USD standard 0.00 JK CHF standard 0.00 JK @@ -1409,6 +1408,7 @@ nan 0 NaN JK test parse infinity and scientific notation overflow set locale en +set lenient 1 begin parse output breaks NaN NaN K @@ -1420,12 +1420,12 @@ NaN NaN K -1E-99999999999999 -0.0 1E2147483648 Inf K 1E2147483647 Inf K -1E2147483646 1E2147483646 C +1E2147483646 1E2147483646 1E-2147483649 0 1E-2147483648 0 -// P returns zero here -1E-2147483647 1E-2147483647 P -1E-2147483646 1E-2147483646 C +// C and P return zero here +1E-2147483647 1E-2147483647 CP +1E-2147483646 1E-2147483646 test format push limits set locale en @@ -1433,20 +1433,19 @@ set minFractionDigits 2 set roundingMode halfDown begin maxFractionDigits format output breaks -// C has trouble formatting too many digits (#11318) -100 987654321987654321 987654321987654321.00 C -100 987654321.987654321 987654321.987654321 C -100 9999999999999.9950000000001 9999999999999.9950000000001 C +100 987654321987654321 987654321987654321.00 +100 987654321.987654321 987654321.987654321 +100 9999999999999.9950000000001 9999999999999.9950000000001 2 9999999999999.9950000000001 10000000000000.00 2 9999999.99499999 9999999.99 // K doesn't support halfDown rounding mode? 2 9999999.995 9999999.99 K 2 9999999.99500001 10000000.00 -100 56565656565656565656565656565656565656565656565656565656565656 56565656565656565656565656565656565656565656565656565656565656.00 C -100 454545454545454545454545454545.454545454545454545454545454545 454545454545454545454545454545.454545454545454545454545454545 C +100 56565656565656565656565656565656565656565656565656565656565656 56565656565656565656565656565656565656565656565656565656565656.00 +100 454545454545454545454545454545.454545454545454545454545454545 454545454545454545454545454545.454545454545454545454545454545 100 0.0000000000000000000123 0.0000000000000000000123 -100 -78787878787878787878787878787878 -78787878787878787878787878787878.00 C -100 -8989898989898989898989.8989898989898989 -8989898989898989898989.8989898989898989 C +100 -78787878787878787878787878787878 -78787878787878787878787878787878.00 +100 -8989898989898989898989.8989898989898989 -8989898989898989898989.8989898989898989 test ticket 11230 set locale en diff --git a/icu4j/main/classes/core/src/com/ibm/icu/impl/number/DecimalQuantity_AbstractBCD.java b/icu4j/main/classes/core/src/com/ibm/icu/impl/number/DecimalQuantity_AbstractBCD.java index 9bd291d9cff..ebf91c710fc 100644 --- a/icu4j/main/classes/core/src/com/ibm/icu/impl/number/DecimalQuantity_AbstractBCD.java +++ b/icu4j/main/classes/core/src/com/ibm/icu/impl/number/DecimalQuantity_AbstractBCD.java @@ -646,14 +646,26 @@ public abstract class DecimalQuantity_AbstractBCD implements DecimalQuantity { if (_scale >= 0) { // 1e22 is the largest exact double. int i = _scale; - for (; i >= 22; i -= 22) + for (; i >= 22; i -= 22) { result *= 1e22; + if (Double.isInfinite(result)) { + // Further multiplications will not be productive. + i = 0; + break; + } + } result *= DOUBLE_MULTIPLIERS[i]; } else { // 1e22 is the largest exact double. int i = _scale; - for (; i <= -22; i += 22) + for (; i <= -22; i += 22) { result /= 1e22; + if (result == 0.0) { + // Further divisions will not be productive. + i = 0; + break; + } + } result /= DOUBLE_MULTIPLIERS[-i]; } if (isNegative()) diff --git a/icu4j/main/classes/core/src/com/ibm/icu/impl/number/parse/ParsedNumber.java b/icu4j/main/classes/core/src/com/ibm/icu/impl/number/parse/ParsedNumber.java index 7dd5ac1c691..9b472eed2d8 100644 --- a/icu4j/main/classes/core/src/com/ibm/icu/impl/number/parse/ParsedNumber.java +++ b/icu4j/main/classes/core/src/com/ibm/icu/impl/number/parse/ParsedNumber.java @@ -161,6 +161,7 @@ public class ParsedNumber { d = d.negate(); } // Special case: MIN_LONG + // TODO: It is supported in quantity.toLong() if quantity had the negative flag. if (d.compareTo(BigDecimal.valueOf(Long.MIN_VALUE)) == 0 && !forceBigDecimal) { return Long.MIN_VALUE; } diff --git a/icu4j/main/tests/core/src/com/ibm/icu/dev/test/format/NumberFormatDataDrivenTest.java b/icu4j/main/tests/core/src/com/ibm/icu/dev/test/format/NumberFormatDataDrivenTest.java index 400ab06c4e1..d8be411eb9d 100644 --- a/icu4j/main/tests/core/src/com/ibm/icu/dev/test/format/NumberFormatDataDrivenTest.java +++ b/icu4j/main/tests/core/src/com/ibm/icu/dev/test/format/NumberFormatDataDrivenTest.java @@ -11,11 +11,11 @@ import org.junit.Test; import com.ibm.icu.dev.test.TestUtil; import com.ibm.icu.dev.text.DecimalFormat_ICU58; import com.ibm.icu.impl.number.DecimalFormatProperties; +import com.ibm.icu.impl.number.DecimalFormatProperties.ParseMode; import com.ibm.icu.impl.number.Padder.PadPosition; import com.ibm.icu.impl.number.PatternStringParser; import com.ibm.icu.impl.number.PatternStringUtils; import com.ibm.icu.impl.number.parse.NumberParserImpl; -import com.ibm.icu.impl.number.parse.NumberParserImpl.ParseMode; import com.ibm.icu.number.LocalizedNumberFormatter; import com.ibm.icu.number.NumberFormatter; import com.ibm.icu.text.DecimalFormat;