Skip to content

Commit 5789992

Browse files
trflynn89trflynn89
committed
Update BasicStringConverter to use std::from_chars when able
Integral types can use std::from_chars, which is a bit better than the std::stoi family. It is non-exceptional and performs range checking. Floating point types are not supported by any compiler yet, though. One downside is that std::from_chars only supports char, not wchar_t, char8_t, etc. So string types other than std::string are first converted to std::string. This is only slightly worse than the situation was with std::stoi, as only char and wchar_t were supported.
1 parent ed41cff commit 5789992

File tree

6 files changed

+28
-178
lines changed

6 files changed

+28
-178
lines changed

‎bench/json/README.md

+6-6
Original file line numberDiff line numberDiff line change
@@ -23,17 +23,17 @@ could use some work :)
2323

2424
| Parser | Duration (ms) | Speed (MB/s) |
2525
| :-- | --: | --: |
26-
| libfly | 93.639 | 22.926 |
27-
| boost | 11.822 | 181.592 |
28-
| nlohmann | 54.645 | 39.285 |
26+
| libfly | 88.977 | 24.127 |
27+
| boost | 11.938 | 179.822 |
28+
| nlohmann | 54.969 | 39.054 |
2929

3030
### [gsoc-2018.json](/bench/json/data/gsoc-2018.json)
3131

3232
| Parser | Duration (ms) | Speed (MB/s) |
3333
| :-- | --: | --: |
34-
| libfly | 42.389 | 74.869 |
35-
| boost | 14.014 | 226.463 |
36-
| nlohmann | 33.634 | 94.359 |
34+
| libfly | 41.520 | 76.437 |
35+
| boost | 14.111 | 224.903 |
36+
| nlohmann | 32.927 | 96.386 |
3737

3838
## Profile
3939

‎fly/types/string/detail/string_converter.hpp

+15-58
Original file line numberDiff line numberDiff line change
@@ -1,23 +1,20 @@
11
#pragma once
22

3+
#include <charconv>
34
#include <cstdint>
4-
#include <limits>
55
#include <optional>
6-
#include <stdexcept>
76
#include <string>
8-
#include <type_traits>
97

