cplusplus / papers

ISO/IEC JTC1 SC22 WG21 paper scheduling and management
636 stars 18 forks source link

LWG3504 condition_variable::wait_for is overspecified #2033

Open jwakely opened 3 weeks ago

jwakely commented 3 weeks ago

https://cplusplus.github.io/LWG/issue3504

During LWG reflector prioritization of LWG3504 there was a suggestion to deprecate the use of timeouts with non-integral rep types. The problem described in the issue is specific to timeouts using types like duration<float>, but there doesn't seem to be a good reason to allow that. We could require users to use 50ms instead of duration<float>(0.05), or to explicitly convert their own floating-point-based timeouts to integer-based timeours before calling a waiting function from the standard library.

Could SG1 please consider this deprecation and let LWG know whether they prefer the resolution shown in the issue (which is what libstdc++ does today), or to split those APIs something like this:

template<class Rep, class Period>
  requires integral<Rep>
  cv_status wait_for(unique_lock<mutex>& lock,
                     const chrono::duration<Rep, Period>& rel_time);
template<class Rep, class Period>
  [[deprecated("y u float?"]]
  cv_status wait_for(unique_lock<mutex>& lock,
                     const chrono::duration<Rep, Period>& rel_time);

Where the first overload requires integral<Rep> to be satisfied, and just does steady_clock::now() + rel_time as today, and the second overload is deprecated and uses the issue's proposed rel-to-abs function to convert a non-integral duration to the clock's duration.

One flaw in this suggestion is that the steady_clock's duration is not actually required to use an integral type, it would be conforming for steady_clock to use duration<float>! And in that case, being unable to pass the clock's native duration would be a bit odd. But that would be an odd implementation anyway.

Maybe the first overload could be requires integral<Rep> || same_as<Rep, chrono::steady_clock::rep> to allow that.

Or maybe we should remove the overspecified steady_clock::now() + rel_time and replace it with prose saying that rel_time is converted to an absolute time using chrono::steady_clock, leaving the details of how that's done as QoI.

Please advise.