patterncppMinor
Portable safe unsigned integer arithmetic
Viewed 0 times
unsignedsafeportableintegerarithmetic
Problem
tl;dr The purpose of the code is to allow writing
in a portable way that won't invoke undefined behavior. If that is enough for you to understand the problem at hand, you are welcome to skip the first two sections and jump straight to the section titled “Code for Review”.
Otherwise, the section titled “Problem Statement” explains why simply writing
Problem Statement
In a recent discussion about a question on another Stack Exchange site, I was made aware that seemingly innocent arithmetic expressions of
On my system, it prints
Fortunately, GCC catches the problem at compile-time
In the case
constexpr auto x = std::uint16_t { UINT16_MAX };
constexpr auto y = std::uint16_t { UINT16_MAX };
static_assert(multiply(x, y) == 1u, "broken math");in a portable way that won't invoke undefined behavior. If that is enough for you to understand the problem at hand, you are welcome to skip the first two sections and jump straight to the section titled “Code for Review”.
Otherwise, the section titled “Problem Statement” explains why simply writing
x * y would not have done the trick and the section “Proposed Solution” provides background information about the implementation of the multiply function I'm asking review for.Problem Statement
In a recent discussion about a question on another Stack Exchange site, I was made aware that seemingly innocent arithmetic expressions of
unsigned integral types in C++ can easily invoke undefined behavior.const auto x = std::uint16_t { UINT16_MAX };
const auto y = std::uint16_t { UINT16_MAX };
std::cout << x * y << '\n'; // undefined behavior on 32 bit platformsOn my system, it prints
-131071 when the intuitively expected result would be \$(2^{16}-1)^2\bmod{2^{16}}=65\,535^2\bmod{65\,536}=4\,294\,836\,225\bmod{65\,536}=1\$.Fortunately, GCC catches the problem at compile-time
main.cxx:10:20: warning: integer overflow in expression [-Woverflow]
std::cout
and (after the example is changed to use run-time expressions), compiling with -fsanitize=undefined makes the program trap at run-time.
main.cxx:10:20: runtime error: signed integer overflow: 65535 * 65535 cannot be represented in type 'int'
The reason for the undefined behavior is that § 5 ¶ 10.5 of the C++ standard (quoting N4140) mandates that integral promotion shall be performed on the operands of an expression. § 4.5 specifies that an integral type of rank less than int shall be promoted to signed int if it can hold all its values or to unsigned int` otherwise.In the case
Solution
I would rename
add, etc. to plus, minus, multiplies and divides to match the functor names from `, as you've done for the bit variants. You could also consider adding variants for comparisons (equal_to...) and logical operations (logical_and...) You could also provide a void specialization to allow the type to be deduced. So promotion::add could be passed to std::transform without specifying the type. That would probably require turning the functions into functors (i.e, classes).
One thing I find annoying is this: -> decltype(T(promote(lhs) + promote(rhs))). It's not necessary with C++14 return type deduction. So it just adds noise. Actually, is auto even necessary here? You're constructing a T so just make the return type T.
On on a similar note, I would also remove const from the parameters. Either pass by const& or by value. In all three options, you can't modify the caller's arguments, but the const just adds noise.
And one note (not a suggestion), you could replace typename with class`. It's less typing and what library devs do.template
constexpr T add(T lhs, T rhs) noexcept
{
return T(promote(lhs) + promote(rhs));
}Code Snippets
template <class T>
constexpr T add(T lhs, T rhs) noexcept
{
return T(promote(lhs) + promote(rhs));
}Context
StackExchange Code Review Q#127803, answer score: 3
Revisions (0)
No revisions yet.