incubator-stdcxx-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Travis Vitek" <>
Subject RE: [jira] Commented: (STDCXX-900) 22.locale.time.get test fails 15 assertions
Date Tue, 15 Jul 2008 21:56:13 GMT

Martin Sebor wrote:
>Travis Vitek wrote:
>> Martin Sebor wrote:
>>> Travis Vitek (JIRA) wrote:
>>>> Well, most of these failures are destined to fail until the 
>>> test is rewritten.
>>> Are you sure you meant that the test needs to be rewritten?
>>> (I'm trying to reconcile that with your subsequent comment
>>> about binary compatibility).
>> The entire test doesn't need to be rewritten, just the 
>assertions that
>> deal with %U and %W...
>>     TEST (T (0, 0, 0, 0, 0, 320, 2, 60), "9", 1, "U", 0, Eof);
>Oh. Well, it's entirely possible that some of them are bogus.
>The ChangeLog entry for the change that added them back in
>2002 (see p4 change 138327) isn't helpful:
>   *  Exercised std::time_get<wchar_t>.
>   *  Exercised facet with user-defined LC_TIME data.
>   *  Exercised extensions implementing the full POSIX spec.
>It could be that I added the assertions even though they were
>failing, thinking I'd get the missing functionality implemented
>next, and then got distracted by something and never got back
>to it.

Yeah, it looks like this may have been the case.

>IMO, we should definitely remove bogus assertions. Unless we
>already have one, we should also open an issue to get these
>directives implemented. We can then decide exactly how and
>to what extent.

Well I don't really feel that the assertions should be removed. I think
the assertions should test things that are possible. I.e., the
assertions should be something like...

     TEST (T (0, 0, 0, 0, 0, 0, 0), "9", 1, "U", 0, Eof);
     TEST (T (0, 0, 0, 0, 0, 0, 0), "2", 1, "W", 0, Eof);

This would verify that the value is parsed, but not used (because there
isn't enough information available), or even

     TEST (T (0, 0, 0, 0, 0, 0, 0), "9", 1, "U", 0, Fail);
     TEST (T (0, 0, 0, 0, 0, 0, 0), "2", 1, "W", 0, Fail);

This would verify that the format specifier is rejected, and would need
to be fixed when the appropriate support is added. I'd also expect to
see the following assertions added (and possibly others that verify the
week is being used to help calculate the resulting date).

     // expect 2008-12-29 given "2009-W01-1"
     TEST (T (0, 0, 0, 29, 12, 28, 1, 363),
           "2009-W01-1", 10, "%Y-W%W-%d", 0, Eof); // or Fail
     // expect 2010-01-03 given "2009-W53-7"
     TEST (T (0, 0, 0,  3,  1, 30, 3,   3),
           "2009-W53-7", 10, "%Y-W%W-%d", 0, Eof); // of Fail

Of course that would mean that we'd have tests that are verifying the
feature is not implemented. In that case I guess it would be best to
just comment out the assertions and add a note explaining why. Input?

>> Even if I had a perfect implementation (i.e. I called the 
>gnu strptime
>> function internally), the first assertion could never be 
>made to pass.
>> Given a Sunday-based week number of 9, I cannot possibly 
>guess the year,
>> weekday and day of year to be 2220 AD, Tuesday and 60 
>respectively. The
>> assertion is bad. This assertion needs to be updated or removed
>> entirely. At the very least the expected date should be all 
>zeros as the
>> below testcase does.
>>     TEST (T (0, 0, 0, 0, 0, 0, 0),  "0", 1, "W", 0, Eof);
>> I modify time_get<>::do_get() to consume and ignore characters that
>> match "%U" and "%W" from the stream. This would get the assertion to
>> pass, but it wouldn't be incredibly useful.
>>>> It is impossible to reliably parse any useful date-time 
>>> information from a string that contains only the formatted 
>>> week number.
>> Just a note; the gnu strptime() succeeds if parsing only the week
>> number, but it doesn't modify the date.


>>> Even if we have a partially specified date (I believe 
>>> we need at least the year and day of week), we still need 
>>> somewhere to store the additional data so that we can store 
>>> the value we parse, and then after the parsing is done so that 
>>> we can use it to calculate something useful. At the very least 
>>> I think we'll be breaking binary compatibility.
>I remember this dilemma when implementing it. I don't recall
>how I thought it could be dealt with except for some hackery
>(borrowing some otherwise unused struct tm members for
>temporary storage in between recursive calls). I'd hate to
>see us break binary compatibility just for this.

Yeah, there aren't any 'unused' members on most platforms, so that is a

>We certainly
>can't hope to make any changes to the public API of the facet.

Yeah. I think I'll remove the offending assertions (with your approval)
and just file a new issue to get support for the %{E,O}{U,W} format
specifiers added and I'll leave it at that.


View raw message