Use `absl::from_chars` instead of `strtod`
This CL switches Tint to use `absl::from_chars` instead of `strtod` in
order to have locale independent parsing of floats.
Bug: tint:1686
Change-Id: Icb3d9a40928a1cb154b32629f7e39173d80e4563
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/122780
Reviewed-by: David Neto <dneto@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: Dan Sinclair <dsinclair@chromium.org>
diff --git a/src/tint/BUILD.gn b/src/tint/BUILD.gn
index 89eb6eef..6479684 100644
--- a/src/tint/BUILD.gn
+++ b/src/tint/BUILD.gn
@@ -14,6 +14,7 @@
import("//build_overrides/build.gni")
+import("../../scripts/dawn_overrides_with_defaults.gni")
import("../../tint_overrides_with_defaults.gni")
if (tint_build_unittests) {
@@ -92,6 +93,17 @@
}
}
+group("abseil") {
+ # When build_with_chromium=true we need to include "//third_party/abseil-cpp:absl" while
+ # it's beneficial to be more specific with standalone Dawn, especially when it comes to
+ # including it as a dependency in other projects (such as Skia).
+ if (build_with_chromium) {
+ public_deps = [ "$dawn_abseil_dir:absl" ]
+ } else {
+ public_deps = [ "${dawn_root}/third_party/gn/abseil-cpp:strings" ]
+ }
+}
+
###############################################################################
# Helper library for IO operations
# Only to be used by tests and sample executable
@@ -953,6 +965,7 @@
]
deps = [
+ ":abseil",
":libtint_ast_src",
":libtint_base_src",
":libtint_builtins_src",
diff --git a/src/tint/CMakeLists.txt b/src/tint/CMakeLists.txt
index 1d31896..6a8f503 100644
--- a/src/tint/CMakeLists.txt
+++ b/src/tint/CMakeLists.txt
@@ -760,7 +760,7 @@
## Tint library
add_library(libtint ${TINT_LIB_SRCS})
tint_default_compile_options(libtint)
-target_link_libraries(libtint tint_diagnostic_utils)
+target_link_libraries(libtint tint_diagnostic_utils absl_strings)
if (${TINT_SYMBOL_STORE_DEBUG_NAME})
target_compile_definitions(libtint PUBLIC "TINT_SYMBOL_STORE_DEBUG_NAME=1")
endif()
@@ -770,7 +770,7 @@
# Tint library with fuzzer instrumentation
add_library(libtint-fuzz ${TINT_LIB_SRCS})
tint_default_compile_options(libtint-fuzz)
- target_link_libraries(libtint-fuzz tint_diagnostic_utils)
+ target_link_libraries(libtint-fuzz tint_diagnostic_utils absl_strings)
if (${COMPILER_IS_LIKE_GNU})
target_compile_options(libtint-fuzz PRIVATE -fvisibility=hidden)
endif()
diff --git a/src/tint/reader/wgsl/lexer.cc b/src/tint/reader/wgsl/lexer.cc
index 28fb135..91c2993 100644
--- a/src/tint/reader/wgsl/lexer.cc
+++ b/src/tint/reader/wgsl/lexer.cc
@@ -25,6 +25,7 @@
#include <type_traits>
#include <utility>
+#include "absl/strings/charconv.h"
#include "src/tint/debug.h"
#include "src/tint/number.h"
#include "src/tint/text/unicode.h"
@@ -352,9 +353,11 @@
// Parse the exponent if one exists
bool has_exponent = false;
+ bool negative_exponent = false;
if (end < length() && (matches(end, 'e') || matches(end, 'E'))) {
end++;
if (end < length() && (matches(end, '+') || matches(end, '-'))) {
+ negative_exponent = matches(end, '-');
end++;
}
@@ -374,10 +377,8 @@
bool has_f_suffix = false;
bool has_h_suffix = false;
if (end < length() && matches(end, 'f')) {
- end++;
has_f_suffix = true;
} else if (end < length() && matches(end, 'h')) {
- end++;
has_h_suffix = true;
}
@@ -386,29 +387,51 @@
return {};
}
- advance(end - start);
- end_source(source);
+ // Note, the `at` method will return a static `0` if the provided position is >= length. We
+ // actually need the end pointer to point to the correct memory location to use `from_chars`.
+ // So, handle the case where we point past the length specially.
+ auto* end_ptr = &at(end);
+ if (end >= length()) {
+ end_ptr = &at(length() - 1) + 1;
+ }
- double value = std::strtod(&at(start), nullptr);
+ double value = 0;
+ auto ret = absl::from_chars(&at(start), end_ptr, value);
+ bool overflow = ret.ec != std::errc();
+
+ // The provided value did not fit in a double and has a negative exponent, so treat it as a 0.
+ if (negative_exponent && ret.ec == std::errc::result_out_of_range) {
+ overflow = false;
+ value = 0.0;
+ }
+
+ TINT_ASSERT(Reader, end_ptr == ret.ptr);
+ advance(end - start);
if (has_f_suffix) {
- if (auto f = CheckedConvert<f32>(AFloat(value))) {
+ auto f = CheckedConvert<f32>(AFloat(value));
+ if (!overflow && f) {
+ advance(1);
+ end_source(source);
return {Token::Type::kFloatLiteral_F, source, static_cast<double>(f.Get())};
- } else {
- return {Token::Type::kError, source, "value cannot be represented as 'f32'"};
}
+ return {Token::Type::kError, source, "value cannot be represented as 'f32'"};
}
if (has_h_suffix) {
- if (auto f = CheckedConvert<f16>(AFloat(value))) {
+ auto f = CheckedConvert<f16>(AFloat(value));
+ if (!overflow && f) {
+ advance(1);
+ end_source(source);
return {Token::Type::kFloatLiteral_H, source, static_cast<double>(f.Get())};
- } else {
- return {Token::Type::kError, source, "value cannot be represented as 'f16'"};
}
+ return {Token::Type::kError, source, "value cannot be represented as 'f16'"};
}
+ end_source(source);
+
TINT_BEGIN_DISABLE_WARNING(FLOAT_EQUAL);
- if (value == HUGE_VAL || -value == HUGE_VAL) {
+ if (overflow || value == HUGE_VAL || -value == HUGE_VAL) {
return {Token::Type::kError, source, "value cannot be represented as 'abstract-float'"};
} else {
return {Token::Type::kFloatLiteral, source, value};
diff --git a/src/tint/reader/wgsl/lexer_test.cc b/src/tint/reader/wgsl/lexer_test.cc
index 95b43a7..a244273 100644
--- a/src/tint/reader/wgsl/lexer_test.cc
+++ b/src/tint/reader/wgsl/lexer_test.cc
@@ -397,7 +397,7 @@
Lexer l(&file);
auto list = l.Lex();
- ASSERT_EQ(2u, list.size());
+ ASSERT_EQ(2u, list.size()) << "Got: " << list[0].to_str();
{
auto& t = list[0];
