696. istream::operator>>(int&) broken

Section: 31.7.5.3.2 [istream.formatted.arithmetic] Status: C++11 Submitter: Martin Sebor Opened: 2007-06-23 Last modified: 2016-01-28

Priority: Not Prioritized

View all other issues in [istream.formatted.arithmetic].

View all issues with C++11 status.

Discussion:

From message c++std-lib-17897:

The code shown in 31.7.5.3.2 [istream.formatted.arithmetic] as the "as if" implementation of the two arithmetic extractors that don't have a corresponding num_get interface (i.e., the short and int overloads) is subtly buggy in how it deals with EOF, overflow, and other similar conditions (in addition to containing a few typos).

One problem is that if num_get::get() reaches the EOF after reading in an otherwise valid value that exceeds the limits of the narrower type (but not LONG_MIN or LONG_MAX), it will set err to eofbit. Because of the if condition testing for (err == 0), the extractor won't set failbit (and presumably, return a bogus value to the caller).

Another problem with the code is that it never actually sets the argument to the extracted value. It can't happen after the call to setstate() since the function may throw, so we need to show when and how it's done (we can't just punt as say: "it happens afterwards"). However, it turns out that showing how it's done isn't quite so easy since the argument is normally left unchanged by the facet on error except when the error is due to a misplaced thousands separator, which causes failbit to be set but doesn't prevent the facet from storing the value.

[ Batavia (2009-05): ]

We believe this part of the Standard has been recently adjusted and that this issue was addressed during that rewrite.

Move to NAD.

[ 2009-05-28 Howard adds: ]

I've moved this issue from Tentatively NAD to Open.

The current wording of N2857 in 28.3.4.3.2.3 [facet.num.get.virtuals] p3, stage 3 appears to indicate that in parsing arithmetic types, the value is always set, but sometimes in addition to setting failbit.

However there is a contradictory sentence in 28.3.4.3.2.3 [facet.num.get.virtuals] p1.

31.7.5.3.2 [istream.formatted.arithmetic] should mimic the behavior of 28.3.4.3.2.3 [facet.num.get.virtuals] (whatever we decide that behavior is) for int and short, and currently does not. I believe that the correct code fragment should look like: 

typedef num_get<charT,istreambuf_iterator<charT,traits> > numget;
iostate err = ios_base::goodbit;
long lval;
use_facet<numget>(loc).get(*this, 0, *this, err, lval);
if (lval < numeric_limits<int>::min())
{
  err |= ios_base::failbit;
  val = numeric_limits<int>::min();
}
else if (lval > numeric_limits<int>::max())
{
  err |= ios_base::failbit;
  val = numeric_limits<int>::max();
}
else
  val = static_cast<int>(lval);
setstate(err);

[ 2009-07 Frankfurt ]

Move to Ready.

Proposed resolution:

Change 28.3.4.3.2.3 [facet.num.get.virtuals], p1:

-1- Effects: Reads characters from in, interpreting them according to str.flags(), use_facet<ctype<charT> >(loc), and use_facet< numpunct<charT> >(loc), where loc is str.getloc(). If an error occurs, val is unchanged; otherwise it is set to the resulting value.

Change 31.7.5.3.2 [istream.formatted.arithmetic], p2 and p3: 

operator>>(short& val);

-2- The conversion occurs as if performed by the following code fragment (using the same notation as for the preceding code fragment): 

typedef num_get<charT,istreambuf_iterator<charT,traits> > numget;
iostate err = iostate_base::goodbit;
long lval;
use_facet<numget>(loc).get(*this, 0, *this, err, lval);
if (err != 0)
  ;
else if (lval < numeric_limits<short>::min()
  || numeric_limits<short>::max() < lval)
     err = ios_base::failbit;
if (lval < numeric_limits<short>::min())
{
  err |= ios_base::failbit;
  val = numeric_limits<short>::min();
}
else if (lval > numeric_limits<short>::max())
{
  err |= ios_base::failbit;
  val = numeric_limits<short>::max();
}
else
  val = static_cast<short>(lval);
setstate(err);

operator>>(int& val);

-3- The conversion occurs as if performed by the following code fragment (using the same notation as for the preceding code fragment): 

typedef num_get<charT,istreambuf_iterator<charT,traits> > numget;
iostate err = iostate_base::goodbit;
long lval;
use_facet<numget>(loc).get(*this, 0, *this, err, lval);
if (err != 0)
  ;
else if (lval < numeric_limits<int>::min()
  || numeric_limits<int>::max() < lval)
     err = ios_base::failbit;
if (lval < numeric_limits<int>::min())
{
  err |= ios_base::failbit;
  val = numeric_limits<int>::min();
}
else if (lval > numeric_limits<int>::max())
{
  err |= ios_base::failbit;
  val = numeric_limits<int>::max();
}
else
  val = static_cast<int>(lval);
setstate(err);