From ef702af6c8b8d12ec5695e5600f0ef4844dd73d3 Mon Sep 17 00:00:00 2001 From: Ben Clayton Date: Wed, 25 May 2022 15:24:34 +0000 Subject: [PATCH] tint/reader/wgsl: Use CheckedConvert() for lexing And simplify diagnostic messages. Bug: tint:1504 Change-Id: Ib649602a828760f434ea9c8de5c482d2a0459757 Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/91362 Reviewed-by: David Neto Commit-Queue: Ben Clayton Kokoro: Kokoro --- src/tint/reader/wgsl/lexer.cc | 122 +++++------------- src/tint/reader/wgsl/lexer_test.cc | 4 +- .../wgsl/parser_impl_const_literal_test.cc | 8 +- 3 files changed, 39 insertions(+), 95 deletions(-) diff --git a/src/tint/reader/wgsl/lexer.cc b/src/tint/reader/wgsl/lexer.cc index 20b33640bd..4730c5f1bf 100644 --- a/src/tint/reader/wgsl/lexer.cc +++ b/src/tint/reader/wgsl/lexer.cc @@ -24,6 +24,7 @@ #include #include "src/tint/debug.h" +#include "src/tint/number.h" #include "src/tint/text/unicode.h" namespace tint::reader::wgsl { @@ -80,42 +81,6 @@ uint32_t hex_value(char c) { return 0; } -/// LimitCheck is the enumerator result of check_limits(). -enum class LimitCheck { - /// The value was within the limits of the data type. - kWithinLimits, - /// The value was too small to fit within the data type. - kTooSmall, - /// The value was too large to fit within the data type. - kTooLarge, -}; - -/// Checks whether the value fits within the integer type `T` -template -LimitCheck check_limits(int64_t value) { - static_assert(std::is_integral_v, "T must be an integer"); - if (value < static_cast(std::numeric_limits::lowest())) { - return LimitCheck::kTooSmall; - } - if (value > static_cast(std::numeric_limits::max())) { - return LimitCheck::kTooLarge; - } - return LimitCheck::kWithinLimits; -} - -/// Checks whether the value fits within the floating point type `T` -template -LimitCheck check_limits(double value) { - static_assert(std::is_floating_point_v, "T must be a floating point"); - if (value < static_cast(std::numeric_limits::lowest())) { - return LimitCheck::kTooSmall; - } - if (value > static_cast(std::numeric_limits::max())) { - return LimitCheck::kTooLarge; - } - return LimitCheck::kWithinLimits; -} - } // namespace Lexer::Lexer(const Source::File* file) : file_(file), location_{1, 1} {} @@ -394,38 +359,30 @@ Token Lexer::try_float() { end_source(source); double value = strtod(&at(start), nullptr); - const double magnitude = std::abs(value); if (has_f_suffix) { - // This errors out if a non-zero magnitude is too small to represent in a - // float. It can't be represented faithfully in an f32. - if (0.0 < magnitude && magnitude < static_cast(std::numeric_limits::min())) { - return {Token::Type::kError, source, "magnitude too small to be represented as f32"}; - } - switch (check_limits(value)) { - case LimitCheck::kTooSmall: - return {Token::Type::kError, source, "value too small for f32"}; - case LimitCheck::kTooLarge: - return {Token::Type::kError, source, "value too large for f32"}; - default: - return {Token::Type::kFloatLiteral_F, source, value}; + if (auto f = CheckedConvert(AFloat(value))) { + return {Token::Type::kFloatLiteral_F, source, value}; + } else { + if (f.Failure() == ConversionFailure::kTooSmall) { + return {Token::Type::kError, source, + "value magnitude too small to be represented as 'f32'"}; + } + return {Token::Type::kError, source, "value cannot be represented as 'f32'"}; } } // TODO(crbug.com/tint/1504): Properly support abstract float: // Change `AbstractFloatType` to `double`, update errors to say 'abstract int'. - using AbstractFloatType = float; - if (0.0 < magnitude && - magnitude < static_cast(std::numeric_limits::min())) { - return {Token::Type::kError, source, "magnitude too small to be represented as f32"}; - } - switch (check_limits(value)) { - case LimitCheck::kTooSmall: - return {Token::Type::kError, source, "value too small for f32"}; - case LimitCheck::kTooLarge: - return {Token::Type::kError, source, "value too large for f32"}; - default: - return {Token::Type::kFloatLiteral, source, value}; + using AbstractFloatType = f32; + if (auto f = CheckedConvert(AFloat(value))) { + return {Token::Type::kFloatLiteral, source, value}; + } else { + if (f.Failure() == ConversionFailure::kTooSmall) { + return {Token::Type::kError, source, + "value magnitude too small to be represented as 'f32'"}; + } + return {Token::Type::kError, source, "value cannot be represented as 'f32'"}; } } @@ -725,44 +682,31 @@ Token Lexer::build_token_from_int_if_possible(Source source, size_t start, int32 int64_t res = strtoll(&at(start), nullptr, base); if (matches(pos(), "u")) { - switch (check_limits(res)) { - case LimitCheck::kTooSmall: - return {Token::Type::kError, source, "unsigned literal cannot be negative"}; - case LimitCheck::kTooLarge: - return {Token::Type::kError, source, "value too large for u32"}; - default: - advance(1); - end_source(source); - return {Token::Type::kIntLiteral_U, source, res}; + if (CheckedConvert(AInt(res))) { + advance(1); + end_source(source); + return {Token::Type::kIntLiteral_U, source, res}; } + return {Token::Type::kError, source, "value cannot be represented as 'u32'"}; } if (matches(pos(), "i")) { - switch (check_limits(res)) { - case LimitCheck::kTooSmall: - return {Token::Type::kError, source, "value too small for i32"}; - case LimitCheck::kTooLarge: - return {Token::Type::kError, source, "value too large for i32"}; - default: - break; + if (CheckedConvert(AInt(res))) { + advance(1); + end_source(source); + return {Token::Type::kIntLiteral_I, source, res}; } - advance(1); - end_source(source); - return {Token::Type::kIntLiteral_I, source, res}; + return {Token::Type::kError, source, "value cannot be represented as 'i32'"}; } // TODO(crbug.com/tint/1504): Properly support abstract int: // Change `AbstractIntType` to `int64_t`, update errors to say 'abstract int'. - using AbstractIntType = int32_t; - switch (check_limits(res)) { - case LimitCheck::kTooSmall: - return {Token::Type::kError, source, "value too small for i32"}; - case LimitCheck::kTooLarge: - return {Token::Type::kError, source, "value too large for i32"}; - default: - end_source(source); - return {Token::Type::kIntLiteral, source, res}; + using AbstractIntType = i32; + if (CheckedConvert(AInt(res))) { + end_source(source); + return {Token::Type::kIntLiteral, source, res}; } + return {Token::Type::kError, source, "value cannot be represented as 'i32'"}; } Token Lexer::try_hex_integer() { diff --git a/src/tint/reader/wgsl/lexer_test.cc b/src/tint/reader/wgsl/lexer_test.cc index 0c3e1628c4..52f5cb8464 100644 --- a/src/tint/reader/wgsl/lexer_test.cc +++ b/src/tint/reader/wgsl/lexer_test.cc @@ -680,7 +680,7 @@ TEST_F(LexerTest, IntegerTest_HexSignedTooLarge) { auto t = l.next(); ASSERT_TRUE(t.Is(Token::Type::kError)); - EXPECT_EQ(t.to_str(), "value too large for i32"); + EXPECT_EQ(t.to_str(), "value cannot be represented as 'i32'"); } TEST_F(LexerTest, IntegerTest_HexSignedTooSmall) { @@ -689,7 +689,7 @@ TEST_F(LexerTest, IntegerTest_HexSignedTooSmall) { auto t = l.next(); ASSERT_TRUE(t.Is(Token::Type::kError)); - EXPECT_EQ(t.to_str(), "value too small for i32"); + EXPECT_EQ(t.to_str(), "value cannot be represented as 'i32'"); } TEST_F(LexerTest, IntegerTest_HexSignedTooManyDigits) { diff --git a/src/tint/reader/wgsl/parser_impl_const_literal_test.cc b/src/tint/reader/wgsl/parser_impl_const_literal_test.cc index cbddce3e4f..c96b197f5a 100644 --- a/src/tint/reader/wgsl/parser_impl_const_literal_test.cc +++ b/src/tint/reader/wgsl/parser_impl_const_literal_test.cc @@ -133,7 +133,7 @@ TEST_F(ParserImplTest, ConstLiteral_Uint_Negative) { auto c = p->const_literal(); EXPECT_FALSE(c.matched); EXPECT_TRUE(c.errored); - EXPECT_EQ(p->error(), "1:1: unsigned literal cannot be negative"); + EXPECT_EQ(p->error(), "1:1: value cannot be represented as 'u32'"); ASSERT_EQ(c.value, nullptr); } @@ -179,7 +179,7 @@ TEST_F(ParserImplTest, ConstLiteral_InvalidFloat_TooSmallMagnitude) { auto c = p->const_literal(); EXPECT_FALSE(c.matched); EXPECT_TRUE(c.errored); - EXPECT_EQ(p->error(), "1:1: magnitude too small to be represented as f32"); + EXPECT_EQ(p->error(), "1:1: value magnitude too small to be represented as 'f32'"); ASSERT_EQ(c.value, nullptr); } @@ -188,7 +188,7 @@ TEST_F(ParserImplTest, ConstLiteral_InvalidFloat_TooLargeNegative) { auto c = p->const_literal(); EXPECT_FALSE(c.matched); EXPECT_TRUE(c.errored); - EXPECT_EQ(p->error(), "1:1: value too small for f32"); + EXPECT_EQ(p->error(), "1:1: value cannot be represented as 'f32'"); ASSERT_EQ(c.value, nullptr); } @@ -197,7 +197,7 @@ TEST_F(ParserImplTest, ConstLiteral_InvalidFloat_TooLargePositive) { auto c = p->const_literal(); EXPECT_FALSE(c.matched); EXPECT_TRUE(c.errored); - EXPECT_EQ(p->error(), "1:1: value too large for f32"); + EXPECT_EQ(p->error(), "1:1: value cannot be represented as 'f32'"); ASSERT_EQ(c.value, nullptr); }