Long live QKeyCombination!

C++20 via P1120 is deprecating arithmetic operations between
unrelated enumeration types, and GCC 10 is already complaining.
Hence, these operations might become illegal in C++23 or C++26 at
the latest.

A case of this that affects Qt is in key combinations: a
QKeySequence can be constructed by summing / ORing modifiers and a
key, for instance:

  Qt::CTRL + Qt::Key_A
  Qt::SHIFT | Qt::CTRL | Qt::Key_G (recommended, see below)

The problem is that the modifiers and the key belong to different
enumerations (and there's 2 enumerations for the modifier, and one
for the key).

To solve this: add a dedicated class to represent a combination of
keys, and operators between those enumerations to build instances
of this class.

I would've simply defined operator|, but again docs and pre-existing
code use operator+ as well, so added both to at least tackle simple
cases (modifier + key).

Multiple modifiers create a problem: operator+ between them yields
int, not the corresponding flags type (because operator+ is not
overloaded for this use case):

  Qt::CTRL + Qt::SHIFT + Qt::Key_A
  \__________________/      /
          int              /
           \______________/
                  int

Not only this loses track of the datatypes involved, but it would
also then "add" the key (with NO warnings, now its int + enum, so
it's not mixing enums!) and yielding int again.

I don't want to special-case this; the point of the class is
that int is the wrong datatype. Everything works just fine when
using operator| instead:

  Qt::CTRL | Qt::SHIFT | Qt::Key_A
  \__________________/      /
      Qt::Modifiers        /
           \______________/
            QKeyCombination

So I'm defining operator+ so that the simple cases still work,
but also deprecating it.

Port some code around Qt to the new class. In certain cases,
it's a huge win for clarity. In some others, I've just added
the necessary casts to make it still compile without warnings,
without attempting refactorings.

[ChangeLog][QtCore][QKeyCombination] New class to represent
a combination of a key and zero or more modifiers, to be used
when defining shortcuts or similar.

[ChangeLog][Potentially Source-Incompatible Changes] A keyboard
modifier (such as Qt::CTRL, Qt::AltModifier, etc.) should be
combined with a key (such as Qt::Key_A, Qt::Key_F1, etc.) by using
operator|, not operator+.  The result is now an object of type
QKeyCombination, that stores the key and the modifiers.

Change-Id: I657a3a328232f059023fff69c5031ee31cc91dd6
Reviewed-by: Volker Hilsheimer <volker.hilsheimer@qt.io>

9 files changed, 59 insertions, 33 deletions

diff --git a/src/gui/kernel/qevent.cpp b/src/gui/kernel/qevent.cpp
index 1a6a43a4cb..b4880eff53 100644
--- a/src/gui/kernel/qevent.cpp
+++ b/src/gui/kernel/qevent.cpp
@@ -1285,6 +1285,15 @@ Qt::KeyboardModifiers QKeyEvent::modifiers() const
     return QInputEvent::modifiers();
 }
 
