[boost] [chrono][system] Make possible to don't provide hybrid error handling.

Subject: [boost] [chrono][system] Make possible to don't provide hybrid error handling.
From: Vicente J. Botet Escriba (vicente.botet_at_[hidden])
Date: 2011-10-08 07:08:46

Hi,

Boost.Chrono provides by default an hybrid error handling for the 
Clock::now() function, implementing the following prototype

Clock::now(system::error_code&= boost::throws());

Now that the standard has accepted the prototype

Clock::now() noexcept;

and set as NAD-Future the Beman proposal (see 
http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2011/n3314.html#935).

The hybrid prototype can not be noexcept, as the use of boost::throws() 
force to throw an exception on error.

We need to overload the function as follows:

Clock::now() noexcept;
Clock::now(system::error_code&);

This lost the advantages of the announced by the hybrid error handling 
design 
(http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2009/n2828.html), as 
we are forced to have two implementations.

In addition, while most of the uses of now(ec) expect that there is no 
exception, the prototype can not state it using noexcept, if the 
boost::throws() semantic is taken in account.

Due to this split, we can now make Boost.Chrono a real header-only 
library if we admit to renounce to use the hybrid interface.

The library could add a macro that prevent the inclusion of these 
prototype, as e.g. BOOST_CHRONO_DONT_PROVIDE_HYBRID_ERROR_HANDLING.

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 ) ) // Note that this 
call is used only when the platform provides this interface.
     {
       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.

It will be maybe better to add yet a 3rd prototype that forces the 
throw_on_error.

Clock::now() noexcept;
Clock::now(system::error_code&) noexcept;
Clock::now(throw_on_error_t&);

   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
   {
     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));
   }

While these functions are quite simple and repeating the code is not an 
issue, when the logic is more complex, this could become a maintenance 
issue that should be resolved other techniques, as using internal 
functions, policies, ...

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&);

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.

Which way do you prefer? What do you think?

Best,
Vicente