Peer-review: `develop` branch on `5aa7c7f`

[GuillaumeDua]

Reviews: Boost.Int128

• Reviewer: Guillaume Dua
• Date: 2026-07-28 - 2026-07-30
• Repo: cppalliance/int128
• Branch: develop, on 5aa7c7f
• Testing environment: WSL2
  • OS: x86_64 Ubuntu 24.04.3 LTS
  • Compilers: GCC 16.0.1, Clang 22.1.8
  • PP directive: -DBOOST_INT128_NO_BUILTIN_INT128 (see below)
  • Builder: CMake 3.28.3

---

Summary

Important

Verdict: accepted, conditionally

• Design: Beside caveats described hereafter, the API feels intuitive and simple for common use-cases. constexpr (especially for C++14) and NTTP support are expected yet great additions too.
• Documentation: https://develop.int128.cpp.al/ looks way better than what I'm used to with other Boost libraries (before its modernisation). The search bar and preview are convenient/efficient.
• Testing: Looks very decent.
• Accessibility: The standalone header working on godbolt (compilation time) is great to experiment fast.

Disclaimers

• I did not audit the numerical algorithms behind the operators (Knuth's Algorithm D and friends).
• This review is written from a user's point of view: the public API, and the behaviors most users will actually expect/hit.
• I tested the library for my personnal use-cases, using C++23 not C++14.
• This review is partial, as a best-effort on my spare time.

Content

Bugs & suprising behaviors

1. Inconsistent conversion to bool

Files:

• detail/uint128_imp.hpp:115-121
• detail/int128_imp.hpp:109-115

Demo: https://godbolt.org/z/1P9szYT4z

constexpr boost::int128::uint128_t value{1, 0}; // same with int128_t

constexpr bool a = value;   // copy-initialization
static_assert(false == a);  // unexpected !

constexpr bool b(value);    // direct-initialization
static_assert(b);           // ok

constexpr bool c = static_cast<bool>(value);
static_assert(c);

if constexpr (value){
    static_assert(true);
}

f(value);

std::vector<bool> vec;
vec.push_back(value); // vec[0] == false

// as a comparison, with compilers intrinsinc __int128:
constexpr unsigned __int128 u = (unsigned __int128)1 << 64;
constexpr bool d = u;
static_assert(d);

Details:

• bool satisfies is_integral && is_unsigned, so the templated constrained operator UnsignedInteger() is a viable conversion to bool and returns static_cast<bool>(low).
• In copy-initialization the explicit operator bool() is not a candidate, so the template is the only one, and it drops high, resulting in a behavior that feels very unexpected/inconsistent.

See

// include/boost/int128/detail/traits.hpp:72
#define BOOST_INT128_DEFAULTED_UNSIGNED_INTEGER_CONCEPT typename UnsignedInteger, std::enable_if_t<detail::is_unsigned_integer_v<UnsignedInteger>, bool> = true

// include/boost/int128/detail/traits.hpp:32-47
template <typename T>
struct unsigned_integer
{
    static constexpr bool value = (std::is_unsigned<T>::value && std::is_integral<T>::value)
    #ifdef BOOST_INT128_HAS_INT128
    || std::is_same<T, builtin_u128>::value;
    #else
    ;
    #endif
};
template <typename T>
BOOST_INT128_INLINE_CONSTEXPR bool is_unsigned_integer_v = unsigned_integer<T>::value;

2. Preprocessor-directive side-effects

Headers disable -Wsign-conversion for the rest of the TU, which can have impacts on user-side by silencing relevant/important warnings.

Files:

• detail/uint128_imp.hpp:403-408 pushes and never pops.
• detail/int128_imp.hpp (with Clang) also nets one extra push: pushes at the line 445 (-Wsign-conversion, -Wsign-compare) and 2151 (-Wassume), single pop at line 2371.

Demo: https://godbolt.org/z/vren91jKb

// g++     : warning: conversion to 'unsigned int' from 'int' may change the sign of the result [-Wsign-conversion]
// clang++ : warning: implicit conversion changes signedness: 'int' to 'unsigned int' [-Wsign-conversion]
unsigned f(int x){ return x; }

#include <boost/int128.hpp>

unsigned g(int x){ return x; } // no warnings even with `-Wsign-conversion`

auto main() -> int {
    [[maybe_unused]] auto f_result = f(42);
    [[maybe_unused]] auto g_result = g(42);
}

3. saturating_mul wrong branch

Disclaimer: Not a math guy, but from my understanding:

saturating_mul(a, b); // == a * b,                      whenever a * b fits
saturating_mul(a, b); // == BOOST_INT128_UINT128_MAX,   otherwise

Demo: https://godbolt.org/z/Y6TW9458e

constexpr auto value = boost::int128::uint128_t{1} << 127;
static_assert(BOOST_INT128_UINT128_MAX == boost::int128::saturating_mul(value, boost::int128::uint128_t{1}));
// expected: BOOST_INT128_UINT128_MAX == value

4. BOOST_INT128_NO_BUILTIN_INT128 gets ignored if/when BOOST_HAS_INT128 is defined

What:

• Opt-out built-ins feels misleading/broken in some case

File:

• detail/config.hpp:28
• doc/modules/ROOT/pages/config.adoc:28

Demo: https://godbolt.org/z/8faorTs8c

#if (defined(BOOST_HAS_INT128) || (defined(__SIZEOF_INT128__) && !defined(_MSC_VER)) && !defined(BOOST_INT128_NO_BUILTIN_INT128)) && !defined(__SYCL_DEVICE_ONLY__)
# pragma message("BOOST_INT128_HAS_INT128")
#else
# pragma message("no built-in")
#endif

# commands
g++ -std=c++23 -O2 test.cpp
g++ -std=c++23 -O2 -DBOOST_INT128_NO_BUILTIN_INT128 test.cpp
g++ -std=c++23 -O2 -DBOOST_HAS_INT128 -DBOOST_INT128_NO_BUILTIN_INT128 test.cpp

note: '#pragma message: BOOST_INT128_HAS_INT128'
note: '#pragma message: no built-in'
note: '#pragma message: BOOST_INT128_HAS_INT128'

Suggested fix:

#if (defined(BOOST_HAS_INT128) || (defined(__SIZEOF_INT128__) && !defined(_MSC_VER)) && !defined(BOOST_INT128_NO_BUILTIN_INT128)) && !defined(__SYCL_DEVICE_ONLY__)

should probably add parenthesis around ||, see demo https://godbolt.org/z/bWKMbK1EE:

#if (defined(BOOST_HAS_INT128) || (defined(__SIZEOF_INT128__) && !defined(_MSC_VER))) && \
    !defined(BOOST_INT128_NO_BUILTIN_INT128) && \
    !defined(__SYCL_DEVICE_ONLY__)

note: '#pragma message: BOOST_INT128_HAS_INT128'
note: '#pragma message: no built-in'
note: '#pragma message: no built-in'

5. boost::random::traits::make_unsigned<int128_t>::type result in int128_t

File:

• include/boost/int128/random.hpp:39.

What:

• This feels unintuitive/misleading

  template <>
  struct make_unsigned<int128::int128_t>
  {
      using type = int128::int128_t;
  };

Suggested fix:

template <>
struct make_unsigned<int128::int128_t>
{
    using type = int128::uint128_t;
    //                   ^--- unsigned
};

API

Ambiguous overload resolutions: __int128 vs. uint128_t

Demo: https://godbolt.org/z/fxbThdfeT

boost::int128::uint128_t v{0, 5};
auto x = v + 1.0; // ambiguous overload for operator+
auto y = static_cast<double>(v) + 1.0;

Ambiguous overload resolutions feel like inconsistency when comparing uint128_t to __int128 usage:

expression	uint128_t	unsigned __int128	diagnostic (GCC 16)
v + 1.0	error	ok	ambiguous overload for 'operator+' (operand types are 'uint128_t' and 'double')
v < 1.0	error	ok	ambiguous overload for 'operator<'
double d; d += v;	error	ok	ambiguous overload for 'operator+='
int i; i |= v;	error	ok	ambiguous overload for 'operator|='
switch (v)	error	ok	ambiguous default type conversion from 'uint128_t'
u128 <=> i128	error	-	ambiguous overload for 'operator<=>'

Note: Clang 22 gives the equivalent errors.

Division by zero: binary operations are inconsistent with operator overloads

File:

• include/boost/int128/cstdlib.hpp:25
• include/boost/int128/detail/int128_imp.hpp:2206

Demo: https://godbolt.org/z/3YdMv3h7x

// assuming -std=c++23 -O2 -fsanitize=undefined
boost::int128::uint128_t v{0, 5};
boost::int128::div(boost::int128::uint128_t{7}, boost::int128::uint128_t{0});

// runtime error: execution reached an unreachable program point
boost::int128::uint128_t{7} / boost::int128::uint128_t{0}; // -> UB: __builtin_unreachable()

Suggested fix:

• Running tests with a sanitizers matrix is costly, but can highlight bugs.

Formatting support: questionable dependency to Boost.Decimal

File:

• include/boost/int128/format.hpp:8

Considering the guard

#if __has_include(<format>) && defined(__cpp_lib_format) && __cpp_lib_format >= 201907L && !defined(BOOST_DECIMAL_DISABLE_CLIB)

then BOOST_INT128_HAS_FORMAT won't be defined if BOOST_DECIMAL_DISABLE_CLIB is defined, which feels misleading/questionable.

Neat

• boost::int128::uint128_t feels odd to write -> uint128 type nested in a int128 namespace.

[mborland]

Thank you for the review. I'll look into each of the bugs listed tomorrow. Looks like many of them will be pretty straightforward to fix.

Your last point, yes, boost::int128::uint128 is awkward, but individual libraries are not supposed to inject things directly into the boost:: namespace.

[mborland]

For the switch statement support the only way to make that happen would be for every conversion operator to be explicit except for one, which then breaks things in the other direction. switch (static_cast<std::uint64_t>(v)) is portable