Subject: Re: [boost] [chrono][system] Make possible to don't provide hybrid error handling.
From: Vicente J. Botet Escriba (vicente.botet_at_[hidden])
Date: 2011-10-11 16:28:25
Le 11/10/11 19:48, Stewart, Robert a écrit :
> Vicente J. Botet Escriba wrote:
>> The library could add a macro that prevent the inclusion of
>> these prototype, as e.g.
>> BOOST_CHRONO_DONT_PROVIDE_HYBRID_ERROR_HANDLING.
> If you had to go that way, s/DONT_PROVIDE/NO/!
>
>> The experience while using these hybrid approach and mixing
>> all these semantics has shown me that this doesn't promotes
>> good design and doesn't compose well. Split the prototypes
>> is cleared.
>>
>> system_clock::time_point system_clock::now() noexcept
>> {
>> timespec ts;
>> if ( ::clock_gettime( CLOCK_REALTIME,&ts ) )
>> {
>> BOOST_ASSERT(0&& "Boost::Chrono - Internal Error");
>> }
>> return time_point(duration(
>> static_cast<system_clock::rep>( ts.tv_sec ) *
>> 1000000000 + ts.tv_nsec));
>> }
>>
>> system_clock::time_point
>> system_clock::now(system::error_code& ec)
>> {
>> timespec ts;
>> if ( ::clock_gettime( CLOCK_REALTIME,&ts ) )
>> {
>> if (BOOST_CHRONO_IS_THROWS(ec)) //[1]
>> {
>> boost::throw_exception(
>> system::system_error(
>> errno,
>> BOOST_CHRONO_SYSTEM_CATEGORY,
>> "chrono::system_clock" ));
>> }
>> else
>> {
>> ec.assign( errno, BOOST_CHRONO_SYSTEM_CATEGORY );
>> return time_point();
>> }
>> }
>>
>> if (!BOOST_CHRONO_IS_THROWS(ec)) // [2]
>> {
>> ec.clear();
>> }
>> return time_point(duration(
>> static_cast<system_clock::rep>( ts.tv_sec ) *
>> 1000000000 + ts.tv_nsec));
>> }
>>
>>
>> Note how the test on whether the ec is thows() in [1] to throw
>> and in [2] see if the ec can be cleared are obscuring what in
>> principle was simple.
> [2] isn't needed. In the case of no error, you clear ec. In the case of an error, you have extra work to do and, frankly, don't care about efficiency. Therefore:
>
> system_clock::time_point
> system_clock::now(system::error_code& _code)
> {
> _code.clear();
> timespec ts;
> if (::clock_gettime(CLOCK_REALTIME,&ts))
> {
> if (BOOST_CHRONO_IS_THROWS(_code))
> {
> boost::throw_exception(
> system::system_error(
> errno, BOOST_CHRONO_SYSTEM_CATEGORY,
> "chrono::system_clock" ));
> }
> else
> {
> _code.assign(errno, BOOST_CHRONO_SYSTEM_CATEGORY);
> return time_point();
> }
> }
> return time_point(
> duration(static_cast<system_clock::rep>(ts.tv_sec)
> * 1000000000 + ts.tv_nsec));
> }
>
> There's still logic to check for the throws() case, but the result is a bit cleaner.
I don't remenber if boost:throws() returned or returns a reference to 0,
so I nneded to check before writing. Beman?
>
>> It will be maybe better to add yet a 3rd prototype that forces
>> the throw_on_error.
>>
>> Clock::now() noexcept;
> 20.9.5 doesn't specify noexcept for now(). Should you add that?
See Beman's response.
>
>> Clock::now(system::error_code&) noexcept;
>> Clock::now(throw_on_error_t&);
> If you don't make the no-arguments overload be noexcept, then you get this:
>
> Clock::now();
> Clock::now(system::error_code&) noexcept;
>
> The first can throw; the second doesn't. That, of course, could eliminate the throws() usage.
I will prefer to provide at least the standard inteface. So
Clock::now() noexcept;
is mandatory for me.
>
>> system_clock::time_point system_clock::now()
>> BOOST_CHRONO_NOEXCEPT
>> {
>> timespec ts;
>> if ( ::clock_gettime( CLOCK_REALTIME,&ts ) )
>> {
>> BOOST_ASSERT(0&& "Boost::Chrono - Internal Error");
>> }
>> return time_point(duration(
>> static_cast<system_clock::rep>( ts.tv_sec ) *
>> 1000000000 + ts.tv_nsec));
>> }
>>
>> system_clock::time_point
>> system_clock::now(system::error_code& ec)
>> {
>> timespec ts;
>> if ( ::clock_gettime( CLOCK_REALTIME,&ts ) )
>> {
>> ec.assign( errno, BOOST_CHRONO_SYSTEM_CATEGORY );
>> return time_point();
>> }
>>
>> ec.clear();
>> return time_point(duration(
>> static_cast<system_clock::rep>( ts.tv_sec ) *
>> 1000000000 + ts.tv_nsec));
>> }
>>
>> system_clock::time_point system_clock::now(
>> throw_on_error_t& /*tag*/) noexcept
> Why by reference, anyway?
This was a type. No need for reference, of course.
>> {
>> timespec ts;
>> if ( ::clock_gettime( CLOCK_REALTIME,&ts ) )
>> {
>> boost::throw_exception(
>> system::system_error(
>> errno,
>> BOOST_CHRONO_SYSTEM_CATEGORY,
>> "chrono::system_clock" ));
>> }
>> return time_point(duration(
>> static_cast<system_clock::rep>( ts.tv_sec ) *
>> 1000000000 + ts.tv_nsec));
>> }
> With my version, you'd get this instead:
>
> system_clock::time_point
> system_clock::now()
> {
> system::error_code error;
> time_point const result(now(error));
> if (error)
> {
> boost::throw_exception(
> system::system_error(error, "chrono::system_clock"));
> }
> return result;
> }
>
> system_clock::time_point
> system_clock::now(system::error_code& _code) noexcept
> {
> _code.clear();
> timespec ts;
> if (::clock_gettime(CLOCK_REALTIME,&ts))
> {
> _code.assign(errno, BOOST_CHRONO_SYSTEM_CATEGORY);
> return time_point();
> }
> return time_point(
> duration(static_cast<system_clock::rep>(ts.tv_sec)
> * 1000000000 + ts.tv_nsec));
> }
>
> That seems very clear. There's no duplication.
See my comment above.
>
>> I see three alternatives:
>>
>> A- Standard
>> Clock::now() noexcept;
>>
>> B- Standard+Hybrid
>> Clock::now() noexcept;
>> Clock::now(system::error_code&); // takes care of
>> boost::throw()
>>
>> C-Standard+EC+TOE
>> Clock::now() noexcept;
>> Clock::now(system::error_code&) noexcept;
>> Clock::now(throw_on_error_t&);
>>
>> A breaks code using the error_code.
>> C breaks code using the boost::throws()
>>
>> Yet another temporary alternative:
>> D-
>> Clock::now() noexcept;
>> Clock::now(system::error_code&); // takes care of
>> boost::throw() but it is deprecated
>> Clock::now(throw_on_error_t&);
> Now E-
>
> Clock::now();
> Clock::now(system::error_code&);
>
>> We can start by providing D and let the user using
>> boost::throws to move to throw_on_error, and the move to C.
>> In addition the library must provide the user the possibility
>> to don't include the extensions.
> With E, there are fewer overloads and the semantics are easy to explain. The downside is current uses of boost::throws() will break. Nevertheless, that avoids any deprecation hassles as the change forces updating calling code to match the change in semantics.
>
>
The idea of Beman was to reduce to a single interface
Clock::now(system::error_code&=boost::throws());
But it was not accepted and with noexcept, you can not have all in a single prototype.
What others think about whether Boost.Chono must provide at least the
standard interface?
Best,
Vicente