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?

• Emily L.: Why is this better than cout<<(int) i?
• gaazkam: @EmilyL. Because in your example you need to know the type of i, or otherwise you cannot know what type to cast to (if, for example, i was a long long int, casting it to int would be wrong). My code is simply more generic, it allows to do input/output correctly without having to care about types
• Emily L.: If it were a long long int you wouldn't need a cast any way so I'm that case you're just adding unnecessary code and complexity.
• gaazkam: @EmilyL. True; but I'd have to know it is a long long int in the first place. With my solution, you don't have to care; everything you need to know or care about i is that it is an integral type; its exact width is irrelevant.

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.

• gaazkam: You recommend the abandonment of the &&s... but there is one problem. W/o the &&s we cannot, for example, print out the results of a function and code like this fails: int twoplustwo() {return 2+2;} int main(std::cout << asInt(twoplustwo()) << '\n'; ideone.com/GkgEcJ
• gaazkam: Actually, my first implementation of this wrapper didn't have the &&s... but because of the above problem, I finally added the &&s... Are you sure it was a bad idea?
• Emily L.: @gaazkam This seems to be a inherent flaw in this general design. As we are required to store a reference to the base type in the wrapper class, this requires that the datum we're wrapping must not be a temporary object. Because storing the address of a temporary beyond the scope of the temporary is UB. Your code compiles and runs but has UB and allows cin>>asInt(3) which is nonsense. Mine is free from UB but doesn't compile for your example. I will see if there is something I can do.
• Emily L.: @gaazkam The only plausible solution I could find on a quick look is to separate input and output. I have updated the example in my answer.
• gaazkam: Many thanks. But, there is one more thing. In case of overflow, your implementation simply casts the original value into the smaller type - so if, for example, we try to read 256 into a char, we might have 0 in our variable: ideone.com/U6bhlk