Templetize and improve beast string-to-integer conversions:

* Properly handle numbers at the edge of precision
* Improve and expand unit test coverage

beast/module/core/text/LexicalCast.cpp

@@ -74,7 +74,6 @@ public:
         try
         {
             lexicalCastThrow<int>("\xef\xbc\x91\xef\xbc\x90"); // utf-8 encoded
-            fail("Should throw");
         }
         catch(BadLexicalCast const&)
         {
@@ -82,6 +81,175 @@ public:
         }
     }
 
+    template <class T>
+    void tryBadConvert (std::string const& s)
+    {
+        T out;
+        expect (!lexicalCastChecked (out, s), s);
+    }
+
+    void testConversionOverflows()
+    {
+        testcase ("conversion overflows");
+
+        tryBadConvert <std::uint64_t> ("99999999999999999999");
+        tryBadConvert <std::uint32_t> ("4294967300");
+        tryBadConvert <std::uint16_t> ("75821");
+    }
+
+    void testConversionUnderflows ()
+    {
+        testcase ("conversion underflows");
+
+        tryBadConvert <std::uint32_t> ("-1");
+
+        tryBadConvert <std::int64_t> ("-99999999999999999999");
+        tryBadConvert <std::int32_t> ("-4294967300");
+        tryBadConvert <std::int16_t> ("-75821");
+    }
+
+    template <class T>
+    bool tryEdgeCase (std::string const& s)
+    {
+        T ret;
+
+        bool const result = lexicalCastChecked (ret, s);
+
+        if (!result)
+            return false;
+
+        return s == std::to_string (ret);
+    }
+
+    void testEdgeCases ()
+    {
+        testcase ("conversion edge cases");
+
+        expect(tryEdgeCase <std::uint64_t> ("18446744073709551614"));
+        expect(tryEdgeCase <std::uint64_t> ("18446744073709551615"));
+        expect(!tryEdgeCase <std::uint64_t> ("18446744073709551616"));
+
+        expect(tryEdgeCase <std::int64_t> ("9223372036854775806"));
+        expect(tryEdgeCase <std::int64_t> ("9223372036854775807"));
+        expect(!tryEdgeCase <std::int64_t> ("9223372036854775808"));
+
+        expect(tryEdgeCase <std::int64_t> ("-9223372036854775807"));
+        expect(tryEdgeCase <std::int64_t> ("-9223372036854775808"));
+        expect(!tryEdgeCase <std::int64_t> ("-9223372036854775809"));
+
+        expect(tryEdgeCase <std::uint32_t> ("4294967294"));
+        expect(tryEdgeCase <std::uint32_t> ("4294967295"));
+        expect(!tryEdgeCase <std::uint32_t> ("4294967296"));
+
+        expect(tryEdgeCase <std::int32_t> ("2147483646"));
+        expect(tryEdgeCase <std::int32_t> ("2147483647"));
+        expect(!tryEdgeCase <std::int32_t> ("2147483648"));
+
+        expect(tryEdgeCase <std::int32_t> ("-2147483647"));
+        expect(tryEdgeCase <std::int32_t> ("-2147483648"));
+        expect(!tryEdgeCase <std::int32_t> ("-2147483649"));
+
+        expect(tryEdgeCase <std::uint16_t> ("65534"));
+        expect(tryEdgeCase <std::uint16_t> ("65535"));
+        expect(!tryEdgeCase <std::uint16_t> ("65536"));
+
+        expect(tryEdgeCase <std::int16_t> ("32766"));
+        expect(tryEdgeCase <std::int16_t> ("32767"));
+        expect(!tryEdgeCase <std::int16_t> ("32768"));
+
+        expect(tryEdgeCase <std::int16_t> ("-32767"));
+        expect(tryEdgeCase <std::int16_t> ("-32768"));
+        expect(!tryEdgeCase <std::int16_t> ("-32769"));
+    }
+
+    template <class T>
+    void testThrowConvert(std::string const& s, bool success)
+    {
+        bool result = !success;
+        T out;
+
+        try
+        {
+            out = lexicalCastThrow <T> (s);
+            result = true;
+        }
+        catch(BadLexicalCast const&)
+        {
+            result = false;
+        }
+
+        expect (result == success, s);
+    }
+
+    void testThrowingConversions ()
+    {
+        testcase ("throwing conversion");
+
+        testThrowConvert <std::uint64_t> ("99999999999999999999", false);
+        testThrowConvert <std::uint64_t> ("9223372036854775806", true);
+
+        testThrowConvert <std::uint32_t> ("4294967290", true);
+        testThrowConvert <std::uint32_t> ("42949672900", false);
+        testThrowConvert <std::uint32_t> ("429496729000", false);
+        testThrowConvert <std::uint32_t> ("4294967290000", false);
+
+        testThrowConvert <std::int32_t> ("5294967295", false);
+        testThrowConvert <std::int32_t> ("-2147483644", true);
+
+        testThrowConvert <std::int16_t> ("66666", false);
+        testThrowConvert <std::int16_t> ("-5711", true);
+    }
+
+    void testZero ()
+    {
+        testcase ("zero conversion");
+
+        {
+            std::int32_t out;
+
+            expect (lexicalCastChecked (out, "-0"), "0");
+            expect (lexicalCastChecked (out, "0"), "0");
+            expect (lexicalCastChecked (out, "+0"), "0");
+        }
+
+        {
+            std::uint32_t out;
+
+            expect (!lexicalCastChecked (out, "-0"), "0");
+            expect (lexicalCastChecked (out, "0"), "0");
+            expect (lexicalCastChecked (out, "+0"), "0");
+        }
+    }
+
+    void testEntireRange ()
+    {
+        testcase ("entire range");
+
+        std::int32_t i = std::numeric_limits<std::int16_t>::min();
+        std::string const empty("");
+
+        while (i <= std::numeric_limits<std::int16_t>::max())
+        {
+            std::int16_t j = static_cast<std::int16_t>(i);
+
+            auto actual = std::to_string (j);
+
+            auto result = lexicalCast (j, empty);
+
+            expect (result == actual, actual + " (string to integer)");
+
+            if (result == actual)
+            {
+                auto number = lexicalCast <std::int16_t> (result);
+
+                if (number != j)
+                    expect (false, actual + " (integer to string)");
+            }
+
+            i++;
+        }
+    }
+
     void run()
     {
         std::int64_t const seedValue = 50;
@@ -98,6 +266,12 @@ public:
         testIntegers <std::uint64_t> (r);
 
         testPathologies();
+        testConversionOverflows ();
+        testConversionUnderflows ();
+        testThrowingConversions ();
+        testZero ();
+        testEdgeCases ();
+        testEntireRange ();
     }
 };
 

