Closed tomaszkam closed 5 years ago
Tim's comment:
On one hand, we say that if you try to construct a weekday_indexed with index 0, you get an unspecified index instead ( [time.cal.wdidx.members]p1),:
The values held are unspecified if !wd.ok() or index is not in the range [1, 5].
OTOH, we take pains to pin down yearmonthweekday's conversion to sys_days when the index is zero ([time.cal.ymwd.members]p19):
If index() is 0 the returned sys_days represents the date 7 days prior to the first weekday() of year()/month().
This seems a bit inconsistent. Perhaps we should allow a slightly wider range (say, [0, 7], since you need at least 3 bits anyway to represent the 5 distinct valid values)?
I believe we have to do this at least for the value 0 to make it consistent with http://eel.is/c++draft/time.cal.ymwd.members#19 which specifies what happens when index()
is 0. And while we're at it, might as well make the specified range [0, 7].
Proposed wording:
27.8.7.2 Member functions [time.cal.wdidx.members], p1:
Effects: Constructs an object of type weekdayindexed by initializing wd with wd and index_ with index. The values held are unspecified if !wd.ok() or index is not in the range [1, 5][0, 7].
Why not [0, 15]? That would be more consistent with day
that has value range driven by required storage (8bits), and we could consume 4bits here.
While my implementation does allow [0, 15], I couldn't come up with a good rationale for mandating that support. This boils down to these two lines of code:
https://github.com/HowardHinnant/date/blob/master/include/date/date.h#L484-L485
As far as practical use cases are concerned, I have found none for !ok()
indexes outside of:
2019y/September/Sunday[0]
(the Sunday prior to the first Sunday in September). And even that has a less confusing representation:
2019y/August/Sunday[last]
Thus I believe this is a case to give implementors more leeway for the case that they may discover reasons in the future to take advantage of an extra bit within weekday_indexed
.
Mailed to LWG chair.
Original comment:
E-mail thread: [isocpp-lib] Range of index for weekday_indexed