108
namespace fly::detail {
119

1210
/**
13-
* Helper struct to convert a std::basic_string type to a plain-old-data type, e.g. int or bool.
11+
* Helper struct to convert a std::basic_string type to a plain-old-data type, e.g. int or float.
1412
*
15-
* Internally, the std::stoi family of functions is used to handle conversions, so only std::string
16-
* and std::wstring may be directly used. For std::u8string, std::u16string, and std::u32string,
17-
* first use BasicStringUnicode::convert_encoding to convert the string to std::string.
13+
* Ideally, this entire helper can be removed when the STL supports floating point types with
14+
* std::from_chars. However, only integral types are currently supported. Thus, integral types will
15+
* use std::from_chars, and floating point types will use the std::stof family of functions.
1816
*
19-
* It is recommended that outside callers use BasicString::convert instead of using this struct
20-
* directly.
17+
* https://en.cppreference.com/w/cpp/compiler_support
2118
*
2219
* @author Timothy Flynn ([email protected])
2320
* @version March 21, 2019
@@ -29,60 +26,20 @@ struct BasicStringConverter;
2926
template <typename StringType, typename T>
3027
struct BasicStringConverter
3128
{
32-
template <typename V = T, std::enable_if_t<(sizeof(V) < 8), bool> = 0>
3329
static std::optional<T> convert(const StringType &value)
3430
{
35-
static constexpr auto s_min = static_cast<long long>(std::numeric_limits<T>::min());
36-
static constexpr auto s_max = static_cast<long long>(std::numeric_limits<T>::max());
31+
const auto *begin = value.data();
32+
const auto *end = begin + value.size();
3733

38-
std::size_t index = 0;
39-
long long result = 0;
34+
T converted {};
35+
auto result = std::from_chars(begin, end, converted);
4036

41-
try
42-
{
43-
result = std::stoll(value, &index);
44-
}
45-
catch (...)
37+
if ((result.ptr != end) || (result.ec != std::errc()))
4638
{
4739
return std::nullopt;
4840
}
4941

50-
if ((index != value.length()) || (result < s_min) || (result > s_max))
51-
{
52-
return std::nullopt;
53-
}
54-
55-
return static_cast<T>(result);
56-
}
57-
58-
template <typename V = T, std::enable_if_t<(sizeof(V) == 8), bool> = 0>
59-
static std::optional<T> convert(const StringType &value)
60-
{
61-
std::size_t index = 0;
62-
T result = 0;
63-
64-
try
65-
{
66-
if constexpr (std::is_signed_v<T>)
67-
{
68-
result = std::stoll(value, &index);
69-
}
70-
else
71-
{
72-
result = std::stoull(value, &index);
73-
}
74-
}
75-
catch (...)
76-
{
77-
return std::nullopt;
78-
}
79-
80-
if (index != value.length())
81-
{
82-
return std::nullopt;
83-
}
84-
85-
return result;
42+
return converted;
8643
}
8744
};
8845

@@ -95,7 +52,7 @@ struct BasicStringConverter<StringType, float>
9552
static std::optional<value_type> convert(const StringType &value)
9653
{
9754
std::size_t index = 0;
98-
value_type result = 0;
55+
value_type result {};
9956

10057
try
10158
{
@@ -124,7 +81,7 @@ struct BasicStringConverter<StringType, double>
12481
static std::optional<value_type> convert(const StringType &value)
12582
{
12683
std::size_t index = 0;
127-
value_type result = 0;
84+
value_type result {};
12885

12986
try
13087
{
@@ -153,7 +110,7 @@ struct BasicStringConverter<StringType, long double>
153110
static std::optional<value_type> convert(const StringType &value)
154111
{
155112
std::size_t index = 0;
156-
value_type result = 0;
113+
value_type result {};
157114

158115
try
159116
{

‎fly/types/string/detail/string_traits.hpp

-8
Original file line numberDiff line numberDiff line change
@@ -127,14 +127,6 @@ struct BasicStringTraits
127127
template <typename T>
128128
inline static constexpr bool is_string_like_v = is_string_like<T>::value;
129129

130-
/**
131-
* Define a trait for testing if the STL has defined the std::stoi family of functions for
132-
* StringType.
133-
*/
134-
using has_stoi_family = any_same<StringType, std::string, std::wstring>;
135-
136-
inline static constexpr bool has_stoi_family_v = has_stoi_family::value;
137-
138130
/**
139131
* Define a trait for whether operator<< is defined for a type on the stream type used for
140132
* StringType.

‎fly/types/string/string.hpp

+3-3
Original file line numberDiff line numberDiff line change
@@ -783,7 +783,7 @@ std::optional<T> BasicString<StringType>::convert(const StringType &value)
783783

784784
return std::nullopt;
785785
}
786-
else if constexpr (traits::has_stoi_family_v)
786+
else if constexpr (std::is_same_v<char_type, char>)
787787
{
788788
return detail::BasicStringConverter<StringType, T>::convert(value);
789789
}
@@ -792,9 +792,9 @@ std::optional<T> BasicString<StringType>::convert(const StringType &value)
792792
auto it = value.cbegin();
793793
const auto end = value.cend();
794794

795-
if (auto result = unicode::template convert_encoding<streamed_type>(it, end); result)
795+
if (auto result = unicode::template convert_encoding<std::string>(it, end); result)
796796
{
797-
return detail::BasicStringConverter<streamed_type, T>::convert(*result);
797+
return detail::BasicStringConverter<std::string, T>::convert(*result);
798798
}
799799

800800
return std::nullopt;

‎test/types/string_converter.cpp

+4-65
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
#include "fly/traits/traits.hpp"
12
#include "fly/types/numeric/literals.hpp"
23
#include "fly/types/string/string.hpp"
34

@@ -54,12 +55,8 @@ CATCH_TEMPLATE_TEST_CASE(
5455
{
5556
using StringType = TestType;
5657
using BasicString = fly::BasicString<StringType>;
57-
using BasicStringTraits = fly::detail::BasicStringTraits<StringType>;
5858
using char_type = typename BasicString::char_type;
5959
using codepoint_type = typename BasicString::codepoint_type;
60-
using streamed_type = typename BasicString::streamed_type;
61-
using streamed_char = typename streamed_type::value_type;
62-
using ustreamed_char = std::make_unsigned_t<streamed_char>;
6360

6461
auto out_of_range_codepoint = []() -> StringType
6562
{
@@ -185,64 +182,6 @@ CATCH_TEMPLATE_TEST_CASE(
185182
}
186183
}
187184

188-
CATCH_SECTION("Convert a string to a Boolean")
189-
{
190-
StringType s;
191-
192-
s = FLY_STR(char_type, "0");
193-
CATCH_CHECK(BasicString::template convert<bool>(s) == false);
194-
195-
s = FLY_STR(char_type, "1");
196-
CATCH_CHECK(BasicString::template convert<bool>(s) == true);
197-
198-
s = FLY_STR(char_type, "-1");
199-
CATCH_CHECK_FALSE(BasicString::template convert<bool>(s));
200-
201-
s = FLY_STR(char_type, "2");
202-
CATCH_CHECK_FALSE(BasicString::template convert<bool>(s));
203-
204-
s = FLY_STR(char_type, "abc");
205-
CATCH_CHECK_FALSE(BasicString::template convert<bool>(s));
206-
207-
s = FLY_STR(char_type, "2a");
208-
CATCH_CHECK_FALSE(BasicString::template convert<bool>(s));
209-
}
210-
211-
CATCH_SECTION("Convert a string to a streamable character type")
212-
{
213-
StringType s;
214-
215-
s = FLY_STR(char_type, "0");
216-
CATCH_CHECK(BasicString::template convert<streamed_char>(s) == '\0');
217-
CATCH_CHECK(BasicString::template convert<ustreamed_char>(s) == '\0');
218-
219-
s = FLY_STR(char_type, "65");
220-
CATCH_CHECK(BasicString::template convert<streamed_char>(s) == 'A');
221-
CATCH_CHECK(
222-
BasicString::template convert<ustreamed_char>(s) == static_cast<ustreamed_char>(65));
223-
224-
s = FLY_STR(char_type, "abc");
225-
CATCH_CHECK_FALSE(BasicString::template convert<streamed_char>(s));
226-
CATCH_CHECK_FALSE(BasicString::template convert<ustreamed_char>(s));
227-
228-
s = FLY_STR(char_type, "2a");
229-
CATCH_CHECK_FALSE(BasicString::template convert<streamed_char>(s));
230-
CATCH_CHECK_FALSE(BasicString::template convert<ustreamed_char>(s));
231-
232-
if constexpr (BasicStringTraits::has_stoi_family_v)
233-
{
234-
CATCH_CHECK_FALSE(
235-
BasicString::template convert<streamed_char>(minstr<StringType, streamed_char>()));
236-
CATCH_CHECK_FALSE(
237-
BasicString::template convert<streamed_char>(maxstr<StringType, streamed_char>()));
238-
239-
CATCH_CHECK_FALSE(BasicString::template convert<ustreamed_char>(
240-
minstr<StringType, ustreamed_char>()));
241-
CATCH_CHECK_FALSE(BasicString::template convert<ustreamed_char>(
242-
maxstr<StringType, ustreamed_char>()));
243-
}
244-
}
245-
246185
CATCH_SECTION("Convert a string to an 8-bit integer")
247186
{
248187
StringType s;
@@ -267,7 +206,7 @@ CATCH_TEMPLATE_TEST_CASE(
267206
CATCH_CHECK_FALSE(BasicString::template convert<std::int8_t>(s));
268207
CATCH_CHECK_FALSE(BasicString::template convert<std::uint8_t>(s));
269208

270-
if constexpr (BasicStringTraits::has_stoi_family_v)
209+
if constexpr (fly::any_same_v<char_type, char, wchar_t>)
271210
{
272211
CATCH_CHECK_FALSE(
273212
BasicString::template convert<std::int8_t>(minstr<StringType, std::int8_t>()));
@@ -305,7 +244,7 @@ CATCH_TEMPLATE_TEST_CASE(
305244
CATCH_CHECK_FALSE(BasicString::template convert<std::int16_t>(s));
306245
CATCH_CHECK_FALSE(BasicString::template convert<std::uint16_t>(s));
307246

308-
if constexpr (BasicStringTraits::has_stoi_family_v)
247+
if constexpr (fly::any_same_v<char_type, char, wchar_t>)
309248
{
310249
CATCH_CHECK_FALSE(
311250
BasicString::template convert<std::int16_t>(minstr<StringType, std::int16_t>()));
@@ -343,7 +282,7 @@ CATCH_TEMPLATE_TEST_CASE(
343282
CATCH_CHECK_FALSE(BasicString::template convert<std::int32_t>(s));
344283
CATCH_CHECK_FALSE(BasicString::template convert<std::uint32_t>(s));
345284

346-
if constexpr (BasicStringTraits::has_stoi_family_v)
285+
if constexpr (fly::any_same_v<char_type, char, wchar_t>)
347286
{
348287
CATCH_CHECK_FALSE(
349288
BasicString::template convert<std::int32_t>(minstr<StringType, std::int32_t>()));

‎test/types/string_traits.cpp

-38
Original file line numberDiff line numberDiff line change
@@ -87,23 +87,6 @@ constexpr bool is_string_like(const T &)
8787
return false;
8888
}
8989

90-
template <
91-
typename StringType,
92-
fly::enable_if_all<typename fly::detail::BasicStringTraits<StringType>::has_stoi_family> = 0>
93-
constexpr int call_stoi(const StringType &str)
94-
{
95-
return std::stoi(str);
96-
}
97-
98-
template <
99-
typename StringType,
100-
fly::enable_if_not_all<typename fly::detail::BasicStringTraits<StringType>::has_stoi_family> =
101-
0>
102-
constexpr int call_stoi(const StringType &)
103-
{
104-
return -1;
105-
}
106-
10790
} // namespace
10891

10992
CATCH_TEMPLATE_TEST_CASE(
@@ -128,27 +111,6 @@ CATCH_TEMPLATE_TEST_CASE(
128111
constexpr bool is_string16 = std::is_same_v<StringType, std::u16string>;
129112
constexpr bool is_string32 = std::is_same_v<StringType, std::u32string>;
130113

131-
CATCH_SECTION("Check whether the STL defines the std::stoi family of functions via traits")
132-
{
133-
CATCH_CHECK(traits::has_stoi_family_v == (is_string || is_wstring));
134-
}
135-
136-
CATCH_SECTION(
137-
"Check whether the STL defines the std::stoi family of functions via SFINAE overloads")
138-
{
139-
const StringType s = FLY_STR(char_type, "123");
140-
const int i = call_stoi(s);
141-
142-
if constexpr (is_string || is_wstring)
143-
{
144-
CATCH_CHECK(i == 123);
145-
}
146-
else
147-
{
148-
CATCH_CHECK(i == -1);
149-
}
150-
}
151-
152114
CATCH_SECTION("Check whether types are supported strings via traits")
153115
{
154116
CATCH_SECTION("Plain data types")

0 commit comments

Comments
 (0)