Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

kstream::to_string should use the STL directly in C++11 mode #39

Closed
sykhro opened this issue Mar 26, 2021 · 0 comments · Fixed by #50
Closed

kstream::to_string should use the STL directly in C++11 mode #39

sykhro opened this issue Mar 26, 2021 · 0 comments · Fixed by #50

Comments

@sykhro
Copy link

sykhro commented Mar 26, 2021

std::to_string is a thing since C++11. Using it in C++11 mode would also solve #28 (as long as you don't care about C++98, but it's still better than nothing)

generalmimon added a commit that referenced this issue Apr 7, 2022
* Replace kstream::to_string() - not just `int`, fix portability

See #8 (comment)

Fix #28,
close #39

* Simplify to_string(): accept `uint64_t` in unsigned_to_decimal()

* to_string(): only allow types whose absolute value fits into `uint64_t`

We only check `std::numeric_limits<I>::max()`, because `min()` is
negative for signed types, and we need its absolute value (so we can
compare it with `std::numeric_limits<uint64_t>::max()`). However, this
value won't fit into the same signed type in two's complement
representation, so we need to use at least an unsigned type of the same
width - but in GCC, `std::make_unsigned<__int128_t>` is not available,
and `uintmax_t` is `uint64_t`, not `__uint128_t`. I don't know how to
get the "real" `uintmax_t` including implementation-specific extended
types like `__uint128_t` - maybe there's no platform-independent way to
do it.

Fortunately, we don't have to deal with this for the `max()` case at
all, which works just fine, so we'll at least check that.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant