QTimer: don't circumvent <chrono> safety net

By templating on the <chrono> types and unconditionally using
duration_cast to coerce the duration into a milliseconds, we
violate a principal design rule of <chrono>, namely that non-
narrowing conversions are implicit, but narrowing conversions
need duration_cast. By accepting any duration, we allow non-
sensical code such as

   QTimer::singleShot(10us, ...)

to compile, which is misleading, since it's actually a zero-
timeout timer.

Overloading a non-template with a template also has adverse
effects: it breaks qOverload().

Fix by replacing the function templates with functions that
just take std::chrono::milliseconds. This way, benign code
such as

    QTimer::singleShot(10s, ...)
    QTimer::singleShot(10min, ...)
    QTimer::singleShot(1h, ...)

work as expected, but attempts to use sub-millisecond
resolution fails to compile / needs an explicit user-
provided duration_cast.

To allow future extension to more precise timers, forcibly
inline the functions, so they don't partake in the ABI of the
class and we can later support sub-millisecond resolution by
simply taking micro- or nano- instead of milliseconds.

Change-Id: I12c9a98bdabefcd8ec18a9eb09f87ad908d889de
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>
bb10
Marc Mutz 2016-10-17 13:00:04 +02:00
parent 3a1245fdac
commit e80faf3db6
3 changed files with 37 additions and 27 deletions

View File