beast/module/core/text/LexicalCast.h

@@ -29,6 +29,8 @@
 #include <string>
 #include <utility>
 
+#include <iostream>
+
 namespace beast {
 
 namespace detail {
@@ -47,14 +49,16 @@ parse_integral (Int& num, FwdIt first, FwdIt last, Accumulator accumulator)
 
     if (first == last)
         return false;
+
     while (first != last)
     {
         auto const c = *first++;
         if (c < '0' || c > '9')
             return false;
-        if (!accumulator(Int(c - '0')))
+        if (!accumulator(num, Int(c - '0')))
             return false;
     }
+
     return true;
 }
 
@@ -62,14 +66,19 @@ template <class Int, class FwdIt>
 bool
 parse_negative_integral (Int& num, FwdIt first, FwdIt last)
 {
-    Int const limit = std::numeric_limits <Int>::min();
+    Int limit_value = std::numeric_limits <Int>::min() / 10;
+    Int limit_digit = std::numeric_limits <Int>::min() % 10;
+
+    if (limit_digit < 0)
+        limit_digit = -limit_digit;
 
     return parse_integral<Int> (num, first, last, 
-        [&num,&limit](Int n)
+        [limit_value, limit_digit](Int& value, Int digit)
         {
-            if ((limit - (10 * num)) > -n)
+            assert ((digit >= 0) && (digit <= 9));
+            if (value < limit_value || (value == limit_value && digit > limit_digit))
                 return false;
-            num = 10 * num - n;
+            value = (value * 10) - digit;
             return true;
         });
 }
@@ -78,14 +87,16 @@ template <class Int, class FwdIt>
 bool
 parse_positive_integral (Int& num, FwdIt first, FwdIt last)
 {
-    Int const limit = std::numeric_limits <Int>::max();
+    Int limit_value = std::numeric_limits <Int>::max() / 10;
+    Int limit_digit = std::numeric_limits <Int>::max() % 10;
 
     return parse_integral<Int> (num, first, last,
-        [&num,&limit](Int n)
+        [limit_value, limit_digit](Int& value, Int digit)
         {
-            if (n > (limit - (10 * num)))
+            assert ((digit >= 0) && (digit <= 9));
+            if (value > limit_value || (value == limit_value && digit > limit_digit))
                 return false;
-            num = 10 * num + n;
+            value = (value * 10) + digit;
             return true;
         });
 }
@@ -129,17 +140,22 @@ struct LexicalCast;
 template <class In>
 struct LexicalCast <std::string, In>
 {
-    bool operator() (std::string& out, short in)                { out = std::to_string(in); return true; }
+    template <class Arithmetic = In>
-    bool operator() (std::string& out, int in)                  { out = std::to_string(in); return true; }
+    std::enable_if_t <std::is_arithmetic <Arithmetic>::value, bool>
-    bool operator() (std::string& out, long in)                 { out = std::to_string(in); return true; }
+    operator () (std::string& out, Arithmetic in)
-    bool operator() (std::string& out, long long in)            { out = std::to_string(in); return true; }
+    {
-    bool operator() (std::string& out, unsigned short in)       { out = std::to_string(in); return true; }
+        out = std::to_string (in);
-    bool operator() (std::string& out, unsigned int in)         { out = std::to_string(in); return true; }
+        return true;
-    bool operator() (std::string& out, unsigned long in)        { out = std::to_string(in); return true; }
+    }
-    bool operator() (std::string& out, unsigned long long in)   { out = std::to_string(in); return true; }
+
-    bool operator() (std::string& out, float in)                { out = std::to_string(in); return true; }
+    template <class Enumeration = In>
-    bool operator() (std::string& out, double in)               { out = std::to_string(in); return true; }
+    std::enable_if_t <std::is_enum <Enumeration>::value, bool>
-    bool operator() (std::string& out, long double in)          { out = std::to_string(in); return true; }
+    operator () (std::string& out, Enumeration in)
+    {
+        out = std::to_string (
+            static_cast <std::underlying_type_t <Enumeration>> (in));
+        return true;
+    }
 };
 
 // Parse std::string to number
@@ -150,43 +166,20 @@ struct LexicalCast <Out, std::string>
         "beast::LexicalCast can only be used with integral types");
 
     template <class Integral = Out>
