Long live QT_TYPESAFE_FLAGS!

This commit adds an opt-in mechanism to kill type-unsafe functions
and implicit conversions of QFlags, therefore removing an entire
category of bugs that QFlags itself is supposed to protect against:

  QFlags<E> f;
  f == 3.14; // OK; f->int, int->double, then operator==(double,double)
  f & UnrelatedEnum;  // OK if of unscoped enum, calls operator&(int)
  f &= 123;   // calls QFlags::operator&=(int)
  int i = f * 42; // f->int, then operator*(int, int)

Thankfully, operator+ and operator- had already been deleted.

By defining QT_TYPESAFE_FLAGS one:

* disables operators taking (u)int, forcing the usage of an enumeration
or another QFlags object;

* turns the implicit conversions towards integers/bool in explicit
(one can always use fromInt/toInt instead);

* adds a convenience set of (in)equality operators against literal 0,
in order to keep code like `(flag & bit) == 0` compile. This set can't
be added normally otherwise it would add ambiguity;

* adds the unary operator~(Enum), turning it into a flag. This is
a source incompatible change, in general, so it's opt-in.

This is a Qt-internal macro at this point. It can't be offered to users
yet because we need to fix some public API flaws first: in some places
(e.g. QPainter::drawText)  we ask users to do type-unsafe manipulation
of flags. A user enabling the macro wouldn't be able to properly use the
functions in question.

This macro will be enabled in a follow-up commit.

Change-Id: I796f2256b446bafc12cdcbaf7de417d12bd3619e
Reviewed-by: Andrei Golubev <andrei.golubev@qt.io>
Reviewed-by: Edward Welbourne <edward.welbourne@qt.io>
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>
This commit is contained in:
Giuseppe D'Angelo 2021-05-02 03:06:18 +02:00
parent a20a424066
commit 631a0cc45c
4 changed files with 117 additions and 16 deletions

View File

@ -36,6 +36,7 @@ qt_internal_add_module(Core
EXCEPTIONS
SOURCES
global/archdetect.cpp
global/qcompare_impl.h
global/qcompare.h
global/qcompilerdetection.h
global/qcontainerinfo.h

View File

@ -45,6 +45,7 @@
#endif
#include <QtCore/qglobal.h>
#include <QtCore/qcompare_impl.h>
QT_BEGIN_NAMESPACE
@ -65,16 +66,6 @@ enum class Uncomparable : CompareUnderlyingType
Unordered = -127
};
// [cmp.categories.pre] / 3, but using a safe bool trick
// and also rejecting std::nullptr_t (unlike the example)
class CompareAgainstLiteralZero {
public:
using SafeZero = void (CompareAgainstLiteralZero::*)();
Q_IMPLICIT constexpr CompareAgainstLiteralZero(SafeZero) noexcept {}
template <typename T, std::enable_if_t<!std::is_same_v<T, int>, bool> = false>
CompareAgainstLiteralZero(T) = delete;
};
} // namespace QtPrivate
// [cmp.partialord]

View File

@ -0,0 +1,69 @@
/****************************************************************************
**
** Copyright (C) 2020 Klarälvdalens Datakonsult AB, a KDAB Group company, info@kdab.com, author Giuseppe D'Angelo <giuseppe.dangelo@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$
**
****************************************************************************/
#if 0
#pragma qt_sync_skip_header_check
#pragma qt_sync_stop_processing
#endif
#ifndef QCOMPARE_IMPL_H
#define QCOMPARE_IMPL_H
#include <QtCore/qglobal.h>
QT_BEGIN_NAMESPACE
namespace QtPrivate {
// [cmp.categories.pre] / 3, but using a safe bool trick
// and also rejecting std::nullptr_t (unlike the example)
class CompareAgainstLiteralZero {
public:
using SafeZero = void (CompareAgainstLiteralZero::*)();
Q_IMPLICIT constexpr CompareAgainstLiteralZero(SafeZero) noexcept {}
template <typename T, std::enable_if_t<!std::is_same_v<T, int>, bool> = false>
CompareAgainstLiteralZero(T) = delete;
};
} // namespace QtPrivate
QT_END_NAMESPACE
#endif // QCOMPARE_IMPL_H

View File

