Deprecate Q_ASSUME()

We've known for a long time that this is producing worse code with GCC
because of how we implemented in Q_ASSUME_IMPL(). So bite the bullet and
actually deprecate the macro, replacing all extant Q_ASSUME() with
Q_ASSERT().

The replacement is in C++23. Backporting the support onto Q_ASSUME_IMPL
was previously rejected by reviewers.

[ChangeLog][Deprecation Notice] The Q_ASSUME() macro is deprecated. This
macro has different side-effects depending on the compiler used (GCC
compared to Clang and MSVC), and there are certain conditions under
which GCC is known to produce worse code than if the macro was absent.
To give a hint to the compiler for optimizations, use the C++23
[[assume]] attribute.

Change-Id: I80612a7d275c41f1baf0fffd177a3a4ad819fb2d
Reviewed-by: Volker Hilsheimer <volker.hilsheimer@qt.io>
This commit is contained in:
Thiago Macieira 2023-08-10 21:38:23 -07:00
parent e8e8e1082e
commit 14d1108d35
16 changed files with 24 additions and 21 deletions

View File

@ -79,11 +79,14 @@ inline T *q_check_ptr(T *p) { Q_CHECK_PTR(p); return p; }
# endif
#endif
Q_DECL_DEPRECATED_X("Q_ASSUME() is deprecated because it can produce worse code than when it's absent; "
"use C++23 [[assume]] instead")
inline bool qt_assume_is_deprecateed(bool cond) noexcept { return cond; }
#define Q_ASSUME(Expr) \
[] (bool valueOfExpression) {\
Q_ASSERT_X(valueOfExpression, "Q_ASSUME()", "Assumption in Q_ASSUME(\"" #Expr "\") was not correct");\
Q_ASSUME_IMPL(valueOfExpression);\
}(Expr)
}(qt_assume_is_deprecated(Expr))
// Don't use these in C++ mode, use static_assert directly.
// These are here only to keep old code compiling.

View File

