Safe Integer Library in C++

Question

I've designed a single-file safe integer library in C++. It catches undefined behavior prior to integer overflows or underflows and throws the respective exceptions. I intend this to be portable, to not rely on undefined behavior, and to rely as little on implementation-defined behavior as possible.

safe_integer.hpp:

/*
 *  Copyright © 2020 James Larrowe
 *
 *  This program is free software: you can redistribute it and/or modify
 *  it under the terms of the GNU General Public License as published by
 *  the Free Software Foundation, either version 2 of the License, or
 *  (at your option) any later version.
 *
 *  This program is distributed in the hope that it will be useful,
 *  but WITHOUT ANY WARRANTY; without even the implied warranty of
 *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
 *  GNU General Public License for more details.
 *
 *  You should have received a copy of the GNU General Public License
 *  along with this program.  If not, see <https://www.gnu.org/licenses/>.
 */

#ifndef SAFE_INTEGER_HPP
# define SAFE_INTEGER_HPP 1

#include <limits>
#include <stdexcept>
#include <type_traits>

template<typename I,
         typename std::enable_if<std::is_integral<I>::value, bool>::type = true>
class safe_int
{
    I val;

public:
    typedef I value_type;
    static constexpr I max = std::numeric_limits<I>::max();
    static constexpr I min = std::numeric_limits<I>::min();

    safe_int(I i) : val { i } { };

    operator I() const { return val; }

    I &operator=(I v) { val = v; }

    safe_int operator+()
    {
        return *this;
    }

    safe_int operator-()
    {
        if(val < -max)
            throw std::overflow_error("");

        return safe_int(-val);
    }

    safe_int &operator++()
    {
        if(val == max)
            throw std::overflow_error("");

        ++val;

        return *this;
    }

    safe_int &operator--()
    {
        if(val == min)
            throw std::underflow_error("");

        --val;

        return *this;
    }

    safe_int operator++(int)
    {
        if(val == max)
            throw std::overflow_error("");

        return safe_int(val++);
    }

    safe_int operator--(int)
    {
        if(val == min)
            throw std::underflow_error("");

        return safe_int(val--);
    }

    safe_int &operator+=(I rhs)
    {
        if( val > 0 && rhs > max - val )
            throw std::overflow_error("");
        else if( val < 0 && rhs < min - val )
            throw std::underflow_error("");

        val += rhs;
        return *this;
    }

    safe_int &operator-=(I rhs)
    {
        if( val >= 0 && rhs < -max )
            throw std::overflow_error("");

        if( val < 0 && rhs > max + val )
            throw std::overflow_error("");
        else if( val > 0 && rhs < min + val )
            throw std::underflow_error("");

        val -= rhs;
        return *this;
    }

    safe_int &operator*=(I rhs)
    {
        if(val > 0)
        {
            if(rhs > max / val)
                throw std::overflow_error("");
        }
        else if(val < 0)
        {
            if(val == -1)
            {
                if(rhs < -max)
                    throw std::overflow_error("");
                goto no_overflow;
            }

            if(rhs > min / val)
                throw std::underflow_error("");
        }
no_overflow:
        val *= rhs;
        return *this;
    }

    safe_int &operator/=(I rhs)
    {
        if( rhs == -1 && val < -max )
            throw std::underflow_error("");
        else if(rhs == 0)
            throw std::domain_error("");

        val /= rhs;
        return *this;
    }

    safe_int &operator%=(I rhs)
    {
        if( rhs == -1 && val < -max )
            throw std::underflow_error("");
        else if(rhs == 0)
            throw std::domain_error("");

        val %= rhs;
        return *this;
    }

    safe_int operator+(I rhs)
    {
        return safe_int(val) += rhs;
    }

    safe_int operator-(I rhs)
    {
        return safe_int(val) -= rhs;
    }

    safe_int operator*(I rhs)
    {
        return safe_int(val) *= rhs;
    }

    safe_int operator/(I rhs)
    {
        return safe_int(val) /= rhs;
    }

