MingyuZhong commented on a change in pull request #8143:
URL: https://github.com/apache/arrow/pull/8143#discussion_r485885307
##########
File path: cpp/src/arrow/util/decimal.cc
##########
@@ -42,34 +45,40 @@ namespace arrow {
using internal::SafeLeftShift;
using internal::SafeSignedAdd;
+#ifdef __SIZEOF_INT128__
+using uint128_t = __uint128_t;
+#else
+using boost::multiprecision::uint128_t;
+#endif
Review comment:
Moved to int128_internal.h. Unfortunately, __int128_t and boost int128_t
are not interchangeable. For example, operator<<(std::ostream, __int128_t) is
not defined, which means I can't replace int128_t in decimal_test.cc. In
addition, there are some subtle behavior differences or perhaps bugs in boost
int128_t (e.g., static_cast<uint64_t>(boost int128_t) returns the max value of
uint64_t when the input has more than 64 bits in some of the configs, while
static_cast<uint64_t>(__int128_t) returns the lower 64 bits). Do you think it's
better to leave this alias within decimal.cc due to the limitations?
Also reverted the changes to testing submodule.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