@ -38,6 +38,7 @@
****************************************************************************/
#include <QtCore/qglobal.h>
#include <QtCore/qcompare_impl.h>
#ifndef QFLAGS_H
#define QFLAGS_H
@ -115,8 +116,10 @@ public:
constexpr static inline QFlags fromInt(Int i) noexcept { return QFlags(QFlag(i)); }
constexpr inline Int toInt() const noexcept { return i; }
#ifndef QT_TYPESAFE_FLAGS
constexpr inline QFlags &operator&=(int mask) noexcept { i &= mask; return *this; }
constexpr inline QFlags &operator&=(uint mask) noexcept { i &= mask; return *this; }
#endif
constexpr inline QFlags &operator&=(QFlags mask) noexcept { i &= mask.i; return *this; }
constexpr inline QFlags &operator&=(Enum mask) noexcept { i &= Int(mask); return *this; }
constexpr inline QFlags &operator|=(QFlags other) noexcept { i |= other.i; return *this; }
@ -124,14 +127,27 @@ public:
constexpr inline QFlags &operator^=(QFlags other) noexcept { i ^= other.i; return *this; }
constexpr inline QFlags &operator^=(Enum other) noexcept { i ^= Int(other); return *this; }
constexpr inline operator Int() const noexcept { return i; }
#ifdef QT_TYPESAFE_FLAGS
constexpr inline explicit operator Int() const noexcept { return i; }
constexpr inline explicit operator bool() const noexcept { return i; }
// For some reason, moc goes through QFlag in order to read/write
// properties of type QFlags; so a conversion to QFlag is also
// needed here. (It otherwise goes through a QFlags->int->QFlag
// conversion sequence.)
constexpr inline explicit operator QFlag() const noexcept { return QFlag(i); }
#else
constexpr inline Q_IMPLICIT operator Int() const noexcept { return i; }
constexpr inline bool operator!() const noexcept { return !i; }
#endif
constexpr inline QFlags operator|(QFlags other) const noexcept { return QFlags(QFlag(i | other.i)); }
constexpr inline QFlags operator|(Enum other) const noexcept { return QFlags(QFlag(i | Int(other))); }
constexpr inline QFlags operator^(QFlags other) const noexcept { return QFlags(QFlag(i ^ other.i)); }
constexpr inline QFlags operator^(Enum other) const noexcept { return QFlags(QFlag(i ^ Int(other))); }
#ifndef QT_TYPESAFE_FLAGS
constexpr inline QFlags operator&(int mask) const noexcept { return QFlags(QFlag(i & mask)); }
constexpr inline QFlags operator&(uint mask) const noexcept { return QFlags(QFlag(i & mask)); }
#endif
constexpr inline QFlags operator&(QFlags other) const noexcept { return QFlags(QFlag(i & other.i)); }
constexpr inline QFlags operator&(Enum other) const noexcept { return QFlags(QFlag(i & Int(other))); }
constexpr inline QFlags operator~() const noexcept { return QFlags(QFlag(~i)); }
@ -143,8 +159,6 @@ public:
constexpr inline void operator-(Enum other) const noexcept = delete;
constexpr inline void operator-(int other) const noexcept = delete;
constexpr inline bool operator!() const noexcept { return !i; }
constexpr inline bool testFlag(Enum flag) const noexcept { return testFlags(flag); }
constexpr inline bool testFlags(QFlags flags) const noexcept { return flags.i ? ((i & flags.i) == flags.i) : i == Int(0); }
constexpr inline bool testAnyFlag(Enum flag) const noexcept { return testAnyFlags(flag); }
@ -167,6 +181,20 @@ public:
friend constexpr inline bool operator!=(Enum lhs, QFlags rhs) noexcept
{ return QFlags(lhs) != rhs; }
#ifdef QT_TYPESAFE_FLAGS
// Provide means of comparing flags against a literal 0; opt-in
// because otherwise they're ambiguous against operator==(int,int)
// after a QFlags->int conversion.
friend constexpr inline bool operator==(QFlags flags, QtPrivate::CompareAgainstLiteralZero) noexcept
{ return flags.i == Int(0); }
friend constexpr inline bool operator!=(QFlags flags, QtPrivate::CompareAgainstLiteralZero) noexcept
{ return flags.i != Int(0); }
friend constexpr inline bool operator==(QtPrivate::CompareAgainstLiteralZero, QFlags flags) noexcept
{ return Int(0) == flags.i; }
friend constexpr inline bool operator!=(QtPrivate::CompareAgainstLiteralZero, QFlags flags) noexcept
{ return Int(0) != flags.i; }
#endif
private:
constexpr static inline Int initializer_list_helper(typename std::initializer_list<Enum>::const_iterator it,
typename std::initializer_list<Enum>::const_iterator end)
@ -183,6 +211,19 @@ private:
typedef QFlags<Enum> Flags;
#endif
#ifdef QT_TYPESAFE_FLAGS
// These are opt-in, for backwards compatibility
#define QT_DECLARE_TYPESAFE_OPERATORS_FOR_FLAGS_ENUM(Flags) \
constexpr inline Flags operator~(Flags::enum_type e) noexcept \
{ return ~Flags(e); } \
constexpr inline void operator|(Flags::enum_type f1, int f2) noexcept = delete;
#else
#define QT_DECLARE_TYPESAFE_OPERATORS_FOR_FLAGS_ENUM(Flags) \
constexpr inline QIncompatibleFlag operator|(Flags::enum_type f1, int f2) noexcept \
{ return QIncompatibleFlag(int(f1) | f2); }
#endif
#define Q_DECLARE_OPERATORS_FOR_FLAGS(Flags) \
constexpr inline QFlags<Flags::enum_type> operator|(Flags::enum_type f1, Flags::enum_type f2) noexcept \
{ return QFlags<Flags::enum_type>(f1) | f2; } \
@ -198,12 +239,11 @@ constexpr inline void operator+(int f1, QFlags<Flags::enum_type> f2) noexcept =
constexpr inline void operator-(Flags::enum_type f1, Flags::enum_type f2) noexcept = delete; \
constexpr inline void operator-(Flags::enum_type f1, QFlags<Flags::enum_type> f2) noexcept = delete; \
constexpr inline void operator-(int f1, QFlags<Flags::enum_type> f2) noexcept = delete; \
constexpr inline QIncompatibleFlag operator|(Flags::enum_type f1, int f2) noexcept \
{ return QIncompatibleFlag(int(f1) | f2); } \
constexpr inline void operator+(int f1, Flags::enum_type f2) noexcept = delete; \
constexpr inline void operator+(Flags::enum_type f1, int f2) noexcept = delete; \
constexpr inline void operator-(int f1, Flags::enum_type f2) noexcept = delete; \
constexpr inline void operator-(Flags::enum_type f1, int f2) noexcept = delete;
constexpr inline void operator-(Flags::enum_type f1, int f2) noexcept = delete; \
QT_DECLARE_TYPESAFE_OPERATORS_FOR_FLAGS_ENUM(Flags)
QT_END_NAMESPACE