Re: [boost] [gsoc 2013] draft proposal for chrono::date

Subject: Re: [boost] [gsoc 2013] draft proposal for chrono::date
From: Vicente J. Botet Escriba (vicente.botet_at_[hidden])
Date: 2013-05-04 08:35:54

Le 04/05/13 08:56, Vicente J. Botet Escriba a écrit :
> Le 04/05/13 01:36, Howard Hinnant a écrit :
>> On May 3, 2013, at 7:19 PM, "Vicente J. Botet Escriba" 
>> <vicente.botet_at_[hidden]> wrote:
>>
>>> Le 04/05/13 01:05, Vicente J. Botet Escriba a écrit :
>>>> Le 03/05/13 19:53, Howard Hinnant a écrit :
>>>>> On May 3, 2013, at 1:46 PM, "Vicente J. Botet Escriba" 
>>>>> <vicente.botet_at_[hidden]> wrote:
>>>>>
>>>
>>
>> In:
>>
>> Unchecked interface:
>> --------------------
>>
>>    date(year y, month m, day d);
>>
>> If any of the y, m or d are out of range, it would index into those 
>> tables upon field->serial conversion (if asked for) and result in 
>> undefined behavior.
>>
>> In:
>>
>> Checked interface:
>> ------------------
>>
>>    date d = year(2013) / month(5) / day(3);
>>    date d = year(2013) / month(5) / 3;
>>    date d = month(5) / day(3) / year(2013);
>>    date d = month(5) / day(3) / 2013;
>>    date d = day(3) / month(5) / year(2013);
>>    date d = day(3) / month(5) / 2013;
>>
>> There would be a single point of validation at the point where all of 
>> y, m and d are known, and there would be an error. Presumably that 
>> error would be an exception if any of the inputs are not constexpr, 
>> or a compile-time failure if all of the inputs are constexpr.
>>
>>
> Just to be sure I'm not trying to solve an issue that doesn't needs to 
> be solved:
> Given
>
>   date d = day(33) / month(5) / 2013;
>
> Note that I has a typo on the day. This is equivalent on my 
> implementation to
>
> date d(year(2013), month(5), day(33));
>
> If I want to throw a bad date exception I would need to check that the 
> day/month and year are in range. Next follows the code:
>
>     days_date::days_date(year y, month m, day d)
>     {
>       if (set_if_valid_date(y, m, d))
>         return;
>       throw bad_date("day " + to_string(d) + " is out of range for "
>           + to_string(y) + '-' + to_string(m));
>     }
>
>     bool days_date::set_if_valid_date(year y, month m, day d) noexcept
>     {
>       // [0]
>       bool leap = is_leap(y.value()); // [1]
>       const day_of_year::rep* year_data = days_in_year_before(leap);
>
>       if (!(d.value() <= year_data[m.value()] - 
> year_data[m.value()-1])) // [2]
>       {
>         return false;
>       }
>       year::rep by = y.value() + 32799; // [3]
>       x_ = days_before_year(by) + year_data[m.value()-1] + d.value(); 
> // [4]
>       return true;
>     }
>
> [1] and [3] presumes the year is in range.
> [2] and [4] presumes the day and month are in range.
>
> If some of 3 values are not in range the function set_if_valid_date 
> could return true or false arbitrarily.
> So if the day/month and year are just names and don't ensure range I 
> would need to start by validating their range in [0] so that if some 
> of them is out of range the function return false;
>
> This mean that the user is not able, using the type system,  to help 
> the implementation to reduce needed check.
> I have no measures of the impact of these 3 range checks (6 comparisons).
> I just wanted to avoid them.
>
> Even if we use other kind of cache, I don't see how the cached data 
> can be accessed without checking the range are correct.
>
> E.g. An alternative implementation using a cache for the year_month_cache
>
>     bool days_date::set_if_valid_date(year y, month m, day d) noexcept
>     {
>       // [0]
>       auto c = year_month_cache(y, m); // [1]
>       if (d.value() >= c.days_in_month) return false;
>       x_ = c.days_before + d.value(); // [2]
>       return true;
>     }
>
> This needs that y and m anre in range at [1] and d in in range at [2].
>
> Howard, could you tell me if you find something wrong on my analysis?
> I want to be too purist?
>
>
Rob, all, you can also comment also :-)

Vicente