+/*!
+    \fn QKeyCombination QKeyEvent::keyCombination() const
+
+    Returns a QKeyCombination object containing both the key() and
+    the modifiers() carried by this event.
+
+    \since 6.0
+*/
+
 #if QT_CONFIG(shortcut)
 /*!
     \fn bool QKeyEvent::matches(QKeySequence::StandardKey key) const
diff --git a/src/gui/kernel/qevent.h b/src/gui/kernel/qevent.h
index 2c0325296d..c8402b6632 100644
--- a/src/gui/kernel/qevent.h
+++ b/src/gui/kernel/qevent.h
@@ -480,6 +480,10 @@ public:
     bool matches(QKeySequence::StandardKey key) const;
 #endif
     Qt::KeyboardModifiers modifiers() const;
+    QKeyCombination keyCombination() const
+    {
+        return QKeyCombination(modifiers(), Qt::Key(m_key));
+    }
     inline QString text() const { return m_text; }
     inline bool isAutoRepeat() const { return m_autoRepeat; }
     inline int count() const { return int(m_count); }
diff --git a/src/gui/kernel/qguivariant.cpp b/src/gui/kernel/qguivariant.cpp
index ebb180dc8e..1af0692173 100644
--- a/src/gui/kernel/qguivariant.cpp
+++ b/src/gui/kernel/qguivariant.cpp
@@ -99,7 +99,6 @@ struct GuiTypesFilter {
     };
 };
 
-
 static const struct : QMetaTypeModuleHelper
 {
 #define QT_IMPL_METATYPEINTERFACE_GUI_TYPES(MetaTypeName, MetaTypeId, RealName) \
@@ -146,7 +145,7 @@ static const struct : QMetaTypeModuleHelper
         );
         QMETATYPE_CONVERTER(QKeySequence, QString, result = source; return true;);
         QMETATYPE_CONVERTER(Int, QKeySequence,
-            result = source.isEmpty() ? 0 : source[0];
+            result = source.isEmpty() ? 0 : source[0].toCombined();
             return true;
         );
         QMETATYPE_CONVERTER(QKeySequence, Int, result = source; return true;);
diff --git a/src/gui/kernel/qkeymapper.cpp b/src/gui/kernel/qkeymapper.cpp
index f0ea66dc79..89eacc944a 100644
--- a/src/gui/kernel/qkeymapper.cpp
+++ b/src/gui/kernel/qkeymapper.cpp
@@ -71,17 +71,20 @@ QKeyMapper::~QKeyMapper()
 {
 }
 
-QList<int> QKeyMapper::possibleKeys(QKeyEvent *e)
+static QList<int> extractKeyFromEvent(QKeyEvent *e)
 {
     QList<int> result;
+    if (e->key() && (e->key() != Qt::Key_unknown))
+        result << e->keyCombination().toCombined();
+    else if (!e->text().isEmpty())
+        result << int(e->text().at(0).unicode() + (int)e->modifiers());
+    return result;
+}
 
-    if (!e->nativeScanCode()) {
-        if (e->key() && (e->key() != Qt::Key_unknown))
-            result << int(e->key() + e->modifiers());
-        else if (!e->text().isEmpty())
-            result << int(e->text().at(0).unicode() + e->modifiers());
-        return result;
-    }
+QList<int> QKeyMapper::possibleKeys(QKeyEvent *e)
+{
+    if (!e->nativeScanCode())
+        return extractKeyFromEvent(e);
 
     return instance()->d_func()->possibleKeys(e);
 }
@@ -132,11 +135,7 @@ QList<int> QKeyMapperPrivate::possibleKeys(QKeyEvent *e)
     if (!result.isEmpty())
         return result;
 
-    if (e->key() && (e->key() != Qt::Key_unknown))
-        result << int(e->key() + e->modifiers());
-    else if (!e->text().isEmpty())
-        result << int(e->text().at(0).unicode() + e->modifiers());
-    return result;
+    return extractKeyFromEvent(e);
 }
 
 QT_END_NAMESPACE
diff --git a/src/gui/kernel/qkeysequence.cpp b/src/gui/kernel/qkeysequence.cpp
index 13e6a058cb..5d52ba139d 100644
--- a/src/gui/kernel/qkeysequence.cpp
+++ b/src/gui/kernel/qkeysequence.cpp
@@ -870,6 +870,17 @@ QKeySequence::QKeySequence(int k1, int k2, int k3, int k4)
 }
 
 /*!
+    Constructs a key sequence with up to 4 keys \a k1, \a k2,
+    \a k3 and \a k4.
+
+    \sa QKeyCombination
+*/
+QKeySequence::QKeySequence(QKeyCombination k1, QKeyCombination k2, QKeyCombination k3, QKeyCombination k4)
+    : QKeySequence(k1.toCombined(), k2.toCombined(), k3.toCombined(), k4.toCombined())
+{
+}
+
+/*!
     Copy constructor. Makes a copy of \a keysequence.
  */
 QKeySequence::QKeySequence(const QKeySequence& keysequence)
@@ -908,11 +919,11 @@ QKeySequence::~QKeySequence()
     delivery.
 */
 
-void QKeySequence::setKey(int key, int index)
+void QKeySequence::setKey(QKeyCombination key, int index)
 {
     Q_ASSERT_X(index >= 0 && index < QKeySequencePrivate::MaxKeyCount, "QKeySequence::setKey", "index out of range");
     qAtomicDetach(d);
-    d->key[index] = key;
+    d->key[index] = key.toCombined();
 }
 
 static_assert(QKeySequencePrivate::MaxKeyCount == 4, "Change docs below");
