Enforcing correct input/output of integers

Question

The problem:

1. Some types, like cstdint aliases or, at least purely theoretically, even std::size_t, may be (but don’t have to be) bound to char, signed char or unsigned char. In practice, std::int_least8_t seems to be often bound to signed char and std::uint_least8_t – to unsigned char. This seems odd, since semantically, all those types are supposed to represent integers rather than characters.
2. In case of variables of a character type, the >> operator writes the character code of the first non-whitespace character of the input stream into the variable, while the << operator outputs a character of the character code equal to the value of the variable.
3. Therefore it may happen that if we, for example, declare a variable as int_least8_t and use the input operator or the output operator on it, then the results will be far from expected. This seems to be even more awkward since whether such aliases are bound to character types or not depends only on the implementation and is a priori unknown.

An example of the above problems: http://ideone.com/JgyFhR A proof that purely theoretically, this can even be the case for std::size_t: https://stackoverflow.com/questions/32915434

I’ve attempted to write a generic solution to all those problems:

#ifndef INTIO_H
#define INTIO_H

#include <istream>
#include <ostream>
#include <type_traits>
#include <limits>

template
<class T, typename = typename std::enable_if<std::is_integral<T>::value>::type>
class integer_IO
{
  T &val;

public:

  integer_IO(T &arg) : val(arg) {}
  integer_IO(T &&arg) : val(arg) {}

  friend std::istream &operator>> (std::istream &is, integer_IO<T> &&i)
  {
    using TT = decltype(+i.val);
    TT hlp;
    is >> hlp;
    TT constexpr minval = static_cast<TT>(std::numeric_limits<T>::min());
    TT constexpr maxval = static_cast<TT>(std::numeric_limits<T>::max());
    i.val = static_cast<T>(hlp > maxval ? maxval : hlp < minval ? minval : hlp);
    if(hlp > maxval || hlp < minval)
      is.setstate(std::ios::failbit);
    return is;
  }

  friend std::ostream &operator<< (std::ostream &os, integer_IO<T> const &&i)
  {
    os << +i.val;
    return os;
  }
};

template
<class T, typename = typename std::enable_if<std::is_integral<T>::value>::type>
integer_IO<T> intIO(T &arg)
{
  return integer_IO<T>(arg);
}

template
<class T, typename = typename std::enable_if<std::is_integral<T>::value>::type>
integer_IO<T> intIO(T &&arg)
{
  return integer_IO<T>(arg);
}

#endif

The usage of this wrapper is simple:

#include <iostream>
#include <cstdint>
#include "intIO.hpp"
using namespace std;

int twoplustwo()
{
  return 2+2;
}

int main()
{
  int_least8_t i;
  cin >> intIO(i);
  cout << intIO(i) << '\n';
  cout << intIO(twoplustwo()) << '\n';
}

What is your opinion of my code?

Answer 1

Avoid name collisions

#ifndef INTIO_H
#define INTIO_H

You should ideally prefix the include guard by something that is unlikely to cause a collision with any one using your library. As it stands now it is at risk of collision if some one else unluckily choose to use INTIO_H as their include guard.

Hide implementation details

To me it looks like the integer_IO class is not directly intended to be used by the user. In that case you should wrap it in an internal namespace so that the primary namespace that the user sees isn't cluttered with internal functions.

Naming

I think that the name integer_IO is kind of mixing two different case standards. To my knowledge there are kind of two major standards emerging, one is to use CameCase in which you would name your class IntegerIO and the other one is to use inderscore instead of space and all lower case so you would have integer_io. If you're going to allow upper case in names, then I would at least suggest that you use leading upper case like so Integer_IO. Any of the above are easier on my eyes than the current name.

Also I think that the intIO functions would be better named asInt:

std::cin>>asInt(x);

Universal references

This here is a universal reference, the parameter arg will inherit whatever qualifiers the constructor was called with.

  integer_IO(T &&arg) : val(arg) {}

If the constructor was called with a temporary like so auto iio = integer_IO(x + y) where x and y are int arguments, then arg would deduce to int&&. Which is an r-value reference, you are then storing the pointer to this temporary into val which refers to... you guessed it something that might be dangling as soon as the constructor returns.

The other constructor:

  integer_IO(T &arg) : val(arg) {}

