Daniel Carvalho has submitted this change. (
https://gem5-review.googlesource.com/c/public/gem5/+/38775 )
Change subject: base: Fix scientific number conversion in base/str
......................................................................
base: Fix scientific number conversion in base/str
Previously converting scientific numbers to non-floating
numbers would yield incorrect results, because the
underlying conversion was using stoll and stoull, which
do not deal with scientific numbers properly. Therefore,
converting to_number("8.234e+08", val) would yield
val=8, which is blatantly wrong (expected 823400000).
This was fixed by searching for "e" within the string to
be converted.
To make sure all double and scientific number conversions
are correct, a few extra tests were added.
Change-Id: I6a9599d8473909d274326b6f8c268e3603044ab4
Signed-off-by: Daniel R. Carvalho <[email protected]>
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/38775
Reviewed-by: Bobby R. Bruce <[email protected]>
Maintainer: Bobby R. Bruce <[email protected]>
Tested-by: kokoro <[email protected]>
---
M src/base/str.hh
M src/base/str.test.cc
2 files changed, 74 insertions(+), 0 deletions(-)
Approvals:
Bobby R. Bruce: Looks good to me, approved; Looks good to me, approved
kokoro: Regressions pass
diff --git a/src/base/str.hh b/src/base/str.hh
index 40fd780..d91761d 100644
--- a/src/base/str.hh
+++ b/src/base/str.hh
@@ -112,6 +112,10 @@
std::is_signed<T>::value, T>
__to_number(const std::string &value)
{
+ // Cannot parse scientific numbers
+ if (value.find('e') != std::string::npos) {
+ throw std::invalid_argument("Cannot convert scientific to
integral");
+ }
// start big and narrow it down if needed, determine the base
dynamically
long long r = std::stoll(value, nullptr, 0);
if (r < std::numeric_limits<T>::lowest()
@@ -126,6 +130,10 @@
!std::is_signed<T>::value, T>
__to_number(const std::string &value)
{
+ // Cannot parse scientific numbers
+ if (value.find('e') != std::string::npos) {
+ throw std::invalid_argument("Cannot convert scientific to
integral");
+ }
// start big and narrow it down if needed, determine the base
dynamically
unsigned long long r = std::stoull(value, nullptr, 0);
if (r > std::numeric_limits<T>::max())
@@ -137,6 +145,10 @@
typename std::enable_if_t<std::is_enum<T>::value, T>
__to_number(const std::string &value)
{
+ // Cannot parse scientific numbers
+ if (value.find('e') != std::string::npos) {
+ throw std::invalid_argument("Cannot convert scientific to
integral");
+ }
auto r = __to_number<typename std::underlying_type<T>::type>(value);
return static_cast<T>(r);
}
diff --git a/src/base/str.test.cc b/src/base/str.test.cc
index f0cd483..d3cbc3d 100644
--- a/src/base/str.test.cc
+++ b/src/base/str.test.cc
@@ -285,6 +285,58 @@
EXPECT_FALSE(to_number(input, output));
}
+/** Test that a double that can be converted to int is always rounded
down. */
+TEST(StrTest, ToNumberUnsigned8BitIntRoundDown)
+{
+ uint8_t output;
+ std::string input_1 = "2.99";
+ EXPECT_TRUE(to_number(input_1, output));
+ EXPECT_EQ(2, output);
+
+ std::string input_2 = "3.99";
+ EXPECT_TRUE(to_number(input_2, output));
+ EXPECT_EQ(3, output);
+}
+
+/**
+ * Test that a double can still be converted to int as long as it is below
+ * the numerical limit + 1.
+ */
+TEST(StrTest, ToNumber8BitUnsignedLimit)
+{
+ uint8_t output;
+ std::string input = "255.99";
+ EXPECT_TRUE(to_number(input, output));
+ EXPECT_EQ(255, output);
+}
+
+/**
+ * Test that a double cannot be converted to int when it passes the
numerical
+ * limit.
+ */
+TEST(StrTest, ToNumber8BitUnsignedOutOfRange)
+{
+ uint8_t output;
+ std::string input = "256.99";
+ EXPECT_FALSE(to_number(input, output));
+}
+
+/** Test that a scientific number cannot be converted to int. */
+TEST(StrTest, ToNumberUnsignedScientific)
+{
+ uint32_t output;
+ std::string input = "8.234e+08";
+ EXPECT_FALSE(to_number(input, output));
+}
+
+/** Test that a negative scientific number cannot be converted to int. */
+TEST(StrTest, ToNumberIntScientificNegative)
+{
+ int32_t output;
+ std::string input = "-8.234e+08";
+ EXPECT_FALSE(to_number(input, output));
+}
+
TEST(StrTest, ToNumber64BitInt)
{
int64_t output;
@@ -379,6 +431,16 @@
EXPECT_EQ(expected_output, output);
}
+/** Test that a scientific number is converted properly to double. */
+TEST(StrTest, ToNumberScientific)
+{
+ double output;
+ std::string input = "8.234e+08";
+ double expected_output = 823400000;
+ EXPECT_TRUE(to_number(input, output));
+ EXPECT_EQ(expected_output, output);
+}
+
/*
* The "to_bool" function takes a string, "true" or "false"
* (case-insenstive), and sets the second argument to the bool equivilent.
5 is the latest approved patch-set.
No files were changed between the latest approved patch-set and the
submitted one.
--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/38775
To unsubscribe, or for help writing mail filters, visit
https://gem5-review.googlesource.com/settings
Gerrit-Project: public/gem5
Gerrit-Branch: develop
Gerrit-Change-Id: I6a9599d8473909d274326b6f8c268e3603044ab4
Gerrit-Change-Number: 38775
Gerrit-PatchSet: 7
Gerrit-Owner: Daniel Carvalho <[email protected]>
Gerrit-Reviewer: Andreas Sandberg <[email protected]>
Gerrit-Reviewer: Bobby R. Bruce <[email protected]>
Gerrit-Reviewer: Daniel Carvalho <[email protected]>
Gerrit-Reviewer: kokoro <[email protected]>
Gerrit-MessageType: merged
_______________________________________________
gem5-dev mailing list -- [email protected]
To unsubscribe send an email to [email protected]
%(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s
