tests: fix some -Wvolatile

C++20 deprecated compound volatile statements such as pre- and
post-increments, to stress that they're not atomic. So instead of

    volatile i;
    ~~~~;
    ++i;

you're now supposed to write

    volatile i;
    ~~~~;
    int j = i; // volatile load
    ++j;
    i = j; // volatile store

which matches more closely what hardware does.

Instead of fixing every use of volatile pre- or post-increment in this
fashion individually, and realising that probably a few more Qt
modules will have the same kind of code patterns in them, write
QtPrivate functions to do the job centrally.

Change-Id: I838097bd484ef2118c071726963f103c080d2ba5
Reviewed-by: Lars Knoll <lars.knoll@qt.io>
This commit is contained in:
Marc Mutz 2021-07-16 09:57:56 +02:00
parent 74a91773af
commit 3a72496b5c
10 changed files with 109 additions and 20 deletions

View File

@ -60,6 +60,7 @@ qt_internal_add_module(Core
global/qsysinfo.h
global/qsystemdetection.h
global/qtypeinfo.h
global/qvolatile_p.h
io/qabstractfileengine.cpp io/qabstractfileengine_p.h
io/qbuffer.cpp io/qbuffer.h
io/qdataurl.cpp io/qdataurl_p.h

View File

@ -0,0 +1,89 @@
/****************************************************************************
**
** Copyright (C) 2021 Klarälvdalens Datakonsult AB, a KDAB Group company, info@kdab.com, author Marc Mutz <marc.mutz@kdab.com>
** Contact: http://www.qt.io/licensing/
**
** This file is part of the QtCore module of the Qt Toolkit.
**
** $QT_BEGIN_LICENSE:LGPL$
** Commercial License Usage
** Licensees holding valid commercial Qt licenses may use this file in
** accordance with the commercial license agreement provided with the
** Software or, alternatively, in accordance with the terms contained in
** a written agreement between you and The Qt Company. For licensing terms
** and conditions see https://www.qt.io/terms-conditions. For further
** information use the contact form at https://www.qt.io/contact-us.
**
** GNU Lesser General Public License Usage
** Alternatively, this file may be used under the terms of the GNU Lesser
** General Public License version 3 as published by the Free Software
** Foundation and appearing in the file LICENSE.LGPL3 included in the
** packaging of this file. Please review the following information to
** ensure the GNU Lesser General Public License version 3 requirements
** will be met: https://www.gnu.org/licenses/lgpl-3.0.html.
**
** GNU General Public License Usage
** Alternatively, this file may be used under the terms of the GNU
** General Public License version 2.0 or (at your option) the GNU General
** Public license version 3 or any later version approved by the KDE Free
** Qt Foundation. The licenses are as published by the Free Software
** Foundation and appearing in the file LICENSE.GPL2 and LICENSE.GPL3
** included in the packaging of this file. Please review the following
** information to ensure the GNU General Public License requirements will
** be met: https://www.gnu.org/licenses/gpl-2.0.html and
** https://www.gnu.org/licenses/gpl-3.0.html.
**
** $QT_END_LICENSE$
**
****************************************************************************/
#ifndef QVOLATILE_P_H
#define QVOLATILE_P_H
//
// W A R N I N G
// -------------
//
// This file is not part of the Qt API. It exists for the convenience
// of a number of Qt sources files. This header file may change from
// version to version without notice, or even be removed.
//
// We mean it.
//
#include <QtCore/qglobal.h>
QT_BEGIN_NAMESPACE
namespace QtPrivate {
template <typename T>
using if_volatile = std::enable_if_t<std::is_volatile_v<T>, bool>;
//
// C++20-deprecated volatile compound operations, rewritten as separated operations
//
// these functions return `auto`, not `T`, to strip cv-qualifiers without having
// to mention the `volatile` keyword
template <typename T, QtPrivate::if_volatile<T> = true>
auto volatilePreIncrement(T &x) {
auto y = x;
++y;
x = y;
return y;
}
template <typename T, QtPrivate::if_volatile<T> = true>
auto volatilePreDecrement(T &x)
{
auto y = x;
--y;
x = y;
return y;
}
} // namespace QtPrivate
QT_END_NAMESPACE
#endif // QVOLATILE_H

View File

@ -7,12 +7,6 @@
qt_internal_add_test(tst_qmutex
SOURCES
tst_qmutex.cpp
)
## Scopes:
#####################################################################
qt_internal_extend_target(tst_qmutex CONDITION WIN32
PUBLIC_LIBRARIES
Qt::CorePrivate
)

View File

@ -36,6 +36,7 @@
#include <qmutex.h>
#include <qthread.h>
#include <qwaitcondition.h>
#include <private/qvolatile_p.h>
class tst_QMutex : public QObject
{
@ -1191,9 +1192,9 @@ void tst_QMutex::tryLockDeadlock()
{
for (int i = 0; i < 100000; ++i) {
if (mut.tryLock(0)) {
if ((++tryLockDeadlockCounter) != 1)
if (QtPrivate::volatilePreIncrement(tryLockDeadlockCounter) != 1)
++tryLockDeadlockFailureCount;
if ((--tryLockDeadlockCounter) != 0)
if (QtPrivate::volatilePreDecrement(tryLockDeadlockCounter) != 0)
++tryLockDeadlockFailureCount;
mut.unlock();
}
@ -1210,9 +1211,9 @@ void tst_QMutex::tryLockDeadlock()
for (int i = 0; i < 100000; ++i) {
mut.lock();
if ((++tryLockDeadlockCounter) != 1)
if (QtPrivate::volatilePreIncrement(tryLockDeadlockCounter) != 1)
++tryLockDeadlockFailureCount;
if ((--tryLockDeadlockCounter) != 0)
if (QtPrivate::volatilePreDecrement(tryLockDeadlockCounter) != 0)
++tryLockDeadlockFailureCount;
mut.unlock();
}

View File

@ -7,4 +7,6 @@
qt_internal_add_test(tst_qreadwritelock
SOURCES
tst_qreadwritelock.cpp
PUBLIC_LIBRARIES
Qt::CorePrivate
)

View File

@ -34,6 +34,7 @@
#include <qmutex.h>
#include <qthread.h>
#include <qwaitcondition.h>
#include <private/qvolatile_p.h>
#ifdef Q_OS_UNIX
#include <unistd.h>
@ -587,12 +588,8 @@ public:
if(count)
qFatal("Non-zero count at start of write! (%d)",count );
// printf(".");
int i;
for(i=0; i<maxval; ++i) {
volatile int lc=count;
++lc;
count=lc;
}
for (int i = 0; i < maxval; ++i)
QtPrivate::volatilePreIncrement(count);
count=0;
testRwlock.unlock();
msleep(ulong(waitTime));

View File

@ -8,4 +8,6 @@ qt_internal_add_test(tst_qsharedpointer
nontracked.cpp
wrapper.cpp
tst_qsharedpointer.cpp
PUBLIC_LIBRARIES
Qt::CorePrivate
)

View File

@ -37,6 +37,7 @@
#include <QtCore/QList>
#include <QtCore/QMap>
#include <QtCore/QThread>
#include <QtCore/private/qvolatile_p.h>
#include "forwarddeclared.h"
#include "nontracked.h"
@ -1944,7 +1945,7 @@ class ThreadData
QAtomicInt * volatile ptr;
public:
ThreadData(QAtomicInt *p) : ptr(p) { }
~ThreadData() { ++ptr; }
~ThreadData() { QtPrivate::volatilePreIncrement(ptr); }
void ref()
{
// if we're called after the destructor, we'll crash

View File

@ -8,6 +8,7 @@ qt_internal_add_benchmark(tst_bench_qmutex
SOURCES
tst_qmutex.cpp
PUBLIC_LIBRARIES
Qt::CorePrivate
Qt::Test
)

View File

@ -28,6 +28,7 @@
#include <QtCore/QtCore>
#include <QTest>
#include <QtCore/private/qvolatile_p.h>
#include <math.h>
@ -158,7 +159,7 @@ void tst_QMutex::noThread()
QBENCHMARK {
count = 0;
for (int i = 0; i < N; i++) {
count++;
QtPrivate::volatilePreIncrement(count);
}
}
break;
@ -167,7 +168,7 @@ void tst_QMutex::noThread()
count = 0;
for (int i = 0; i < N; i++) {
mtx.lock();
count++;
QtPrivate::volatilePreIncrement(count);
mtx.unlock();
}
}
@ -177,7 +178,7 @@ void tst_QMutex::noThread()
count = 0;
for (int i = 0; i < N; i++) {
QMutexLocker locker(&mtx);
count++;
QtPrivate::volatilePreIncrement(count);
}
}
break;