stdcxx-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Martin Sebor <se...@roguewave.com>
Subject Re: [jira] Commented: (STDCXX-900) 22.locale.time.get test fails 15 assertions
Date Tue, 15 Jul 2008 20:29:53 GMT
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.

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.

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

Right.

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

I think it does in some cases modify it if can use the parsed
data to complete the contents of the struct. This is allowed
by the following sentence in POSIX strptime():

   It is unspecified whether multiple calls to strptime() using
   the same tm structure will update the current contents of
   the structure or overwrite all contents of the structure.

> On solaris, that same call seems
> to fail if it encounters the %U or %W specifier in the format string,
> even if that string contains a fully formatted date.
> 
>> The test tries to to do just this for {{%U}} and 
>> {{%W}}.
> 
> I'm not exactly sure what the expected behavior should be, given the
> above results on linux and solaris. Perhaps parsing the values and not
> using them is sufficient, or just failing outright when reading them (as
> it does now). Regardless of what we decide, 
> 
>> 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. We certainly
can't hope to make any changes to the public API of the facet.

Martin

>>> So, for the time being, I think the right thing to do is to 
>> fix the portion of the test that attempts to to verify {{%U}} 
>> and {{%W}} so that it expects failure.
> 
> As mentioned above...
> 
>> If we wish to fully 
>> support this extension, then we should add a new test that 
>> verifies {{%U}} and {{%W}} with the necessary format 
>> specifiers and then fix the code for 4.3 or later.
> 
> The test would need include something like this. As mentioned in my
> previous post, I think we would need to break binary compatibility to
> even have a chance of making these pass.
> 
>     // expect 2008-12-29 given "2009-W01-1"
>     TEST (T (0, 0, 0, 29, 12, 28, 1, 363), "2009-W01-1", 1, "%Y-W%W-%d",
> 0, Eof);
> 
>     // expect 2010-01-03 given "2009-W53-7"
>     TEST (T (0, 0, 0,  3,  1, 30, 3,   3), "2009-W53-7", 1, "%Y-W%W-%d",
> 0, Eof);
> 
> Travis


Mime
View raw message