    safe_int operator%(I rhs)
    {
        return safe_int(val) %= rhs;
    }

    safe_int &operator+=(safe_int rhs)
    {
        return *this += static_cast<I>(rhs);
    }

    safe_int &operator-=(safe_int rhs)
    {
        return *this -= static_cast<I>(rhs);
    }

    safe_int &operator*=(safe_int rhs)
    {
        return *this *= static_cast<I>(rhs);
    }

    safe_int &operator/=(safe_int rhs)
    {
        return *this /= static_cast<I>(rhs);
    }

    safe_int &operator%=(safe_int rhs)
    {
        return *this %= static_cast<I>(rhs);
    }

    safe_int operator+(safe_int rhs)
    {
        return safe_int(val) += static_cast<I>(rhs);
    }

    safe_int operator-(safe_int rhs)
    {
        return safe_int(val) -= static_cast<I>(rhs);
    }

    safe_int operator*(safe_int rhs)
    {
        return safe_int(val) *= static_cast<I>(rhs);
    }

    safe_int operator/(safe_int rhs)
    {
        return safe_int(val) /= static_cast<I>(rhs);
    }

    safe_int operator%(safe_int rhs)
    {
        return safe_int(val) %= static_cast<I>(rhs);
    }
};

#endif

This should work on non-two's complement systems and with any integer type.

Here's a little example:

#include "safe_integer.hpp"

int main(void)
{
    safe_int<int> i = 0;

    i -= -0x80000000;

    return 0;
}

Output:

terminate called after throwing an instance of 'std::overflow_error'
  what():  
Aborted

What I'm particularly interested in:

• Are there any corner cases I've missed?

• Is there any undefined behavior (probably not)?

• Is there any way I can simplify all of the (somewhat redundant) operator overloads?

What I'm not interested in:

• efficiency. I agree that my solution may not perform well but my personal opinion is that leaving the checks in unconditionally and not using undefined behavior to get a faster result are worth the cost.

Answer 1

You can get rid of the goto in operator*= by adding an else to the if (val == -1) statement.

The % operator cannot underflow, as the result always has a magnitude less than the rhs value. So you don't need your underflow check (which is incorrect anyways, as it would throw rather than return a 0).

An "underflow" represents a number that is too small to represent, and is typically applied to floating point types. A calculation that gives a number that is negative and too large to store in the result (i.e., is less than min) is still an overflow, as the result has overflowed the storage space available. So all those places that you throw an underflow_error should be overflow_error (unless you're changing the usage of underflow to represent too large of a negative value).

How does the code behave if I instantiate a safe_int<unsigned>? The evaluation of -max in that case will not give the correct result, and possibly cause a compiler warning (for negation of an unsigned value).

Answer 2

Code Review

/*
 *  Copyright © 2020 James Larrowe
 *
 *  This program is free software: you can redistribute it and/or modify
 *  it under the terms of the GNU General Public License as published by
 *  the Free Software Foundation, either version 2 of the License, or
 *  (at your option) any later version.
 *
 *  This program is distributed in the hope that it will be useful,
 *  but WITHOUT ANY WARRANTY; without even the implied warranty of
 *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
 *  GNU General Public License for more details.
 *
 *  You should have received a copy of the GNU General Public License
 *  along with this program.  If not, see <https://www.gnu.org/licenses/>.
 */

You should probably note that this code is now also available under the Creative Commons license which you agreed to by posting it on this page. See the base of the page for details.

user contributions licensed under cc by-sa 4.0 with attribution required. rev 2020.1.30.35920

---

In the old days this was not legal (the space after the hash). But I am not sure if this has been updated.

# define SAFE_INTEGER_HPP 1

Anyway it looks untidy :-)

---

Here you have undefined behavior.

    I &operator=(I v) { val = v; }

You are supposed to return a reference yet don't return anything.

Also I am concerned about the return type. Why are you returning a reference to the internal integer? Should that not be a reference to the safe_int? Is this some kind of pre-mature optimization?