-    typename std::enable_if <std::is_unsigned <Integral>::value, bool>::type
+    std::enable_if_t <std::is_unsigned <Integral>::value, bool>
     operator () (Integral& out, std::string const& in) const
     {
         return parseUnsigned (out, std::begin(in), std::end(in));
     }
 
     template <class Integral = Out>
-    typename std::enable_if <std::is_signed <Integral>::value, bool>::type
+    std::enable_if_t <std::is_signed <Integral>::value, bool>
     operator () (Integral& out, std::string const& in) const
     {
         return parseSigned (out, std::begin(in), std::end(in));
     }
 };
 
-#if 0
-template <class Out>
-bool
-LexicalCast <Out, std::string>::operator() (bool& out, std::string const& in) const
-{
-    // boost::lexical_cast is very strict, it
-    // throws on anything but "1" or "0"
-    //
-    if (in == "1")
-    {
-        out = true;
-        return true;
-    }
-    else if (in == "0")
-    {
-        out = false;
-        return true;
-    }
-
-    return false;
-}
-#endif
-
 //------------------------------------------------------------------------------
 
 // Conversion from null terminated char const*
@@ -206,15 +199,7 @@ struct LexicalCast <Out, char*>
 {
     bool operator() (Out& out, char* in) const
     {
-        Out result;
+        return LexicalCast <Out, std::string>()(out, in);
-
-        if (LexicalCast <Out, char const*> () (result, in))
-        {
-            out = result;
-            return true;
-        }
-
-        return false;
     }
 };