will not bind to temporaries because it must be able to take the address of the argument. Other than that it will bind to reference to const because T will be deduced to const T. So this constructor covers all legal use cases that the one with universal reference does, so you can remove either one. I suggest you nuke the universal one because it is slightly harder to understand.

Stream operators

Your two stream operators:

friend std::istream &operator>> (std::istream &is, integer_IO<T> &&i)
friend std::ostream &operator<< (std::ostream &os, integer_IO<T> const &&i)

are using r-value references (NOTE: They are not universal references). Note that const r-value reference doesn't make any sense.

Anyway, I assume that this is because you only want to allow usage on the form:

std::cout<<intIO(x);

and prohibit some one from doing:

auto foo = intIO(x);
std::cout<<foo;

However I do not see the point of prohibiting this use at the cost of making the implementation harder to read (I had to reason about those functions for quite some while before I was sure they work and why they work). You could just as well have

friend std::istream &operator>> (std::istream &is, integer_IO<T> i)
friend std::ostream &operator<< (std::ostream &os, const integer_IO<T>& i)

and your code would be easier to follow and would generate the same code. And allow the above usage syntax.

Reading into a temporary??

This tickled my funny bone:

template
<class T, typename = typename std::enable_if<std::is_integral<T>::value>::type>
integer_IO<T> intIO(T &&arg)
{
  return integer_IO<T>(arg);
}

As you are using universal references this means that arg will bind to anything and retain it's qualifiers. Even r-value references... which means that the following will compile and run doing some manner of undefined behaviour:

std::cin>>intIO(3);

Jolly.

In summary

I can see how this can have some uses in template code but I believe that your implementation is too complicated. My advice is to stop trying to be fancy with && and just make something that works.

I would probably have implemented it like this:

#include <istream>
#include <ostream>
#include <iostream>
#include <limits>
#include <type_traits>
#include <cstdint>

#define ENABLE_IF_INTEGRAL(T)\
typename = typename std::enable_if<std::is_integral<std::decay_t<T>>::value>::type

namespace detail{
    template<class T, ENABLE_IF_INTEGRAL(T)>
    struct wrapper{
        T& x;
        wrapper(T& xx) : x(xx){};

        friend std::istream& operator >> (std::istream& is, wrapper<T> w){
            auto read = +w.x;
            is>>read;
            w.x = static_cast<std::decay_t<T>>(read);
            if(w.x != read){
                is.setstate(std::ios::failbit);
                //C++11 behaviour
                constexpr auto max = std::numeric_limits<std::decay_t<T>>::max();
                constexpr auto min = std::numeric_limits<std::decay_t<T>>::min();
                w.x = static_cast<T>(read < min ? min : max);
            }
            return is;
        }
    };
}

template<class T, ENABLE_IF_INTEGRAL(T)>
auto intoInt(T& x){ return detail::wrapper<T>(x);}

template<class T, ENABLE_IF_INTEGRAL(T)>
auto asInt(T x){ return +x; }


#undef ENABLE_IF_INTEGRAL    

int oneplusone(){return 1 + 1;}

int main() {
    char foo;

    // Shouldn't compile
    //std::cin>>asInt(foo);
    //std::cin>>asInt(3);
    //std::cin>>intoInt(3);
    //std::cin>>intoInt(std::move(foo));
    //std::cout<<intoInt(std::move(foo));
    //std::cout<<intoInt(3);
    //std::cout<<intoInt(foo);

    // Readable into
    std::cin>>intoInt(foo);

    // Check status
    std::cout<< (std::cin.fail() ? "failed":"success")<<std::endl;

    // Binds to reference
    auto& ref = foo;
    std::cout<<asInt(ref)<<std::endl;

    // Binds to const reference
    const auto& cref = foo;
    std::cout<<asInt(cref)<<std::endl;

    // Binds to r-value reference
    std::cout<<asInt(std::move(foo))<<std::endl;

    // Binds to value returned function call (r-value really)
    std::cout<<asInt(oneplusone())<<std::endl;

    // Binds to literal
    std::cout<<asInt(31)<<std::endl;

    return 0;
}

In addition to being shorter and easier to comprehend, it also prohibits: std::cin<<asInt(3); from happening.