Travis Vitek wrote:
Martin Sebor wrote:
[...]
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 agree. I just don't think that starting by deciding what the
behavior should be in the corner/unspecified cases is going to
the most efficient way to dispatch the issue. The test fails
because of bad assertions. The most efficient way to resolve
the issue is to remove the bad assertions :) The fact that
the assertions exercise unimplemented functionality is
a separate problem.
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?
Sounds good to me. Better to have good assertions commented out
than bad ones firing!
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.
[...]
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
no-go.
I meant unused by the current directive. Or simply borrow one,
use it for the duration of the function, and restore it when
we're done. But I haven't thought about this too hard so don't
put too much faith in it. It's just an unbaked idea for a dirty
hack to get around the ABI constraint.
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.
Okay.
Travis
