<iomanip>: get_time handles leading zeros incorrectly

[StephanTLavavej]

Reported by @randydu in #944 (comment) (thanks @statementreply for looking into this).

While reviewing #1280, we noticed and fixed an issue with leading zeros, but the fix was incomplete. Here's a slightly enhanced version of @randydu's test case:

C:\Temp>type meow.cpp

#include <cassert>
#include <cstdio>
#include <ctime>
#include <iomanip>
#include <sstream>
using namespace std;

int main() {
  istringstream iss("19700405T000006");
  tm t{};
  iss >> get_time(&t, "%Y%m%dT%H%M%S");
  assert(iss);

  printf("Expected hour 0, min 0, sec 6\n");
  printf("     Got hour %d, min %d, sec %d\n", t.tm_hour, t.tm_min, t.tm_sec);

  assert(t.tm_year == 70);
  assert(t.tm_mon == 3);
  assert(t.tm_mday == 5);
  assert(t.tm_hour == 0);
  assert(t.tm_min == 0);
  assert(t.tm_sec == 6);
}

C:\Temp>cl /EHsc /nologo /W4 /MTd meow.cpp
meow.cpp

C:\Temp>meow
Expected hour 0, min 0, sec 6
     Got hour 0, min 6, sec 0
Assertion failed: t.tm_min == 0, file meow.cpp, line 21

We start by calculating the maximum number of digits we can consume (currently always within [1, 4]):

STL/stl/inc/xloctime

Line 543 in 6779b61

const int _Hi_digits = (_Hi <= 9 ? 1 : _Hi <= 99 ? 2 : _Hi <= 999 ? 3 : 4);

But we'll consume an unlimited number of leading zeros, updating _Digits_seen accordingly:

STL/stl/inc/xloctime

Lines 561 to 563 in 6779b61

	for (; _First != _Last && _Ctype_fac.narrow(*_First) == '0'; ++_First) { // strip leading zeros
	++_Digits_seen;
	}

When we reach the point of consuming digits after the leading zeros, we check _Digits_seen < _Hi_digits:

STL/stl/inc/xloctime

Lines 569 to 571 in 6779b61

	for (char* const _Pe = &_Ac[_MAX_INT_DIG - 1];
	_First != _Last && '0' <= (_Ch = _Ctype_fac.narrow(*_First)) && _Ch <= '9' && _Digits_seen < _Hi_digits;
	++_Digits_seen, (void) ++_First) { // copy digits

So in this test case, the hour field %H consumes 00000 (after the T), then notices it's consumed way more than the limit of 2 digits for an hour. It succeeds, and t.tm_hour == 0, but we should have consumed only 00 to get that result.

Then, the minute field %M consumes the 6 at the end of the input.

We end up returning with only eofbit set, and never parse the %S. (Separately, I am surprised that we don't failbit when we were unable to parse all fields, but I am not an iostreams expert - this part may be intentional.)

STL/stl/inc/xloctime

Lines 134 to 139 in 6779b61

	_First = do_get(_First, _Last, _Iosbase, _State, _Pt, _Specifier, _Modifier); // convert a single field
	if (_State != ios_base::goodbit) {
	// _State is fail, eof, or bad. Do not proceed to the next fields. Return with current _State.
	break;
	}

I believe that the leading zeros loop needs to be limited with _Digits_seen < _Hi_digits. I'm unsure whether other code changes will be necessary.

[cpplearner]

It seems that libstdc++ and libc++ also do not recognize leading + or -.

[StephanTLavavej]

@cpplearner If that's a pre-existing bug where you can cite Standardese showing that all of these implementations are incorrect, please report it as a separate issue. I would like to use this issue to track the leading zeros bug by itself, since it appears to have a distinct cause and fix.

[cpplearner]

I think this is MSVC STL's bug. POSIX only says that a leading '+' or '-' character is permitted for 'C', 'y' and 'Y'.

I haven't checked ISO/IEC 9945:2003, but it might be the case that the 2003 version did not permit '+' and '-' at all.

[MattStephanson]

I haven't checked ISO/IEC 9945:2003, but it might be the case that the 2003 version did not permit '+' and '-' at all.

I believe this is correct, based on https://pubs.opengroup.org/onlinepubs/009695399/functions/strptime.html.

I think this is MSVC STL's bug. POSIX only says that a leading '+' or '-' character is permitted for 'C', 'y' and 'Y'.

The difficulty here is that neither the MSVC ABI nor [locale.time.get]/8.4 have any way to pass a user-specified width, which is also new in the 2017 version. So, it's impossible to fully implement the newer strptime specification anyway. FWIW, The current MSVC implementation will parse +/- but not count it toward the number of characters read.