[SERVER-41547] convert strtod to std::from_chars to support StringData Created: 05/Jun/19  Updated: 26/Dec/22  Resolved: 14/Dec/22

Status: Closed
Project: Core Server
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Improvement Priority: Minor - P4
Reporter: Nathan Brown (Inactive) Assignee: Alex Neben
Resolution: Won't Fix Votes: 0
Labels: neweng, post-v4-toolchain
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Assigned Teams:
Server Development Platform
Participants:

 Description   

strtod requires a null terminated string, but std::from_chars does not.



 Comments   
Comment by Alex Neben [ 14/Dec/22 ]

Here is the diff that is missing the +/- sign as well as leading whitespace

diff --git a/src/mongo/base/parse_number.cpp b/src/mongo/base/parse_number.cpp
index 2bb46f0ba10..73e5e001ae8 100644
--- a/src/mongo/base/parse_number.cpp
+++ b/src/mongo/base/parse_number.cpp
@@ -32,7 +32,7 @@
 #include "mongo/base/parse_number.h"
 
 #include <algorithm>
-#include <cerrno>
+#include <charconv>
 #include <cstdint>
 #include <cstdlib>
 #include <limits>
@@ -227,44 +227,29 @@ Status parseNumberFromStringHelper<double>(StringData stringValue,
 
     std::string str = stringValue.toString();
     const char* cStr = str.c_str();
-    char* endp;
-    errno = 0;
-    double d = strtod(cStr, &endp);
-    int actualErrno = errno;
-    if (endp == cStr) {
-#ifdef _WIN32
-        // The Windows libc implementation of strtod cannot parse +/-infinity or nan,
-        // so handle that here.
-        for (char& c : str)
-            c = ctype::toLower(c);
-        if (str == "nan"_sd) {
-            *result = std::numeric_limits<double>::quiet_NaN();
-            if (endptr)
-                *endptr = stringValue.end();
-            return Status::OK();
-        } else if (str == "+infinity"_sd || str == "infinity"_sd) {
-            *result = std::numeric_limits<double>::infinity();
-            if (endptr)
-                *endptr = stringValue.end();
-            return Status::OK();
-        } else if (str == "-infinity"_sd) {
-            *result = -std::numeric_limits<double>::infinity();
-            if (endptr)
-                *endptr = stringValue.end();
-            return Status::OK();
-        }
-#endif  // defined(_WIN32)
-        return Status(ErrorCodes::FailedToParse, "Did not consume any digits");
-    }
-    if (actualErrno == ERANGE) {
+    double d;
+    auto conversion_result = std::from_chars(
+        cStr, cStr + str.size(), d, std::chars_format::general | std::chars_format::hex);
+    if (conversion_result.ec == std::errc::result_out_of_range) {
+        std::cout << "result_out_of_range" << std::endl;
         return Status(ErrorCodes::Overflow, "Out of range");
-    }
+    }  // else if (conversion_result.ec == std::errc::invalid_argument) {
+    //     std::cout << "invalid_argument " << cStr << std::endl;
+    //     return Status(ErrorCodes::FailedToParse, "Did not consume any digits");
+    // }
+
     if (endptr) {
-        size_t charsConsumed = endp - cStr;
+        size_t charsConsumed = conversion_result.ptr - cStr;
+        std::cout << "Yeet" << std::endl;
+        std::cout << charsConsumed << std::endl;
+        std::cout << stringValue << std::endl;
         *endptr = stringValue.begin() + charsConsumed;
     }
-    if (!parser._allowTrailingText && endp != (cStr + str.size()))
+    if (!parser._allowTrailingText && conversion_result.ptr != (cStr + str.size())) {
+        std::cout << "_allowTrailingText" << std::endl;
         return Status(ErrorCodes::FailedToParse, "Did not consume whole string.");
+    }
+
     *result = d;
     return Status::OK();
 }
 

Comment by Alex Neben [ 14/Dec/22 ]

From the docs this behaves differently than strtod. https://en.cppreference.com/w/cpp/utility/from_chars

 

+/- sign and leading whitespace make this a noticeable change.

Comment by Andrew Morrow (Inactive) [ 13/Jun/19 ]

Probably not worth it then since we aren't going to upgrade GCC or clang for some time. Lets stop working on it, move it to PM-1381, and set the fixVersion to backlog. We will try again when we have newer GCC and clang. Hopefully they have implemented it by then.

Comment by A. Jesse Jiryu Davis [ 13/Jun/19 ]

str::from_chars doesn't support doubles in GCC or Clang yet, only MSVC, so we can't replace strtod yet. We could use std::from_chars in place of our handwritten integer parsing code in parseNumberFromStringWithBase. acm does that seem worthwhile?

Generated at Thu Feb 08 04:58:02 UTC 2024 using Jira 9.7.1#970001-sha1:2222b88b221c4928ef0de3161136cc90c8356a66.