SERVER-27193 Use ItoA to handle integer cases in StringBuilder

5 files changed, 83 insertions, 22 deletions

diff --git a/src/mongo/bson/util/builder.h b/src/mongo/bson/util/builder.h
index 3a0c02d3258..e40238ca28f 100644
--- a/src/mongo/bson/util/builder.h
+++ b/src/mongo/bson/util/builder.h
@@ -30,6 +30,7 @@
 #pragma once
 
 #include <cfloat>
+#include <cinttypes>
 #include <cstdint>
 #include <sstream>
 #include <stdio.h>
@@ -48,6 +49,7 @@
 #include "mongo/stdx/type_traits.h"
 #include "mongo/util/allocator.h"
 #include "mongo/util/assert_util.h"
+#include "mongo/util/itoa.h"
 #include "mongo/util/shared_buffer.h"
 
 namespace mongo {
@@ -380,25 +382,25 @@ public:
         return SBNUM(x, MONGO_DBL_SIZE, "%g");
     }
     StringBuilderImpl& operator<<(int x) {
-        return SBNUM(x, MONGO_S32_SIZE, "%d");
+        return appendIntegral(x, MONGO_S32_SIZE);
     }
     StringBuilderImpl& operator<<(unsigned x) {
-        return SBNUM(x, MONGO_U32_SIZE, "%u");
+        return appendIntegral(x, MONGO_U32_SIZE);
     }
     StringBuilderImpl& operator<<(long x) {
-        return SBNUM(x, MONGO_S64_SIZE, "%ld");
+        return appendIntegral(x, MONGO_S64_SIZE);
     }
     StringBuilderImpl& operator<<(unsigned long x) {
-        return SBNUM(x, MONGO_U64_SIZE, "%lu");
+        return appendIntegral(x, MONGO_U64_SIZE);
     }
     StringBuilderImpl& operator<<(long long x) {
-        return SBNUM(x, MONGO_S64_SIZE, "%lld");
+        return appendIntegral(x, MONGO_S64_SIZE);
     }
     StringBuilderImpl& operator<<(unsigned long long x) {
-        return SBNUM(x, MONGO_U64_SIZE, "%llu");
+        return appendIntegral(x, MONGO_U64_SIZE);
     }
     StringBuilderImpl& operator<<(short x) {
-        return SBNUM(x, MONGO_S16_SIZE, "%hd");
+        return appendIntegral(x, MONGO_S16_SIZE);
     }
     StringBuilderImpl& operator<<(const void* x) {
         if (sizeof(x) == 8) {
@@ -472,6 +474,21 @@ public:
 
 private:
     _BufBuilder<Allocator> _buf;
+    template <typename T>
+    StringBuilderImpl& appendIntegral(T val, int maxSize) {
+        MONGO_STATIC_ASSERT(!std::is_same<T, char>());  // char shouldn't append as number.
+        MONGO_STATIC_ASSERT(std::is_integral<T>());
+        MONGO_STATIC_ASSERT(std::numeric_limits<T>::max() <= std::numeric_limits<uint64_t>::max());
+
+        if (val < 0) {
+            *this << '-';
+            append(StringData(ItoA(-uint64_t(val))));  // Send the magnitude to ItoA.
+        } else {
+            append(StringData(ItoA(uint64_t(val))));
+        }
+
+        return *this;
+    }
 
     template <typename T>
     StringBuilderImpl& SBNUM(T val, int maxSize, const char* macro) {
diff --git a/src/mongo/bson/util/builder_test.cpp b/src/mongo/bson/util/builder_test.cpp
index f56ba5999c8..b45a0dcf5aa 100644
--- a/src/mongo/bson/util/builder_test.cpp
+++ b/src/mongo/bson/util/builder_test.cpp
@@ -82,4 +82,45 @@ TEST(Builder, StackAllocatorShouldNotLeak) {
     stackAlloc.malloc(StackAllocator::SZ + 1);  // Force heap allocation.
     // Let the builder go out of scope. If this leaks, it will trip the ASAN leak detector.
 }
+
+template <typename T>
+void testStringBuilderIntegral() {
+    auto check = [](T num) { ASSERT_EQ(std::string(str::stream() << num), std::to_string(num)); };
+
+    // Do some simple sanity checks.
+    check(0);
+    check(1);
+    check(-1);
+    check(std::numeric_limits<T>::min());
+    check(std::numeric_limits<T>::max());
+
+    // Check the full range of int16_t. Using int32_t as loop variable to detect when we are done.
+    for (int32_t num = std::numeric_limits<int16_t>::min();
+         num <= std::numeric_limits<int16_t>::max();
+         num++) {
+        check(num);
+    }
+}
+
+TEST(Builder, AppendInt) {
+    testStringBuilderIntegral<int>();
+}
+TEST(Builder, AppendUnsigned) {
+    testStringBuilderIntegral<unsigned>();
+}
+TEST(Builder, AppendLong) {
+    testStringBuilderIntegral<long>();
+}
+TEST(Builder, AppendUnsignedLong) {
+    testStringBuilderIntegral<unsigned long>();
+}
+TEST(Builder, AppendLongLong) {
+    testStringBuilderIntegral<long long>();
+}
+TEST(Builder, AppendUnsignedLongLong) {
+    testStringBuilderIntegral<unsigned long long>();
+}
+TEST(Builder, AppendShort) {
+    testStringBuilderIntegral<short>();
+}
 }
diff --git a/src/mongo/util/itoa.cpp b/src/mongo/util/itoa.cpp
index e19048b87ed..d00e0bd9713 100644
--- a/src/mongo/util/itoa.cpp
+++ b/src/mongo/util/itoa.cpp
@@ -1038,7 +1038,7 @@ const char kIndexTable[] =
 
 constexpr size_t ItoA::kBufSize;
 
-ItoA::ItoA(std::uint32_t val) {
+ItoA::ItoA(std::uint64_t val) {
     if (val < 10u) {
         _str = kIndexTable + (2 * val);
         _len = 1;
diff --git a/src/mongo/util/itoa.h b/src/mongo/util/itoa.h
index 50966093d3f..3443d4125ee 100644
--- a/src/mongo/util/itoa.h
+++ b/src/mongo/util/itoa.h
@@ -29,6 +29,7 @@
 #pragma once
 
 #include <cstdint>
+#include <limits>
 
 #include "mongo/base/disallow_copying.h"
 #include "mongo/base/string_data.h"
@@ -43,9 +44,11 @@ class ItoA {
     MONGO_DISALLOW_COPYING(ItoA);
 
 public:
-    static constexpr size_t kBufSize = 11;
+    static constexpr size_t kBufSize = std::numeric_limits<uint64_t>::digits10  //
+        + 1   // digits10 is 1 less than the maximum number of digits.
+        + 1;  // NUL byte.
 
-    explicit ItoA(std::uint32_t i);
+    explicit ItoA(std::uint64_t i);
 
     operator StringData() {
         return {_str, _len};
@@ -54,7 +57,6 @@ public:
 private:
     const char* _str{nullptr};
     std::size_t _len{0};
-    // 11 is provably the max size we need as uint32_t max has 10 digits.
     char _buf[kBufSize];
 };
 
diff --git a/src/mongo/util/itoa_test.cpp b/src/mongo/util/itoa_test.cpp
index b5158bb1fa5..0956d632cde 100644
--- a/src/mongo/util/itoa_test.cpp
+++ b/src/mongo/util/itoa_test.cpp
@@ -40,18 +40,19 @@ namespace {
 using namespace mongo;
 
 TEST(ItoA, StringDataEquality) {
-    ASSERT_EQ(ItoA::kBufSize - 1, std::to_string(std::numeric_limits<std::uint32_t>::max()).size());
+    ASSERT_EQ(ItoA::kBufSize - 1, std::to_string(std::numeric_limits<std::uint64_t>::max()).size());
 
-    for (auto testCase : {1u,
-                          12u,
-                          133u,
-                          1446u,
-                          17789u,
-                          192923u,
-                          2389489u,
-                          29313479u,
-                          1928127389u,
-                          std::numeric_limits<std::uint32_t>::max()}) {
+    for (auto testCase : {uint64_t(1),
+                          uint64_t(12),
+                          uint64_t(133),
+                          uint64_t(1446),
+                          uint64_t(17789),
+                          uint64_t(192923),
+                          uint64_t(2389489),
+                          uint64_t(29313479),
+                          uint64_t(1928127389),
+                          std::numeric_limits<std::uint64_t>::max() - 1,
+                          std::numeric_limits<std::uint64_t>::max()}) {
         ItoA itoa{testCase};
         ASSERT_EQ(std::to_string(testCase), StringData(itoa));
     }