From 347c74e6712654d6c96409164cb6cb4956e9c2a8 Mon Sep 17 00:00:00 2001 From: David Neto Date: Wed, 20 Oct 2021 22:07:43 +0000 Subject: [PATCH] wgsl: decimal float: point is optional; can use E for exponent Also: Check for too-small non-zero magnitude on negative numbers too. Fixes: tint:1255 Change-Id: I3c231ce197c56e1ec517708b8073d49a8ae67ccb Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/67100 Reviewed-by: Antonio Maiorano Commit-Queue: David Neto Kokoro: Kokoro Auto-Submit: David Neto --- src/reader/wgsl/lexer.cc | 51 +++++++++++++------ src/reader/wgsl/lexer_test.cc | 48 +++++++++++++---- .../wgsl/parser_impl_const_literal_test.cc | 33 +++++++++++- 3 files changed, 106 insertions(+), 26 deletions(-) diff --git a/src/reader/wgsl/lexer.cc b/src/reader/wgsl/lexer.cc index 700c33fb56..5c57c9ce67 100644 --- a/src/reader/wgsl/lexer.cc +++ b/src/reader/wgsl/lexer.cc @@ -210,43 +210,58 @@ Token Lexer::try_float() { auto end = pos_; auto source = begin_source(); + bool has_mantissa_digits = false; if (matches(end, "-")) { end++; } while (end < len_ && is_digit(content_->data[end])) { + has_mantissa_digits = true; end++; } - if (end >= len_ || !matches(end, ".")) { - return {}; + bool has_point = false; + if (end < len_ && matches(end, ".")) { + has_point = true; + end++; } - end++; while (end < len_ && is_digit(content_->data[end])) { + has_mantissa_digits = true; end++; } + if (!has_mantissa_digits) { + return {}; + } + // Parse the exponent if one exists - if (end < len_ && matches(end, "e")) { + bool has_exponent = false; + if (end < len_ && (matches(end, "e") || matches(end, "E"))) { end++; if (end < len_ && (matches(end, "+") || matches(end, "-"))) { end++; } - auto exp_start = end; while (end < len_ && isdigit(content_->data[end])) { + has_exponent = true; end++; } - // Must have an exponent - if (exp_start == end) - return {}; + // If an 'e' or 'E' was present, then the number part must also be present. + if (!has_exponent) { + const auto str = content_->data.substr(start, end - start); + return {Token::Type::kError, source, + "incomplete exponent for floating point literal: " + str}; + } } - auto str = content_->data.substr(start, end - start); - if (str == "." || str == "-.") + if (!has_point && !has_exponent) { + // If it only has digits then it's an integer. return {}; + } + + const auto str = content_->data.substr(start, end - start); pos_ = end; location_.column += (end - start); @@ -254,16 +269,22 @@ Token Lexer::try_float() { end_source(source); auto res = strtod(content_->data.c_str() + start, nullptr); - // This handles if the number is a really small in the exponent - if (res > 0 && res < static_cast(std::numeric_limits::min())) { - return {Token::Type::kError, source, "f32 (" + str + " too small"}; + // 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. + const auto magnitude = std::fabs(res); + if (0.0 < magnitude && + magnitude < static_cast(std::numeric_limits::min())) { + return {Token::Type::kError, source, + "f32 (" + str + ") magnitude too small, not representable"}; } // This handles if the number is really large negative number if (res < static_cast(std::numeric_limits::lowest())) { - return {Token::Type::kError, source, "f32 (" + str + ") too small"}; + return {Token::Type::kError, source, + "f32 (" + str + ") too large (negative)"}; } if (res > static_cast(std::numeric_limits::max())) { - return {Token::Type::kError, source, "f32 (" + str + ") too large"}; + return {Token::Type::kError, source, + "f32 (" + str + ") too large (positive)"}; } return {source, static_cast(res)}; diff --git a/src/reader/wgsl/lexer_test.cc b/src/reader/wgsl/lexer_test.cc index 4c5e9b1e0a..d5fad28afc 100644 --- a/src/reader/wgsl/lexer_test.cc +++ b/src/reader/wgsl/lexer_test.cc @@ -149,6 +149,12 @@ INSTANTIATE_TEST_SUITE_P(LexerTest, FloatData{"-5.7", -5.7f}, FloatData{"-5.", -5.f}, FloatData{"-.7", -.7f}, + // No decimal, with exponent + FloatData{"1e5", 1e5f}, + FloatData{"1E5", 1e5f}, + FloatData{"1e-5", 1e-5f}, + FloatData{"1E-5", 1e-5f}, + // With decimal and exponents FloatData{"0.2e+12", 0.2e12f}, FloatData{"1.2e-5", 1.2e-5f}, FloatData{"2.57e23", 2.57e23f}, @@ -163,15 +169,39 @@ TEST_P(FloatTest_Invalid, Handles) { auto t = l.next(); EXPECT_FALSE(t.Is(Token::Type::kFloatLiteral)); } -INSTANTIATE_TEST_SUITE_P(LexerTest, - FloatTest_Invalid, - testing::Values(".", - "-.", - "2.5e+256", - "-2.5e+127", - "2.5e-300", - "2.5e 12", - "2.5e+ 123")); +INSTANTIATE_TEST_SUITE_P( + LexerTest, + FloatTest_Invalid, + testing::Values(".", + "-.", + // Need a mantissa digit + ".e5", + ".E5", + // Need exponent digits + ".e", + ".e+", + ".e-", + ".E", + ".e+", + ".e-", + // Overflow + "2.5e+256", + "-2.5e+127", + // Magnitude smaller than smallest positive f32. + "2.5e-300", + "-2.5e-300", + // Decimal exponent must immediately + // follow the 'e'. + "2.5e 12", + "2.5e +12", + "2.5e -12", + "2.5e+ 123", + "2.5e- 123", + "2.5E 12", + "2.5E +12", + "2.5E -12", + "2.5E+ 123", + "2.5E- 123")); using IdentifierTest = testing::TestWithParam; TEST_P(IdentifierTest, Parse) { diff --git a/src/reader/wgsl/parser_impl_const_literal_test.cc b/src/reader/wgsl/parser_impl_const_literal_test.cc index 33d972c6ee..97d625e734 100644 --- a/src/reader/wgsl/parser_impl_const_literal_test.cc +++ b/src/reader/wgsl/parser_impl_const_literal_test.cc @@ -77,12 +77,41 @@ TEST_F(ParserImplTest, ConstLiteral_Float) { EXPECT_EQ(c->source.range, (Source::Range{{1u, 1u}, {1u, 8u}})); } -TEST_F(ParserImplTest, ConstLiteral_InvalidFloat) { +TEST_F(ParserImplTest, ConstLiteral_InvalidFloat_IncompleteExponent) { + auto p = parser("1.0e+"); + auto c = p->const_literal(); + EXPECT_FALSE(c.matched); + EXPECT_TRUE(c.errored); + EXPECT_EQ(p->error(), + "1:1: incomplete exponent for floating point literal: 1.0e+"); + ASSERT_EQ(c.value, nullptr); +} + +TEST_F(ParserImplTest, ConstLiteral_InvalidFloat_TooSmallMagnitude) { + auto p = parser("1e-256"); + auto c = p->const_literal(); + EXPECT_FALSE(c.matched); + EXPECT_TRUE(c.errored); + EXPECT_EQ(p->error(), + "1:1: f32 (1e-256) magnitude too small, not representable"); + ASSERT_EQ(c.value, nullptr); +} + +TEST_F(ParserImplTest, ConstLiteral_InvalidFloat_TooLargeNegative) { + auto p = parser("-1.2e+256"); + auto c = p->const_literal(); + EXPECT_FALSE(c.matched); + EXPECT_TRUE(c.errored); + EXPECT_EQ(p->error(), "1:1: f32 (-1.2e+256) too large (negative)"); + ASSERT_EQ(c.value, nullptr); +} + +TEST_F(ParserImplTest, ConstLiteral_InvalidFloat_TooLargePositive) { auto p = parser("1.2e+256"); auto c = p->const_literal(); EXPECT_FALSE(c.matched); EXPECT_TRUE(c.errored); - EXPECT_EQ(p->error(), "1:1: f32 (1.2e+256) too large"); + EXPECT_EQ(p->error(), "1:1: f32 (1.2e+256) too large (positive)"); ASSERT_EQ(c.value, nullptr); }