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>
This commit is contained in:
dan sinclair 2023-03-07 18:20:23 +00:00 committed by Dawn LUCI CQ
parent 64196ad60a
commit 2fe7f19026
4 changed files with 51 additions and 15 deletions

View File

@ -14,6 +14,7 @@
import("//build_overrides/build.gni") import("//build_overrides/build.gni")
import("../../scripts/dawn_overrides_with_defaults.gni")
import("../../tint_overrides_with_defaults.gni") import("../../tint_overrides_with_defaults.gni")
if (tint_build_unittests) { if (tint_build_unittests) {
@ -92,6 +93,17 @@ config("tint_config") {
} }
} }
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 # Helper library for IO operations
# Only to be used by tests and sample executable # Only to be used by tests and sample executable
@ -953,6 +965,7 @@ libtint_source_set("libtint_wgsl_reader_src") {
] ]
deps = [ deps = [
":abseil",
":libtint_ast_src", ":libtint_ast_src",
":libtint_base_src", ":libtint_base_src",
":libtint_builtins_src", ":libtint_builtins_src",

View File

@ -760,7 +760,7 @@ target_link_libraries(tint_val tint_utils_io)
## Tint library ## Tint library
add_library(libtint ${TINT_LIB_SRCS}) add_library(libtint ${TINT_LIB_SRCS})
tint_default_compile_options(libtint) 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}) if (${TINT_SYMBOL_STORE_DEBUG_NAME})
target_compile_definitions(libtint PUBLIC "TINT_SYMBOL_STORE_DEBUG_NAME=1") target_compile_definitions(libtint PUBLIC "TINT_SYMBOL_STORE_DEBUG_NAME=1")
endif() endif()
@ -770,7 +770,7 @@ if (${TINT_BUILD_FUZZERS})
# Tint library with fuzzer instrumentation # Tint library with fuzzer instrumentation
add_library(libtint-fuzz ${TINT_LIB_SRCS}) add_library(libtint-fuzz ${TINT_LIB_SRCS})
tint_default_compile_options(libtint-fuzz) 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}) if (${COMPILER_IS_LIKE_GNU})
target_compile_options(libtint-fuzz PRIVATE -fvisibility=hidden) target_compile_options(libtint-fuzz PRIVATE -fvisibility=hidden)
endif() endif()

View File

@ -25,6 +25,7 @@
#include <type_traits> #include <type_traits>
#include <utility> #include <utility>
#include "absl/strings/charconv.h"
#include "src/tint/debug.h" #include "src/tint/debug.h"
#include "src/tint/number.h" #include "src/tint/number.h"
#include "src/tint/text/unicode.h" #include "src/tint/text/unicode.h"
@ -352,9 +353,11 @@ Token Lexer::try_float() {
// Parse the exponent if one exists // Parse the exponent if one exists
bool has_exponent = false; bool has_exponent = false;
bool negative_exponent = false;
if (end < length() && (matches(end, 'e') || matches(end, 'E'))) { if (end < length() && (matches(end, 'e') || matches(end, 'E'))) {
end++; end++;
if (end < length() && (matches(end, '+') || matches(end, '-'))) { if (end < length() && (matches(end, '+') || matches(end, '-'))) {
negative_exponent = matches(end, '-');
end++; end++;
} }
@ -374,10 +377,8 @@ Token Lexer::try_float() {
bool has_f_suffix = false; bool has_f_suffix = false;
bool has_h_suffix = false; bool has_h_suffix = false;
if (end < length() && matches(end, 'f')) { if (end < length() && matches(end, 'f')) {
end++;
has_f_suffix = true; has_f_suffix = true;
} else if (end < length() && matches(end, 'h')) { } else if (end < length() && matches(end, 'h')) {
end++;
has_h_suffix = true; has_h_suffix = true;
} }
@ -386,29 +387,51 @@ Token Lexer::try_float() {
return {}; return {};
} }
advance(end - start); // Note, the `at` method will return a static `0` if the provided position is >= length. We
end_source(source); // 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 (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())}; 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 (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())}; 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); 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'"}; return {Token::Type::kError, source, "value cannot be represented as 'abstract-float'"};
} else { } else {
return {Token::Type::kFloatLiteral, source, value}; return {Token::Type::kFloatLiteral, source, value};

View File

@ -397,7 +397,7 @@ TEST_P(FloatTest, Parse) {
Lexer l(&file); Lexer l(&file);
auto list = l.Lex(); auto list = l.Lex();
ASSERT_EQ(2u, list.size()); ASSERT_EQ(2u, list.size()) << "Got: " << list[0].to_str();
{ {
auto& t = list[0]; auto& t = list[0];