From 631a0cc45cbe70940746cd944d325d7d3bab8c15 Mon Sep 17 00:00:00 2001 From: Giuseppe D'Angelo Date: Sun, 2 May 2021 03:06:18 +0200 Subject: 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 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 Reviewed-by: Edward Welbourne Reviewed-by: Thiago Macieira --- src/corelib/global/qflags.h | 52 +++++++++++++++++++++++++++++++++++++++------ 1 file changed, 46 insertions(+), 6 deletions(-) (limited to 'src/corelib/global/qflags.h') diff --git a/src/corelib/global/qflags.h b/src/corelib/global/qflags.h index ab46af2d8e..7f96d900fb 100644 --- a/src/corelib/global/qflags.h +++ b/src/corelib/global/qflags.h @@ -38,6 +38,7 @@ ****************************************************************************/ #include +#include #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::const_iterator it, typename std::initializer_list::const_iterator end) @@ -183,6 +211,19 @@ private: typedef QFlags 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 operator|(Flags::enum_type f1, Flags::enum_type f2) noexcept \ { return QFlags(f1) | f2; } \ @@ -198,12 +239,11 @@ constexpr inline void operator+(int f1, QFlags 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 f2) noexcept = delete; \ constexpr inline void operator-(int f1, QFlags 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 -- cgit v1.2.3