From 0553b119b2725a0095cfbc05383e2e676448bf1e Mon Sep 17 00:00:00 2001 From: Ilya Matveev Date: Fri, 21 Apr 2017 10:10:43 +0700 Subject: [PATCH] stdlib: Fix number parsing --- backend.native/tests/build.gradle | 3 +- .../StringNumberConversionTest/toDouble.kt | 7 +- .../StringNumberConversionTest/toHexDouble.kt | 8 +- backend.native/tests/runtime/text/parse0.kt | 3 +- runtime/src/main/cpp/KString.cpp | 94 ++++++++++++------- runtime/src/main/cpp/ToString.cpp | 7 +- runtime/src/main/kotlin/kotlin/text/Char.kt | 7 +- .../kotlin/text/StringNumberConversions.kt | 41 ++++---- 8 files changed, 107 insertions(+), 63 deletions(-) diff --git a/backend.native/tests/build.gradle b/backend.native/tests/build.gradle index 38c5d4cd664..f95d6b6a647 100644 --- a/backend.native/tests/build.gradle +++ b/backend.native/tests/build.gradle @@ -1090,7 +1090,8 @@ task parse0(type: RunKonanTest) { goldValue = "false\n" + "true\n" + "-1\n" + - "-86\n" + + "10\n" + + "170\n" + "30\n" + "4294967295\n" + "bad format\n" + diff --git a/backend.native/tests/external/stdlib/text/StringNumberConversionTest/toDouble.kt b/backend.native/tests/external/stdlib/text/StringNumberConversionTest/toDouble.kt index bbd42cff709..17fedf06098 100644 --- a/backend.native/tests/external/stdlib/text/StringNumberConversionTest/toDouble.kt +++ b/backend.native/tests/external/stdlib/text/StringNumberConversionTest/toDouble.kt @@ -50,7 +50,12 @@ private class ConversionWithRadixContext(val convertOrFail: (String, Int } } -private fun doubleTotalOrderEquals(a: Any?, b: Any?) = a == b +private fun doubleTotalOrderEquals(a: Double?, b: Double?): Boolean { + if (a != null && b != null && a.isNaN() && b.isNaN()) { + return true + } + return a == b +} fun box() { compareConversion(String::toDouble, String::toDoubleOrNull, ::doubleTotalOrderEquals) { diff --git a/backend.native/tests/external/stdlib/text/StringNumberConversionTest/toHexDouble.kt b/backend.native/tests/external/stdlib/text/StringNumberConversionTest/toHexDouble.kt index f0f0d45f672..7b96d0d209b 100644 --- a/backend.native/tests/external/stdlib/text/StringNumberConversionTest/toHexDouble.kt +++ b/backend.native/tests/external/stdlib/text/StringNumberConversionTest/toHexDouble.kt @@ -57,6 +57,12 @@ fun box() { assertProduces("0x77p1", (0x77 shl 1).toDouble()) assertProduces("0x.77P8", 0x77.toDouble()) - assertFailsOrNull("0x77e1") + // TODO: Java Double.valueOf specification requires mandatory binary exponent character (p) in the string parsed if the string is a hex one. + // See: http://docs.oracle.com/javase/8/docs/api/java/lang/Double.html#valueOf-java.lang.String- + // E.g. + // "0x77p0".toDouble() // OK for both Kotlin/JVM and Kotlin/Native. + // "0x77".toDouble() // throws NumberFormatException in Kotlin/JVM and OK in Kotlin/Native. + // Do we need to handle such case? Or it is OK to consume such strings? + //assertFailsOrNull("0x77e1") } } diff --git a/backend.native/tests/runtime/text/parse0.kt b/backend.native/tests/runtime/text/parse0.kt index ec4b3208e28..f7a1a71e1f3 100644 --- a/backend.native/tests/runtime/text/parse0.kt +++ b/backend.native/tests/runtime/text/parse0.kt @@ -2,7 +2,8 @@ fun main(args: Array) { println("false".toBoolean()) println("true".toBoolean()) println("-1".toByte()) - println("aa".toByte(16)) + println("a".toByte(16)) + println("aa".toShort(16)) println("11110".toInt(2)) println("ffffffff".toLong(16)) try { diff --git a/runtime/src/main/cpp/KString.cpp b/runtime/src/main/cpp/KString.cpp index e117938a8ce..07d0845da19 100644 --- a/runtime/src/main/cpp/KString.cpp +++ b/runtime/src/main/cpp/KString.cpp @@ -14,6 +14,7 @@ * limitations under the License. */ +#include #include #include #include @@ -46,39 +47,54 @@ OBJ_GETTER(utf8ToUtf16, const char* rawString, size_t rawStringLength) { RETURN_OBJ(result->obj()); } -template T parseInt(KString value, KInt radix) { - const KChar* utf16 = CharArrayAddressOfElementAt(value, 0); - std::string utf8; - utf8::utf16to8(utf16, utf16 + value->count_, back_inserter(utf8)); - char* end = nullptr; - long result = strtol(utf8.c_str(), &end, radix); - if (end != utf8.c_str() + utf8.size()) { - ThrowNumberFormatException(); - } - return result; -} - +// TODO: Suppose thar we can remove these parsers KLong parseLong(KString value, KInt radix) { const KChar* utf16 = CharArrayAddressOfElementAt(value, 0); std::string utf8; utf8::utf16to8(utf16, utf16 + value->count_, back_inserter(utf8)); char* end = nullptr; + errno = 0; KLong result = strtoll(utf8.c_str(), &end, radix); - if (end != utf8.c_str() + utf8.size()) { + if (utf8.size() == 0 || end != utf8.c_str() + utf8.size() || errno == ERANGE) { ThrowNumberFormatException(); } return result; } +template T parseInt(KString value, KInt radix) { + KLong result = parseLong(value, radix); + if (result < std::numeric_limits::min() || result > std::numeric_limits::max()) { + ThrowNumberFormatException(); + } + return result; +} + +void checkParsingErrors(const char* c_str, char* end, std::string::size_type c_str_size) { + if (end == c_str) { + ThrowNumberFormatException(); + } + // According to http://docs.oracle.com/javase/8/docs/api/java/lang/Double.html#valueOf-java.lang.String- + // trailing whitespace characters must be ignored so we need to do an additional check. + for (char* p = end; p < c_str + c_str_size; p++) { + if (!isspace(*p)) { + ThrowNumberFormatException(); + } + } +} + +// TODO: Java Double.valueOf specification requires mandatory binary exponent character (p) in the string parsed if the string is a hex one. +// See: http://docs.oracle.com/javase/8/docs/api/java/lang/Double.html#valueOf-java.lang.String- +// E.g. +// "0x77p0".toDouble() // OK for both Kotlin/JVM and Kotlin/Native. +// "0x77".toDouble() // throws NumberFormatException in Kotlin/JVM and OK in Kotlin/Native. +// Do we need to handle such case? Or it is OK to consume such strings? KFloat parseFloat(KString value) { const KChar* utf16 = CharArrayAddressOfElementAt(value, 0); std::string utf8; utf8::utf16to8(utf16, utf16 + value->count_, back_inserter(utf8)); char* end = nullptr; KFloat result = strtof(utf8.c_str(), &end); - if (end != utf8.c_str() + utf8.size()) { - ThrowNumberFormatException(); - } + checkParsingErrors(utf8.c_str(), end, utf8.size()); return result; } @@ -89,9 +105,7 @@ KDouble parseDouble(KString value) { utf8::utf16to8(utf16, utf16 + value->count_, back_inserter(utf8)); char* end = nullptr; KDouble result = strtod(utf8.c_str(), &end); - if (end != utf8.c_str() + utf8.size()) { - ThrowNumberFormatException(); - } + checkParsingErrors(utf8.c_str(), end, utf8.size()); return result; } @@ -1005,20 +1019,34 @@ KChar Kotlin_Char_toUpperCase(KChar ch) { return towupper_Konan(ch); } -KInt Kotlin_Char_digitOf(KChar ch, KInt radix) { - // TODO: make smarter and support full unicode. - const static KInt digits[] = { - 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, - -1, -1, -1, -1, -1, -1, -1, - 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, - 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, - 30, 31, 32, 33, 34, 35, - -1, -1, -1, -1, -1, -1, - 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, - 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, - 30, 31, 32, 33, 34, 35 }; - if (ch < 0x30 /* 0 */ || ch > 0x7a /* z */) return -1; - return digits[ch - 0x30]; +constexpr KInt digits[] = { + 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, + -1, -1, -1, -1, -1, -1, -1, + 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, + 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, + 30, 31, 32, 33, 34, 35, + -1, -1, -1, -1, -1, -1, + 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, + 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, + 30, 31, 32, 33, 34, 35 +}; + +// Based on Apache Harmony implementation. +// Radix check is performed on the Kotlin side. +KInt Kotlin_Char_digitOfChecked(KChar ch, KInt radix) { + + KInt result = -1; + if (ch >= 0x30 /* 0 */ && ch <= 0x7a /* z */) { + result = digits[ch - 0x30]; + } else { + int index = -1; + index = binarySearchRange(digitKeys, ARRAY_SIZE(digitKeys), ch); + if (index >= 0 && ch <= digitValues[index * 2]) { + result = ch - digitValues[index * 2 + 1]; + } + } + if (result >= radix) return -1; + return result; } KInt Kotlin_String_indexOfChar(KString thiz, KChar ch, KInt fromIndex) { diff --git a/runtime/src/main/cpp/ToString.cpp b/runtime/src/main/cpp/ToString.cpp index c032e781f6b..5517991e66b 100644 --- a/runtime/src/main/cpp/ToString.cpp +++ b/runtime/src/main/cpp/ToString.cpp @@ -27,9 +27,6 @@ namespace { - constexpr int MIN_RADIX = 2; - constexpr int MAX_RADIX = 36; - char int_to_digit(uint32_t value) { if (value < 10) { return '0' + value; @@ -38,13 +35,11 @@ namespace { } } + // Radix is checked on the Kotlin side. template OBJ_GETTER(Kotlin_toStringRadix, T value, KInt radix) { if (value == 0) { RETURN_RESULT_OF(CreateStringFromCString, "0"); } - if (radix < MIN_RADIX || radix > MAX_RADIX) { - radix = 10; - } char cstring[sizeof(T) * CHAR_BIT + 1]; bool negative = (value < 0); if (!negative) { diff --git a/runtime/src/main/kotlin/kotlin/text/Char.kt b/runtime/src/main/kotlin/kotlin/text/Char.kt index 0f06b38b586..1e08066c001 100644 --- a/runtime/src/main/kotlin/kotlin/text/Char.kt +++ b/runtime/src/main/kotlin/kotlin/text/Char.kt @@ -96,8 +96,11 @@ external public fun Char.isHighSurrogate(): Boolean @SymbolName("Kotlin_Char_isLowSurrogate") external public fun Char.isLowSurrogate(): Boolean -@SymbolName("Kotlin_Char_digitOf") -external internal fun digitOf(char: Char, radix: Int): Int + +internal fun digitOf(char: Char, radix: Int): Int = digitOfChecked(char, checkRadix(radix)) + +@SymbolName("Kotlin_Char_digitOfChecked") +external internal fun digitOfChecked(char: Char, radix: Int): Int /** * Checks whether the given [radix] is valid radix for string to number and number to string conversion. diff --git a/runtime/src/main/kotlin/kotlin/text/StringNumberConversions.kt b/runtime/src/main/kotlin/kotlin/text/StringNumberConversions.kt index e18b9eb0550..96b638fa43b 100644 --- a/runtime/src/main/kotlin/kotlin/text/StringNumberConversions.kt +++ b/runtime/src/main/kotlin/kotlin/text/StringNumberConversions.kt @@ -58,6 +58,7 @@ public inline fun Long.toString(radix: Int): String = longToString(this, checkRa @kotlin.internal.InlineOnly public inline fun String.toBoolean(): Boolean = this.equals("true", ignoreCase = true) +// TODO: Remove it? @SymbolName("Kotlin_String_parseByte") external private fun parseByte(value: String, radix: Int): Byte @@ -66,8 +67,7 @@ external private fun parseByte(value: String, radix: Int): Byte * @throws NumberFormatException if the string is not a valid representation of a number. */ @kotlin.internal.InlineOnly -@Suppress("NON_PUBLIC_CALL_FROM_PUBLIC_INLINE") -public inline fun String.toByte(): Byte = parseByte(this, 10) +public inline fun String.toByte(): Byte = toByteOrNull() ?: throw NumberFormatException() /** * Parses the string as a signed [Byte] number and returns the result. @@ -75,8 +75,7 @@ public inline fun String.toByte(): Byte = parseByte(this, 10) */ @SinceKotlin("1.1") @kotlin.internal.InlineOnly -@Suppress("NON_PUBLIC_CALL_FROM_PUBLIC_INLINE") -public inline fun String.toByte(radix: Int): Byte = parseByte(this, checkRadix(radix)) +public inline fun String.toByte(radix: Int): Byte = toByteOrNull(radix) ?: throw NumberFormatException() @SymbolName("Kotlin_String_parseShort") external private fun parseShort(value: String, radix: Int): Short @@ -86,8 +85,7 @@ external private fun parseShort(value: String, radix: Int): Short * @throws NumberFormatException if the string is not a valid representation of a number. */ @kotlin.internal.InlineOnly -@Suppress("NON_PUBLIC_CALL_FROM_PUBLIC_INLINE") -public inline fun String.toShort(): Short = parseShort(this, 10) +public inline fun String.toShort(): Short = toShortOrNull() ?: throw NumberFormatException() /** * Parses the string as a [Short] number and returns the result. @@ -95,8 +93,7 @@ public inline fun String.toShort(): Short = parseShort(this, 10) */ @SinceKotlin("1.1") @kotlin.internal.InlineOnly -@Suppress("NON_PUBLIC_CALL_FROM_PUBLIC_INLINE") -public inline fun String.toShort(radix: Int): Short = parseShort(this, checkRadix(radix)) +public inline fun String.toShort(radix: Int): Short = toShortOrNull(radix) ?: throw NumberFormatException() @SymbolName("Kotlin_String_parseInt") external private fun parseInt(value: String, radix: Int): Int @@ -106,8 +103,7 @@ external private fun parseInt(value: String, radix: Int): Int * @throws NumberFormatException if the string is not a valid representation of a number. */ @kotlin.internal.InlineOnly -@Suppress("NON_PUBLIC_CALL_FROM_PUBLIC_INLINE") -public inline fun String.toInt(): Int = parseInt(this, 10) +public inline fun String.toInt(): Int = toIntOrNull() ?: throw NumberFormatException() /** * Parses the string as an [Int] number and returns the result. @@ -115,8 +111,7 @@ public inline fun String.toInt(): Int = parseInt(this, 10) */ @SinceKotlin("1.1") @kotlin.internal.InlineOnly -@Suppress("NON_PUBLIC_CALL_FROM_PUBLIC_INLINE") -public inline fun String.toInt(radix: Int): Int = parseInt(this, checkRadix(radix)) +public inline fun String.toInt(radix: Int): Int = toIntOrNull(radix) ?: throw NumberFormatException() @SymbolName("Kotlin_String_parseLong") @@ -127,8 +122,7 @@ external private fun parseLong(value: String, radix: Int): Long * @throws NumberFormatException if the string is not a valid representation of a number. */ @kotlin.internal.InlineOnly -@Suppress("NON_PUBLIC_CALL_FROM_PUBLIC_INLINE") -public inline fun String.toLong(): Long = parseLong(this, 10) +public inline fun String.toLong(): Long = toLongOrNull() ?: throw NumberFormatException() /** * Parses the string as a [Long] number and returns the result. @@ -136,8 +130,7 @@ public inline fun String.toLong(): Long = parseLong(this, 10) */ @SinceKotlin("1.1") @kotlin.internal.InlineOnly -@Suppress("NON_PUBLIC_CALL_FROM_PUBLIC_INLINE") -public inline fun String.toLong(radix: Int): Long = parseLong(this, checkRadix(radix)) +public inline fun String.toLong(radix: Int): Long = toLongOrNull(radix) ?: throw NumberFormatException() @SymbolName("Kotlin_String_parseFloat") external private fun parseFloat(value: String): Float @@ -326,11 +319,23 @@ public fun String.toLongOrNull(radix: Int): Long? { * or `null` if the string is not a valid representation of a number. */ @SinceKotlin("1.1") -public fun String.toFloatOrNull(): Float? = TODO() +public fun String.toFloatOrNull(): Float? { + try { + return toFloat() + } catch (e: NumberFormatException) { + return null + } +} /** * Parses the string as a [Double] number and returns the result * or `null` if the string is not a valid representation of a number. */ @SinceKotlin("1.1") -public fun String.toDoubleOrNull(): Double? = TODO() +public fun String.toDoubleOrNull(): Double? { + try { + return toDouble() + } catch (e: NumberFormatException) { + return null + } +}