@ -401,7 +401,7 @@ qint64 QBuffer::writeData(const char *data, qint64 len)
if (required > quint64(d->buf->size())) { // capacity exceeded
// The following must hold, since qsizetype covers half the virtual address space:
Q_ASSUME(required <= quint64((std::numeric_limits<qsizetype>::max)()));
Q_ASSERT(required <= quint64((std::numeric_limits<qsizetype>::max)()));
d->buf->resize(qsizetype(required));
if (quint64(d->buf->size()) != required) { // could not resize
qWarning("QBuffer::writeData: Memory allocation error");

View File

@ -276,7 +276,7 @@ static void customConstruct(const QtPrivate::QMetaTypeInterface *iface, QVariant
if constexpr (moveOption == ForceMove)
Q_ASSERT(isMoveConstructible(iface));
if constexpr (nullability == NonNull)
Q_ASSUME(copy != nullptr);
Q_ASSERT(copy != nullptr);
// need to check for nullptr_t here, as this can get called by fromValue(nullptr). fromValue() uses
// std::addressof(value) which in this case returns the address of the nullptr object.

View File

@ -1011,7 +1011,7 @@ QJsonValue QJsonValueConstRef::concrete(QJsonValueConstRef self) noexcept
QString QJsonValueConstRef::objectKey(QJsonValueConstRef self)
{
Q_ASSERT(self.is_object);
Q_ASSUME(self.is_object);
Q_ASSERT(self.is_object);
const QCborContainerPrivate *d = QJsonPrivate::Value::container(self);
qsizetype index = QJsonPrivate::Value::indexHelper(self);

View File

@ -276,7 +276,7 @@ int qstricmp(const char *str1, const char *str2)
// yes, find out where
uint start = qCountTrailingZeroBits(mask);
uint end = sizeof(mask) * 8 - qCountLeadingZeroBits(mask);
Q_ASSUME(end >= start);
Q_ASSERT(end >= start);
offset += start;
n = end - start;
break;

View File

@ -729,7 +729,7 @@ static T dtoString(double d, QLocaleData::DoubleForm form, int precision, bool u
result.append(Char(uppercase ? 'E' : 'e'));
result.append(Char(exponent < 0 ? '-' : '+'));
exponent = std::abs(exponent);
Q_ASSUME(exponent <= D::max_exponent10 + D::max_digits10);
Q_ASSERT(exponent <= D::max_exponent10 + D::max_digits10);
int exponentDigits = digits(exponent);
// C's printf guarantees a two-digit exponent, and so do we:
if (exponentDigits == 1)

View File

@ -1104,7 +1104,7 @@ void QRegularExpressionPrivate::doMatch(QRegularExpressionMatchPrivate *priv,
const QRegularExpressionMatchPrivate *previous) const
{
Q_ASSERT(priv);
Q_ASSUME(priv != previous);
Q_ASSERT(priv != previous);
const qsizetype subjectLength = priv->subject.size();

View File

@ -9755,7 +9755,7 @@ qsizetype QtPrivate::findString(QLatin1StringView haystack, qsizetype from, QLat
if (cs == Qt::CaseSensitive) {
if (needle.size() == 1) {
Q_ASSUME(haystack.data() != nullptr); // see size check above
Q_ASSERT(haystack.data() != nullptr); // see size check above
if (auto it = memchr(haystack.data() + from, needle.front().toLatin1(), adjustedSize))
return static_cast<const char *>(it) - haystack.data();
return -1;

View File

@ -203,7 +203,7 @@ static inline const uchar *simdFindNonAscii(const uchar *src, const uchar *end,
continue;
uint n = _mm256_movemask_epi8(data);
Q_ASSUME(n);
Q_ASSERT(n);
// find the next probable ASCII character
// we don't want to load 32 bytes again in this loop if we know there are non-ASCII

View File

@ -1289,9 +1289,9 @@ using HashBlock = QSmallByteArray<maxHashBlockSize()>;
static HashBlock xored(const HashBlock &block, quint8 val) noexcept
{
// some hints for the optimizer:
Q_ASSUME(block.size() >= minHashBlockSize());
Q_ASSUME(block.size() <= maxHashBlockSize());
Q_ASSUME(block.size() % gcdHashBlockSize() == 0);
Q_ASSERT(block.size() >= minHashBlockSize());
Q_ASSERT(block.size() <= maxHashBlockSize());
Q_ASSERT(block.size() % gcdHashBlockSize() == 0);
HashBlock result;
result.resizeForOverwrite(block.size());
for (qsizetype i = 0; i < block.size(); ++i)

View File

@ -823,7 +823,7 @@ Q_OUTOFLINE_TEMPLATE void QVLABase<T>::reallocate_impl(qsizetype prealloc, void
qsizetype osize = size();
const qsizetype copySize = qMin(asize, osize);
Q_ASSUME(copySize >= 0);
Q_ASSERT(copySize >= 0);
if (aalloc != capacity()) {
QVLABaseBase::malloced_ptr guard;

View File

@ -2261,7 +2261,7 @@ void QRasterPaintEngine::drawImage(const QRectF &r, const QImage &img, const QRe
|| d->rasterBuffer->compositionMode == QPainter::CompositionMode_Source))
{
RotationType rotationType = qRotationType(s->matrix);
Q_ASSUME(d->rasterBuffer->format < QImage::NImageFormats);
Q_ASSERT(d->rasterBuffer->format < QImage::NImageFormats);
const QPixelLayout::BPP plBpp = qPixelLayouts[d->rasterBuffer->format].bpp;
if (rotationType != NoRotation && qMemRotateFunctions[plBpp][rotationType] && img.rect().contains(sr.toAlignedRect())) {

View File

@ -902,7 +902,7 @@ void QPaintEngineEx::drawPoints(const QPoint *points, int pointCount)
void QPaintEngineEx::drawPolygon(const QPointF *points, int pointCount, PolygonDrawMode mode)
{
Q_ASSUME(pointCount >= 2);
Q_ASSERT(pointCount >= 2);
QVectorPath path((const qreal *) points, pointCount, nullptr, QVectorPath::polygonFlags(mode));
if (mode == PolylineMode)
@ -913,7 +913,7 @@ void QPaintEngineEx::drawPolygon(const QPointF *points, int pointCount, PolygonD
void QPaintEngineEx::drawPolygon(const QPoint *points, int pointCount, PolygonDrawMode mode)
{
Q_ASSUME(pointCount >= 2);
Q_ASSERT(pointCount >= 2);
int count = pointCount<<1;
QVarLengthArray<qreal> pts(count);

View File

@ -481,7 +481,7 @@ void QNetworkDatagram::makeReply_helper_inplace(const QByteArray &data)
void QNetworkDatagram::destroy(QNetworkDatagramPrivate *d)
{
Q_ASSUME(d);
Q_ASSERT(d);
delete d;
}

View File

@ -289,8 +289,8 @@ QImage QGtk3Interface::qt_convert_gdk_pixbuf(GdkPixbuf *buf) const
const guint8 *gdata = gdk_pixbuf_read_pixels(buf);
static_assert(std::is_same<decltype(gdata), const uchar *>::value,
"guint8 has diverted from uchar. Code needs fixing.");
Q_ASSUME(gdk_pixbuf_get_bits_per_sample(buf) == 8);
Q_ASSUME(gdk_pixbuf_get_n_channels(buf) == 4);
Q_ASSERT(gdk_pixbuf_get_bits_per_sample(buf) == 8);
Q_ASSERT(gdk_pixbuf_get_n_channels(buf) == 4);
const uchar *data = static_cast<const uchar *>(gdata);
const int width = gdk_pixbuf_get_width(buf);

View File

@ -2803,11 +2803,11 @@ void tst_QWindow::stateChangeSignal()
"On other operating systems, the signal may be emitted twice.");
#endif
QWindow w;
Q_ASSUME(connect (&w, &QWindow::windowStateChanged, [](Qt::WindowState s){qCDebug(lcTests) << "State change to" << s;}));
Q_ASSERT(connect (&w, &QWindow::windowStateChanged, [](Qt::WindowState s){qCDebug(lcTests) << "State change to" << s;}));
QSignalSpy spy(&w, SIGNAL(windowStateChanged(Qt::WindowState)));
unsigned short signalCount = 0;
QList<Qt::WindowState> effectiveStates;
Q_ASSUME(connect(&w, &QWindow::windowStateChanged, [&effectiveStates](Qt::WindowState state)
Q_ASSERT(connect(&w, &QWindow::windowStateChanged, [&effectiveStates](Qt::WindowState state)
{ effectiveStates.append(state); }));
// Part 1:
// => test signal emission on programmatic state changes