From 78073f8ab688f1dc2ede939984ec347c0867de18 Mon Sep 17 00:00:00 2001 From: Marc Mutz Date: Sat, 26 Feb 2022 03:26:30 +0100 Subject: [PATCH] Restore C++20-deprecated mixed-enum bitwise operators C++20 deprecated arithmetic on enum types. For enums used on QFlags<>, these operators have always been user-defined, but when the two enums are of different type, such as QFrame::Shape and QFrame::Shadow, the deprecation warning pops up. We have in the past fixed these in our headers by manual casts, but that doesn't help our users when our API requires them to OR together enums of different type. Until we can rework these APIs to use a variadic QFlags type, we need to fix it in an SC and BC way, which is what this patch sets out to do. The idea is simply to mark pairs of enums that are designed to be ORed together and replace the deprecated built-in bitwise operators with user-defined ones in C++20. To ensure SC and BC, we pass an explicit result type and use that to check, in C++17 builds, that it matches the decltype of the result of the built-in operator. This patch is the first in a series of similar patches. It introduces said markup macro and applies it to all enum pairs that create warnings on (my) Linux GCC 11.3 and Clang 10.0.0 builds. It is expected that more such markups are needed, for other modules, and for symmetry. Even with this patch, there is one mixed-enum warning left, in qxcbwindow.cpp. This appears to be a genuine bug (cf. QTBUG-101306), so this patch doesn't mark the enums involved in it as designed to be used together. This patch also unearthed that QT_TYPESAFE_FLAGS, possibly unsurprisingly so, breaks several mixed bitwise flags-enum operations (QTBUG-101344). Pick-to: 6.3 6.2 5.15 Task-number: QTBUG-99948 Change-Id: I86ec11c1e4d31dfa81e2c3aad031b2aa113503eb Reviewed-by: Qt CI Bot Reviewed-by: Allan Sandfeld Jensen --- src/corelib/global/qflags.h | 27 ++++++++++++++ src/corelib/global/qnamespace.h | 2 ++ src/corelib/text/qunicodetools.cpp | 3 ++ src/widgets/kernel/qsizepolicy.h | 1 + src/widgets/widgets/qframe.h | 2 ++ .../auto/corelib/global/qflags/tst_qflags.cpp | 35 +++++++++++++++++++ 6 files changed, 70 insertions(+) diff --git a/src/corelib/global/qflags.h b/src/corelib/global/qflags.h index 5e27ea9304e..4ca57b0e978 100644 --- a/src/corelib/global/qflags.h +++ b/src/corelib/global/qflags.h @@ -249,6 +249,33 @@ constexpr inline void operator-(int f1, Flags::enum_type f2) noexcept = delete; constexpr inline void operator-(Flags::enum_type f1, int f2) noexcept = delete; \ QT_DECLARE_TYPESAFE_OPERATORS_FOR_FLAGS_ENUM(Flags) +// restore bit-wise enum-enum operators deprecated in C++20, +// but used in a few places in the API +#if __cplusplus > 201702L // assume compilers don't warn if in C++17 mode + // in C++20 mode, provide user-defined operators to override the deprecated operations: +# define Q_DECLARE_MIXED_ENUM_OPERATOR(op, Ret, LHS, RHS) \ + constexpr inline Ret operator op (LHS lhs, RHS rhs) noexcept \ + { return static_cast(qToUnderlying(lhs) op qToUnderlying(rhs)); } \ + /* end */ +#else + // in C++17 mode, statically-assert that this compiler's result of the + // operation is the same that the C++20 version would produce: +# define Q_DECLARE_MIXED_ENUM_OPERATOR(op, Ret, LHS, RHS) \ + static_assert(std::is_same_v() op std::declval()), Ret>); +#endif + +#define Q_DECLARE_MIXED_ENUM_OPERATORS(Ret, Flags, Enum) \ + Q_DECLARE_MIXED_ENUM_OPERATOR(|, Ret, Flags, Enum) \ + Q_DECLARE_MIXED_ENUM_OPERATOR(&, Ret, Flags, Enum) \ + Q_DECLARE_MIXED_ENUM_OPERATOR(^, Ret, Flags, Enum) \ + /* end */ + +#define Q_DECLARE_MIXED_ENUM_OPERATORS_SYMMETRIC(Ret, Flags, Enum) \ + Q_DECLARE_MIXED_ENUM_OPERATORS(Ret, Flags, Enum) \ + Q_DECLARE_MIXED_ENUM_OPERATORS(Ret, Enum, Flags) \ + /* end */ + + QT_END_NAMESPACE #endif // QFLAGS_H diff --git a/src/corelib/global/qnamespace.h b/src/corelib/global/qnamespace.h index 843e3f85854..bcf4dabf593 100644 --- a/src/corelib/global/qnamespace.h +++ b/src/corelib/global/qnamespace.h @@ -208,6 +208,7 @@ namespace Qt { // size of a multi-variant string. TextLongestVariant = 0x80000 }; + Q_DECLARE_MIXED_ENUM_OPERATORS_SYMMETRIC(int, AlignmentFlag, TextFlag) enum TextElideMode { ElideLeft, @@ -215,6 +216,7 @@ namespace Qt { ElideMiddle, ElideNone }; + Q_DECLARE_MIXED_ENUM_OPERATORS_SYMMETRIC(int, TextElideMode, TextFlag) enum WhiteSpaceMode { WhiteSpaceNormal, diff --git a/src/corelib/text/qunicodetools.cpp b/src/corelib/text/qunicodetools.cpp index a924addda05..8223c759e9c 100644 --- a/src/corelib/text/qunicodetools.cpp +++ b/src/corelib/text/qunicodetools.cpp @@ -1695,6 +1695,8 @@ enum MymrCharClassFlags { Mymr_CF_AFTER_KINZI = 0x00100000 }; +Q_DECLARE_MIXED_ENUM_OPERATORS(int, MymrCharClassValues, MymrCharClassFlags) + /* Characters that get refrered to by name */ enum MymrChar { @@ -1942,6 +1944,7 @@ enum KhmerCharClassFlags { CF_POS_MASK = 0x000f0000 }; +Q_DECLARE_MIXED_ENUM_OPERATORS(int, KhmerCharClassValues, KhmerCharClassFlags) /* Characters that get referred to by name */ enum KhmerChar { diff --git a/src/widgets/kernel/qsizepolicy.h b/src/widgets/kernel/qsizepolicy.h index 53c7afc9069..e6d98b1e650 100644 --- a/src/widgets/kernel/qsizepolicy.h +++ b/src/widgets/kernel/qsizepolicy.h @@ -198,6 +198,7 @@ private: Q_DECLARE_TYPEINFO(QSizePolicy, Q_PRIMITIVE_TYPE); Q_DECLARE_OPERATORS_FOR_FLAGS(QSizePolicy::ControlTypes) +Q_DECLARE_MIXED_ENUM_OPERATORS(int, QSizePolicy::Policy, QSizePolicy::PolicyFlag) #ifndef QT_NO_DATASTREAM Q_WIDGETS_EXPORT QDataStream &operator<<(QDataStream &, const QSizePolicy &); diff --git a/src/widgets/widgets/qframe.h b/src/widgets/widgets/qframe.h index 144003bc0ef..2e97938c6b1 100644 --- a/src/widgets/widgets/qframe.h +++ b/src/widgets/widgets/qframe.h @@ -123,6 +123,8 @@ private: Q_DECLARE_PRIVATE(QFrame) }; +Q_DECLARE_MIXED_ENUM_OPERATORS_SYMMETRIC(int, QFrame::Shape, QFrame::Shadow) + QT_END_NAMESPACE #endif // QFRAME_H diff --git a/tests/auto/corelib/global/qflags/tst_qflags.cpp b/tests/auto/corelib/global/qflags/tst_qflags.cpp index 09c2b0743d9..cdc5b056331 100644 --- a/tests/auto/corelib/global/qflags/tst_qflags.cpp +++ b/tests/auto/corelib/global/qflags/tst_qflags.cpp @@ -44,6 +44,7 @@ class tst_QFlags: public QObject private slots: void boolCasts() const; void operators() const; + void mixingDifferentEnums() const; void testFlag() const; void testFlagZeroFlag() const; void testFlagMultiBits() const; @@ -118,7 +119,41 @@ void tst_QFlags::operators() const CHECK(&, Qt::AlignHCenter, Qt::AlignHCenter, Qt::AlignHCenter); CHECK(^, Qt::AlignHCenter, Qt::AlignVCenter, Qt::AlignCenter); CHECK(^, Qt::AlignHCenter, Qt::AlignHCenter, Qt::Alignment()); +#undef CHECK +} +void tst_QFlags::mixingDifferentEnums() const +{ +#define CHECK(op, LHS, RHS, RES) \ + /* LHS must be QFlags'able */ \ + do { \ + QCOMPARE((LHS op RHS), (RES)); \ + QCOMPARE((RHS op LHS), (RES)); \ + /*QCOMPARE(( / *CTAD* / QFlags(LHS) op RHS), (RES));*/ \ + /*QCOMPARE((QFlags(LHS) op ## = RHS), (RES));*/ \ + } while (false) + + // AlignmentFlags <-> TextFlags + { + CHECK(|, Qt::AlignCenter, Qt::TextSingleLine, 0x0184); + CHECK(&, Qt::AlignCenter, Qt::TextSingleLine, 0x0000); + CHECK(^, Qt::AlignCenter, Qt::TextSingleLine, 0x0184); + } + // QFlags <-> TextFlags + { +#ifndef QT_TYPESAFE_FLAGS // QTBUG-101344 + Qt::Alignment MyAlignCenter = Qt::AlignCenter; // convert enum to QFlags + CHECK(|, MyAlignCenter, Qt::TextSingleLine, 0x0184U); // yes, unsigned! + CHECK(&, MyAlignCenter, Qt::TextSingleLine, 0x0000U); // yes, unsigned! + CHECK(^, MyAlignCenter, Qt::TextSingleLine, 0x0184U); // yes, unsigned! +#endif + } + // TextElideMode <-> TextFlags + { + CHECK(|, Qt::ElideNone, Qt::TextSingleLine, 0x0103); + CHECK(&, Qt::ElideNone, Qt::TextSingleLine, 0x0000); + CHECK(^, Qt::ElideNone, Qt::TextSingleLine, 0x0103); + } #undef CHECK }