From 0624d81ded955faf4192e389136f7a2d30c24fa1 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). Task-number: QTBUG-99948 Change-Id: I86ec11c1e4d31dfa81e2c3aad031b2aa113503eb Reviewed-by: Qt CI Bot Reviewed-by: Allan Sandfeld Jensen (cherry picked from commit 78073f8ab688f1dc2ede939984ec347c0867de18) Reviewed-by: Fabian Kosmale --- 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 ab46af2d8e7..c6cbaa1adb2 100644 --- a/src/corelib/global/qflags.h +++ b/src/corelib/global/qflags.h @@ -205,6 +205,33 @@ 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; +// 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 6bbe2426098..99678cf97e7 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 be235aa0bf7..bf271223f73 100644 --- a/src/corelib/text/qunicodetools.cpp +++ b/src/corelib/text/qunicodetools.cpp @@ -1699,6 +1699,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 { @@ -1946,6 +1948,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 5e948175f31..977f8008ead 100644 --- a/src/widgets/kernel/qsizepolicy.h +++ b/src/widgets/kernel/qsizepolicy.h @@ -199,6 +199,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 c252d797b21..e9caaf6dce4 100644 --- a/tests/auto/corelib/global/qflags/tst_qflags.cpp +++ b/tests/auto/corelib/global/qflags/tst_qflags.cpp @@ -35,6 +35,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; @@ -109,7 +110,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 }