Enforcing correct input/output of integers

The problem:

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

• 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






• 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 
#include 
#include 
#include 

template
::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 &&i)
  {
    using TT = decltype(+i.val);
    TT hlp;
    is >> hlp;
    TT constexpr minval = static_cast(std::numeric_limits::min());
    TT constexpr maxval = static_cast(std::numeric_limits::max());
    i.val = static_cast(hlp > maxval ? maxval : hlp  maxval || hlp  const &&i)
  {
    os ::value>::type>
integer_IO intIO(T &arg)
{
  return integer_IO(arg);
}

template
::value>::type>
integer_IO intIO(T &&arg)
{
  return integer_IO(arg);
}

#endif

The usage of this wrapper is simple:

``
#include
#include
#in

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 &&i)
friend std::ostream &operator 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 i)
friend std::ostream &operator& 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
::value>::type>
integer_IO intIO(T &&arg)
{
  return integer_IO(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
#include
#include
#include
#include
#include

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

namespace detail{
template
struct wrapper{
T& x;
wrapper(T& xx) : x(xx){};

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

template
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(foo);

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

// Binds to refe

StackExchange Code Review Q#146669, answer score: 3