@ -533,7 +533,7 @@ void QTimer::singleShot(int msec, Qt::TimerType timerType, const QObject *receiv
*/
/*!
\fn void QTimer::singleShot(std::chrono::duration<Rep, Period> value, const QObject *receiver, const char *member)
\fn void QTimer::singleShot(std::chrono::milliseconds msec, const QObject *receiver, const char *member)
\since 5.8
\overload
\reentrant
@ -545,13 +545,13 @@ void QTimer::singleShot(int msec, Qt::TimerType timerType, const QObject *receiv
create a local QTimer object.
The \a receiver is the receiving object and the \a member is the slot. The
time interval is given in the duration object \a value.
time interval is given in the duration object \a msec.
\sa start()
*/
/*!
\fn void QTimer::singleShot(std::chrono::duration<Rep, Period> value, Qt::TimerType timerType, const QObject *receiver, const char *member)
\fn void QTimer::singleShot(std::chrono::milliseconds msec, Qt::TimerType timerType, const QObject *receiver, const char *member)
\since 5.8
\overload
\reentrant
@ -563,18 +563,18 @@ void QTimer::singleShot(int msec, Qt::TimerType timerType, const QObject *receiv
create a local QTimer object.
The \a receiver is the receiving object and the \a member is the slot. The
time interval is given in the duration object \a value. The \a timerType affects the
time interval is given in the duration object \a msec. The \a timerType affects the
accuracy of the timer.
\sa start()
*/
/*!
\fn void QTimer::start(std::chrono::duration<Rep, Period> value)
\fn void QTimer::start(std::chrono::milliseconds msec)
\since 5.8
\overload
Starts or restarts the timer with a timeout of duration \a value.
Starts or restarts the timer with a timeout of duration \a msec milliseconds.
If the timer is already running, it will be
\l{QTimer::stop()}{stopped} and restarted.

View File

@ -165,38 +165,40 @@ Q_SIGNALS:
public:
#if QT_HAS_INCLUDE(<chrono>) || defined(Q_QDOC)
template <class Rep, class Period>
void setInterval(std::chrono::duration<Rep, Period> value)
Q_ALWAYS_INLINE
void setInterval(std::chrono::milliseconds value)
{
setInterval(std::chrono::duration_cast<std::chrono::milliseconds>(value).count());
setInterval(value.count());
}
Q_ALWAYS_INLINE
std::chrono::milliseconds intervalAsDuration() const
{
return std::chrono::milliseconds(interval());
}
Q_ALWAYS_INLINE
std::chrono::milliseconds remainingTimeAsDuration() const
{
return std::chrono::milliseconds(remainingTime());
}
template <class Rep, class Period>
static void singleShot(std::chrono::duration<Rep, Period> value, const QObject *receiver, const char *member)
Q_ALWAYS_INLINE
static void singleShot(std::chrono::milliseconds value, const QObject *receiver, const char *member)
{
singleShot(int(std::chrono::duration_cast<std::chrono::milliseconds>(value).count()), receiver, member);
singleShot(int(value.count()), receiver, member);
}
template <class Rep, class Period>
static void singleShot(std::chrono::duration<Rep, Period> value, Qt::TimerType timerType, const QObject *receiver, const char *member)
Q_ALWAYS_INLINE
static void singleShot(std::chrono::milliseconds value, Qt::TimerType timerType, const QObject *receiver, const char *member)
{
singleShot(int(std::chrono::duration_cast<std::chrono::milliseconds>(value).count()), timerType, receiver, member);
singleShot(int(value.count()), timerType, receiver, member);
}
template <class Rep, class Period>
void start(std::chrono::duration<Rep, Period> value)
Q_ALWAYS_INLINE
void start(std::chrono::milliseconds value)
{
start(int(std::chrono::duration_cast<std::chrono::milliseconds>(value).count()));
start(int(value.count()));
}
#endif
@ -215,15 +217,13 @@ private:
const QObject *receiver, QtPrivate::QSlotObjectBase *slotObj);
#if QT_HAS_INCLUDE(<chrono>)
template <class Rep, class Period>
static Qt::TimerType defaultTypeFor(std::chrono::duration<Rep, Period> interval)
{ return defaultTypeFor(int(std::chrono::duration_cast<std::chrono::milliseconds>(interval).count())); }
static Qt::TimerType defaultTypeFor(std::chrono::milliseconds interval)
{ return defaultTypeFor(int(interval.count())); }
template <class Rep, class Period>
static void singleShotImpl(std::chrono::duration<Rep, Period> interval, Qt::TimerType timerType,
static void singleShotImpl(std::chrono::milliseconds interval, Qt::TimerType timerType,
const QObject *receiver, QtPrivate::QSlotObjectBase *slotObj)
{
singleShotImpl(int(std::chrono::duration_cast<std::chrono::milliseconds>(interval).count()),
singleShotImpl(int(interval.count()),
timerType, receiver, slotObj);
}
#endif

View File

@ -216,6 +216,16 @@ void tst_QTimer::remainingTimeDuringActivation()
}
}
namespace {
#if QT_HAS_INCLUDE(<chrono>)
template <typename T>
std::chrono::milliseconds to_ms(T t)
{ return std::chrono::duration_cast<std::chrono::milliseconds>(t); }
#endif
} // unnamed namespace
void tst_QTimer::basic_chrono()
{
#if !QT_HAS_INCLUDE(<chrono>)
@ -225,7 +235,7 @@ void tst_QTimer::basic_chrono()
using namespace std::chrono;
TimerHelper helper;
QTimer timer;
timer.setInterval(nanoseconds(0));
timer.setInterval(to_ms(nanoseconds(0)));
timer.start();
QCOMPARE(timer.intervalAsDuration().count(), milliseconds::rep(0));
QCOMPARE(timer.remainingTimeAsDuration().count(), milliseconds::rep(0));
@ -248,7 +258,7 @@ void tst_QTimer::basic_chrono()
QVERIFY(helper.count > oldCount);
helper.count = 0;
timer.start(microseconds(200000));
timer.start(to_ms(microseconds(200000)));
QCOMPARE(timer.intervalAsDuration().count(), milliseconds::rep(200));
QTest::qWait(50);
QCOMPARE(helper.count, 0);
@ -864,7 +874,7 @@ void tst_QTimer::singleShot_chrono()
QCOMPARE(nhelper.count, 1);
int count = 0;
QTimer::singleShot(microseconds(0), CountedStruct(&count));
QTimer::singleShot(to_ms(microseconds(0)), CountedStruct(&count));
QCoreApplication::processEvents();
QCOMPARE(count, 1);