6
\$\begingroup\$

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.
\$\endgroup\$
7
\$\begingroup\$

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

| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ I'm curious because you always say negation of unsigned values may cause a compiler warning: Does your compiler warn you about this? It should just convert the negative value into an unsigned one using the documented conversion rules, no warning necessary. \$\endgroup\$ – S.S. Anne Jan 30 at 0:49
  • \$\begingroup\$ @S.S.Anne MSVC can issue "warning C4146: unary minus operator applied to unsigned type, result still unsigned". \$\endgroup\$ – 1201ProgramAlarm Jan 30 at 4:33
  • \$\begingroup\$ That's ridiculous. I've never heard of GCC doing such a thing, nor would I ever like to. But anyway, I've fixed that by adding std::is_signed<I> && inside the loop. \$\endgroup\$ – S.S. Anne Jan 30 at 11:57
5
\$\begingroup\$

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;
    }
| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ Eh, don't care about licensing. People can use it with the GPL or CC-by-SA 4.0, as long as they use it. \$\endgroup\$ – S.S. Anne Jan 30 at 16:00
  • \$\begingroup\$ Space after hash is fine. \$\endgroup\$ – S.S. Anne Jan 30 at 16:01
  • \$\begingroup\$ Unary - on val only overflows when val is min and min is less than -max. \$\endgroup\$ – S.S. Anne Jan 30 at 16:03
  • \$\begingroup\$ Fixed goto. I think I wrote that one when I was tired. \$\endgroup\$ – S.S. Anne Jan 30 at 16:05
  • 1
    \$\begingroup\$ "In the old days this was not legal (the space after the hash). But I am not sure if this has been updated." Yeah it was updated in the fancy new ISO standardization from 1989/1990 :) \$\endgroup\$ – Lundin Jan 31 at 12:32
3
\$\begingroup\$

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.

| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ Thanks -- my compiler doesn't support C++20, nor concepts. I'm not terribly familiar with them, so do you mind writing it out for the "lots of enable_if" approach? \$\endgroup\$ – S.S. Anne Jan 31 at 12:22
  • \$\begingroup\$ I don't have time right now for that (and that's why the code I show isn't thoroughly tested - it's just intended as a guide to the right direction). The Concepts syntax isn't too hard to follow - just look for the lines with requires. \$\endgroup\$ – Toby Speight Jan 31 at 12:27
  • \$\begingroup\$ Thanks -- I've implemented most of these. How can I write two constructors that covert from bigger types and check for narrowing? \$\endgroup\$ – S.S. Anne Feb 8 at 0:22

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.