[chrono][system] Make possible to don't provide hybrid error handling.
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
On Sat, Oct 8, 2011 at 7:08 AM, Vicente J. Botet Escriba <vicente.botet@wanadoo.fr> wrote: ... Discussion of alternatives ...
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&); ... Which way do you prefer? What do you think?
D - Standard with additional behavior: * Return a special time_point value when an error occurs. Do this by having time_point provide "static constexpr time_point error();" in addition to min() and max(). Add something like this to the docs: "Since most programs will run in only in environments where clocks never fail, most programmers can safely ignore the possibility of this value being returned." (You might consider on Windows doing a fall-back if the Win32 high resolution timer fails, by the way.) * At compile time, detect platforms (such as tiny embedded systems) that never support the Clock, and issue an #error. Perhaps folks on the list could help identifying such systems. They are probably rare. --Beman
Beman Dawes wrote:
On Sat, Oct 8, 2011 at 7:08 AM, Vicente J. Botet Escriba <vicente.botet@> wrote:
... Discussion of alternatives ...
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&); ... Which way do you prefer? What do you think?
D - Standard with additional behavior:
* Return a special time_point value when an error occurs. Do this by having time_point provide "static constexpr time_point error();" in addition to min() and max(). Add something like this to the docs: "Since most programs will run in only in environments where clocks never fail, most programmers can safely ignore the possibility of this value being returned."
I don't like the addition of not_a_time_point value as the result of the standard prototype, as it forces the user to check any call to now(). If we think that in some platforms clock::now could result in an error that the user can do something about, we should use either an error code or an exception, but not an invalid value. If the user couldn't do anything, as this is an internal error, the standard behavior is enough. Well this is my opinion ;-)
(You might consider on Windows doing a fall-back if the Win32 high resolution timer fails, by the way.)
What do you mean by doing a fallback?
* At compile time, detect platforms (such as tiny embedded systems) that never support the Clock, and issue an #error. Perhaps folks on the list could help identifying such systems. They are probably rare.
The library provides already macros to check for the availability of all the clocks expect system_clock, which is mandatory. I will check if I have generated a #error for platforms other than win, posix or mac. Best, Vicente -- View this message in context: http://boost.2283326.n4.nabble.com/chrono-system-Make-possible-to-don-t-prov... Sent from the Boost - Dev mailing list archive at Nabble.com.
Vicente Botet wrote:
Beman Dawes wrote:
* Return a special time_point value when an error occurs. Do this by having time_point provide "static constexpr time_point error();" in addition to min() and max(). Add something like this to the docs: "Since most programs will run in only in environments where clocks never fail, most programmers can safely ignore the possibility of this value being returned."
I don't like the addition of not_a_time_point value as the result of the standard prototype, as it forces the user to check any call to now(). If we think that in some platforms clock::now could result in an error that the user can do something about, we should use either an error code or an exception, but not an invalid value. If the user couldn't do anything, as this is an internal error, the standard behavior is enough.
+1 Unless there's a way to assert that a given platform never returns the invalid value, so programmers using that platform can write code that ignores the possibility, then safe programming practice requires adding the checks. _____ Rob Stewart robert.stewart@sig.com Software Engineer using std::disclaimer; Dev Tools & Components Susquehanna International Group, LLP http://www.sig.com ________________________________ IMPORTANT: The information contained in this email and/or its attachments is confidential. If you are not the intended recipient, please notify the sender immediately by reply and immediately delete this message and all its attachments. Any review, use, reproduction, disclosure or dissemination of this message or any attachment by an unintended recipient is strictly prohibited. Neither this message nor any attachment is intended as or should be construed as an offer, solicitation or recommendation to buy or sell any security or other financial instrument. Neither the sender, his or her employer nor any of their respective affiliates makes any warranties as to the completeness or accuracy of any of the information contained herein or that this message or any of its attachments is free of viruses.
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.
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?
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.
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?
{ 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.
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. _____ Rob Stewart robert.stewart@sig.com Software Engineer using std::disclaimer; Dev Tools & Components Susquehanna International Group, LLP http://www.sig.com ________________________________ IMPORTANT: The information contained in this email and/or its attachments is confidential. If you are not the intended recipient, please notify the sender immediately by reply and immediately delete this message and all its attachments. Any review, use, reproduction, disclosure or dissemination of this message or any attachment by an unintended recipient is strictly prohibited. Neither this message nor any attachment is intended as or should be construed as an offer, solicitation or recommendation to buy or sell any security or other financial instrument. Neither the sender, his or her employer nor any of their respective affiliates makes any warranties as to the completeness or accuracy of any of the information contained herein or that this message or any of its attachments is free of viruses.
On Tue, Oct 11, 2011 at 1:48 PM, Stewart, Robert <Robert.Stewart@sig.com> wrote:
Clock::now() noexcept;
20.9.5 doesn't specify noexcept for now(). Should you add that?
Actually it does. It was added at the last meeting, so is only reflected in the final standard and not in the drafts from prior meetings. Also, stuff got moved around so the reference is now 20.11.7, [time.clock] Most code never runs on platforms that don't have functioning clocks, so can ignore the whole problem. Since these platforms are always believed to be freestanding implementations (17.6.1.3 Freestanding implementations), they may not even have a <chrono> header in the standard library implementation. --Beman
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
participants (4)
-
Beman Dawes -
Stewart, Robert -
Vicente Botet -
Vicente J. Botet Escriba