@@ -967,7 +978,7 @@ QKeySequence QKeySequence::mnemonic(const QString &text)
             if (c.isPrint()) {
                 if (!found) {
                     c = c.toUpper();
-                    ret = QKeySequence(c.unicode() + Qt::ALT);
+                    ret = QKeySequence(QKeyCombination(Qt::ALT, Qt::Key(c.unicode())));
 #ifdef QT_NO_DEBUG
                     return ret;
 #else
@@ -1377,8 +1388,8 @@ QKeySequence::SequenceMatch QKeySequence::matches(const QKeySequence &seq) const
     SequenceMatch match = (userN == seqN ? ExactMatch : PartialMatch);
 
     for (uint i = 0; i < userN; ++i) {
-        int userKey = (*this)[i],
-            sequenceKey = seq[i];
+        QKeyCombination userKey = (*this)[i],
+                    sequenceKey = seq[i];
         if (userKey != sequenceKey)
             return NoMatch;
     }
@@ -1397,10 +1408,10 @@ QKeySequence::operator QVariant() const
     Returns a reference to the element at position \a index in the key
     sequence. This can only be used to read an element.
  */
-int QKeySequence::operator[](uint index) const
+QKeyCombination QKeySequence::operator[](uint index) const
 {
     Q_ASSERT_X(index < QKeySequencePrivate::MaxKeyCount, "QKeySequence::operator[]", "index out of range");
-    return d->key[index];
+    return QKeyCombination::fromCombined(d->key[index]);
 }
 
 
diff --git a/src/gui/kernel/qkeysequence.h b/src/gui/kernel/qkeysequence.h
index 8b7ff12bed..880d0a581a 100644
--- a/src/gui/kernel/qkeysequence.h
+++ b/src/gui/kernel/qkeysequence.h
@@ -155,6 +155,10 @@ public:
     QKeySequence();
     QKeySequence(const QString &key, SequenceFormat format = NativeText);
     QKeySequence(int k1, int k2 = 0, int k3 = 0, int k4 = 0);
+    QKeySequence(QKeyCombination k1,
+                 QKeyCombination k2 = QKeyCombination::fromCombined(0),
+                 QKeyCombination k3 = QKeyCombination::fromCombined(0),
+                 QKeyCombination k4 = QKeyCombination::fromCombined(0));
     QKeySequence(const QKeySequence &ks);
     QKeySequence(StandardKey key);
     ~QKeySequence();
@@ -179,7 +183,7 @@ public:
     static QList<QKeySequence> keyBindings(StandardKey key);
 
     operator QVariant() const;
-    int operator[](uint i) const;
+    QKeyCombination operator[](uint i) const;
     QKeySequence &operator=(const QKeySequence &other);
     QKeySequence &operator=(QKeySequence &&other) noexcept { swap(other); return *this; }
     void swap(QKeySequence &other) noexcept { qSwap(d, other.d); }
@@ -201,7 +205,7 @@ private:
     static QString encodeString(int key);
     int assign(const QString &str);
     int assign(const QString &str, SequenceFormat format);
-    void setKey(int key, int index);
+    void setKey(QKeyCombination key, int index);
 
     QKeySequencePrivate *d;
 
diff --git a/src/gui/kernel/qkeysequence_p.h b/src/gui/kernel/qkeysequence_p.h
index 8c59505561..99d17e98d1 100644
--- a/src/gui/kernel/qkeysequence_p.h
+++ b/src/gui/kernel/qkeysequence_p.h
@@ -64,7 +64,7 @@ struct QKeyBinding
 {
     QKeySequence::StandardKey standardKey;
     uchar priority;
-    uint shortcut;
+    QKeyCombination shortcut;
     uint platform;
 };
 
diff --git a/src/gui/kernel/qplatformtheme.cpp b/src/gui/kernel/qplatformtheme.cpp
index 81c897def7..5de1c98127 100644
--- a/src/gui/kernel/qplatformtheme.cpp
+++ b/src/gui/kernel/qplatformtheme.cpp
@@ -645,7 +645,7 @@ QList<QKeySequence> QPlatformTheme::keyBindings(QKeySequence::StandardKey key) c
         if (!(it->platform & platform))
             continue;
 
-        uint shortcut = it->shortcut;
+        uint shortcut = it->shortcut.toCombined();
 
         if (it->priority > 0)
             list.prepend(QKeySequence(shortcut));
diff --git a/src/gui/kernel/qshortcutmap.cpp b/src/gui/kernel/qshortcutmap.cpp
index 4b6368a749..e79aaacd1a 100644
--- a/src/gui/kernel/qshortcutmap.cpp
+++ b/src/gui/kernel/qshortcutmap.cpp
@@ -543,12 +543,12 @@ void QShortcutMap::createNewSequences(QKeyEvent *e, QList<QKeySequence> &ksl, in
                 curKsl.setKey(curSeq[2], 2);
                 curKsl.setKey(curSeq[3], 3);
             } else {
-                curKsl.setKey(0, 0);
-                curKsl.setKey(0, 1);
-                curKsl.setKey(0, 2);
-                curKsl.setKey(0, 3);
+                curKsl.setKey(QKeyCombination::fromCombined(0), 0);
+                curKsl.setKey(QKeyCombination::fromCombined(0), 1);
+                curKsl.setKey(QKeyCombination::fromCombined(0), 2);
+                curKsl.setKey(QKeyCombination::fromCombined(0), 3);
             }
-            curKsl.setKey(possibleKeys.at(pkNum) & ~ignoredModifiers, index);
+            curKsl.setKey(QKeyCombination::fromCombined(possibleKeys.at(pkNum) & ~ignoredModifiers), index);
         }
     }
 }
@@ -574,8 +574,8 @@ QKeySequence::SequenceMatch QShortcutMap::matches(const QKeySequence &seq1,
                                             : QKeySequence::PartialMatch);
 
     for (uint i = 0; i < userN; ++i) {
-        int userKey = seq1[i],
-            sequenceKey = seq2[i];
+        int userKey = seq1[i].toCombined(),
+            sequenceKey = seq2[i].toCombined();
         if ((userKey & Qt::Key_unknown) == Qt::Key_hyphen)
             userKey = (userKey & Qt::KeyboardModifierMask) | Qt::Key_Minus;
         if ((sequenceKey & Qt::Key_unknown) == Qt::Key_hyphen)