HiveBrain v1.2.0
Get Started
← Back to all entries
patterncppMinor

Portable safe unsigned integer arithmetic

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
unsignedsafeportableintegerarithmetic

Problem

tl;dr The purpose of the code is to allow writing

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 platforms


On 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.