It has to be an int so passing by value seems OK until you come along and adapt the class for some other type (might be useful for std::complex<int>?). Then passing by value is no longer a good solution.

Assume somebody will use your class in a way you have not anticipated and thus program defensively. Pass parameters by const reference unless you know that forever it is only going to be an int and you can't know that because its an I.

---

Are you sure that's a a safe test?

    safe_int operator-()
    {
        if(val < -max)
            throw std::overflow_error("");

        return safe_int(-val);
    }

What about signed integer?
Do all systems use integers where the std::abs(min) > max? Not sure I know that answer.

---

Hard to spot that &.

    safe_int &operator++()

In C++ (unlike C) the & and the * are usually placed with the type information. So it would normally be written as:

    safe_int& operator++()

---

You could think of no other way of writing that without the goto?

    safe_int &operator*=(I rhs)
    {
                goto no_overflow;
no_overflow:
        val *= rhs;
        return *this;
    }

Answer 3

A useful idea; thanks for sharing it with us.

Limitations and bugs

I have a nagging doubt about the implicit conversion to/from value_type. There are cases where we could have unchecked arithmetic when we expected it to be checked. For example, given:

safe_int<int> i = std::numeric_limits<int>::max();
int j = i;

With the above, I find that i + j throws, but j + i continues with its undefined behaviour. I think we'd prefer both expressions to throw.

I admit I was slightly surprised that the above didn't reach an ambiguous overload. However, long j = i causes i + j to be ambiguous and j + i to be unchecked; that's no better IMO.

All in all, I think the safety would be greatly improved if we declare the conversion to unsafe with the explicit keyword:

explicit operator value_type() const noexcept { return val; }
value_type value() const noexcept{ return val; }

The value() accessor is useful as it allows us to avoid numerous static_cast operators (which always need closer inspection).

---

We're missing promotions from narrower safe_int types. For example, this code ought to be valid, but isn't:

safe_int<int> i = std::numeric_limits<int>::max();
safe_int<long> j = i;

We need some converting constructors such as these (I'll use Concepts syntax, as it's easier to read than lots of std::enable_if - adapt as necessary if that's not available to you):

template<typename T>
      requires std::is_assignable_v<value_type&,T>
safe_int(T t = {})
      noexcept(std::is_nothrow_assignable_v<value_type&,T>)
    : val{t}
{}

template<typename T>
      requires std::is_assignable_v<value_type&,T>
safe_int(safe_int<T> t)
      noexcept(std::is_nothrow_assignable_v<value_type&,T>)
    : val{static_cast<value_type>(t)}
{}

We also need binary operations that promote to safe_int of the common type of the arguments (for i+j and j+i to both work); this is my quick attempt:

template<typename T, typename U>
safe_int<std::common_type_t<T,U>> operator+(safe_int<T> a, safe_int<U> b)
{
    using V = std::common_type_t<T,U>;
    return safe_int<V>(std::move(a)) + safe_int<V>(std::move(b));
}

template<typename T, typename U>
    requires std::is_integral_v<U>
safe_int<std::common_type_t<T,U>> operator+(safe_int<T> a, U b)
{
    using V = std::common_type_t<T,U>;
    return safe_int<V>(std::move(a)) + safe_int<V>(b);
}

template<typename T, typename U>
    requires std::is_integral_v<U>
safe_int<std::common_type_t<T,U>> operator+(U a, safe_int<T> b)
{
    return std::b + a;
}

---

Style

This seems old-fashioned:

typedef I value_type;

Modern C++ authors prefer using:

using value_type = I;

Instead of static_cast of arguments to the assignment operators, just use the val member directly:

safe_int &operator+=(safe_int rhs)
{
    return *this += rhs.val;
}

There's a stray ; after the converting constructor.

Answer 4

You might want to look at https://github.com/dcleblanc/SafeInt/. The biggest area you're missing is the whole problem of mixed types and operator casts.