QChar: make construction from integral explicit

QChar should not be convertible from any integral type except from
char16_t, short and possibly char (since it's a direct superset).

David provided the perfect example:

  if (str == 123) { ~~~ }

compiles, with 123 implicitly converted to QChar (str == "123"
was meant instead). But similarly one can construct other
scenarios where QString(123) gets accidentally used (instead of
QString::number(123)), like QString s; s += 123;.

Add a macro to revert to the implicit constructors, for backwards
compatibility.

The breaks are mostly in tests that "abuse" of integers (arithmetic,
etc.). Maybe it's time for user-defined literals for QChar/QString,
but that is left for another commit.

[ChangeLog][Potentially Source-Incompatible Changes][QChar] QChar
constructors from integral types are now by default explicit.
It is recommended to use explicit conversions, QLatin1Char,
QChar::fromUcs4 instead of implicit conversions. The old behavior
can be restored by defining the QT_IMPLICIT_QCHAR_CONSTRUCTION
macro.

Change-Id: I6175f6ab9bcf1956f6f97ab0c9d9d5aaf777296d
Reviewed-by: Volker Hilsheimer <volker.hilsheimer@qt.io>
Reviewed-by: Tor Arne Vestbø <tor.arne.vestbo@qt.io>

9 files changed, 87 insertions, 25 deletions

diff --git a/src/corelib/io/qstorageinfo_win.cpp b/src/corelib/io/qstorageinfo_win.cpp
index 8a3db90f87..5a65b78d63 100644
--- a/src/corelib/io/qstorageinfo_win.cpp
+++ b/src/corelib/io/qstorageinfo_win.cpp
@@ -192,7 +192,7 @@ QList<QStorageInfo> QStorageInfoPrivate::mountedVolumes()
             if (!drive.rootPath().isEmpty()) // drive exists, but not mounted
                 volumes.append(drive);
         }
-        driveName[0] = driveName[0].unicode() + 1;
+        driveName[0] = QChar(driveName[0].unicode() + 1);
         driveBits = driveBits >> 1;
     }
 
diff --git a/src/corelib/text/qchar.cpp b/src/corelib/text/qchar.cpp
index 14c82c7ee4..dcc36d18ce 100644
--- a/src/corelib/text/qchar.cpp
+++ b/src/corelib/text/qchar.cpp
@@ -158,6 +158,12 @@ QT_BEGIN_NAMESPACE
     to construct a QChar from an 8-bit \c char, and you will need to
     call toLatin1() to get the 8-bit value back.
 
+    Starting with Qt 6.0, most QChar constructors are \c explicit. This
+    is done to avoid dangerous mistakes when accidentally mixing
+    integral types and strings. You can opt-out (and make these
+    constructors implicit) by defining the macro \c
+    QT_IMPLICIT_QCHAR_CONSTRUCTION.
+
     For more information see
     \l{http://www.unicode.org/ucd/}{"About the Unicode Character Database"}.
 
@@ -2118,4 +2124,51 @@ static bool normalizationQuickCheckHelper(QString *str, QString::NormalizationFo
     return true;
 }
 
+/*!
+    \macro QT_IMPLICIT_QCHAR_CONSTRUCTION
+    \since 6.0
+    \relates QChar
+
+    Defining this macro makes certain QChar constructors implicit
+    rather than explicit. This is done to enforce safe conversions:
+
+    \badcode
+
+    QString str = getString();
+    if (str == 123) {
+        // Oops, meant str == "123". By default does not compile,
+        // *unless* this macro is defined, in which case, it's interpreted
+        // as `if (str == QChar(123))`, that is, `if (str == '{')`.
+        // Likely, not what we meant.
+    }
+
+    \endcode
+
+    This macro is provided to keep existing code working; it is
+    recommended to instead use explicit conversions and/or QLatin1Char.
+    For instance:
+
+    \code
+
+    QChar c1 =  'x'; // OK, unless QT_NO_CAST_FROM_ASCII is defined
+    QChar c2 = u'x'; // always OK, recommended
+    QChar c3 = QLatin1Char('x'); // always OK, recommended
+
+    // from int to 1 UTF-16 code unit: must guarantee that the input is <= 0xFFFF
+    QChar c4 = 120;        // compile error, unless QT_IMPLICIT_QCHAR_CONSTRUCTION is defined
+    QChar c5(120);         // OK (direct initialization)
+    auto  c6 = QChar(120); // ditto
+
+    // from int/char32_t to 1/2 UTF-16 code units:
+    // 𝄞 'MUSICAL SYMBOL G CLEF' (U+1D11E)
+    auto c7 = QChar(0x1D11E);           // compiles, but undefined behavior at runtime
+    auto c8 = QChar::fromUcs4(0x1D11E);       // always OK
+    auto c9 = QChar::fromUcs4(U'\U0001D11E'); // always OK
+    // => use c8/c9 as QStringView objects
+
+    \endcode
+
+    \sa QLatin1Char, QChar::fromUcs4, QT_NO_CAST_FROM_ASCII
+*/
+
 QT_END_NAMESPACE
diff --git a/src/corelib/text/qchar.h b/src/corelib/text/qchar.h
index 982abe9346..f66671e27c 100644
--- a/src/corelib/text/qchar.h
+++ b/src/corelib/text/qchar.h
@@ -101,12 +101,18 @@ public:
         LastValidCodePoint = 0x10ffff
     };
 
+#ifdef QT_IMPLICIT_QCHAR_CONSTRUCTION
+#define QCHAR_MAYBE_IMPLICIT Q_IMPLICIT
+#else
+#define QCHAR_MAYBE_IMPLICIT explicit
+#endif
+
     constexpr Q_IMPLICIT QChar() noexcept : ucs(0) {}
     constexpr Q_IMPLICIT QChar(ushort rc) noexcept : ucs(rc) {}
-    constexpr Q_IMPLICIT QChar(uchar c, uchar r) noexcept : ucs(char16_t((r << 8) | c)) {}
+    constexpr QCHAR_MAYBE_IMPLICIT QChar(uchar c, uchar r) noexcept : ucs(char16_t((r << 8) | c)) {}
     constexpr Q_IMPLICIT QChar(short rc) noexcept : ucs(char16_t(rc)) {}
-    constexpr Q_IMPLICIT QChar(uint rc) noexcept : ucs(char16_t(rc & 0xffff)) {}
-    constexpr Q_IMPLICIT QChar(int rc) noexcept : ucs(char16_t(rc & 0xffff)) {}
+    constexpr QCHAR_MAYBE_IMPLICIT QChar(uint rc) noexcept : ucs(char16_t(rc & 0xffff)) {}
+    constexpr QCHAR_MAYBE_IMPLICIT QChar(int rc) noexcept : ucs(char16_t(rc & 0xffff)) {}
     constexpr Q_IMPLICIT QChar(SpecialCharacter s) noexcept : ucs(char16_t(s)) {} // implicit
     constexpr Q_IMPLICIT QChar(QLatin1Char ch) noexcept : ucs(ch.unicode()) {} // implicit
     constexpr Q_IMPLICIT QChar(char16_t ch) noexcept : ucs(ch) {} // implicit
@@ -114,18 +120,19 @@ public:
     static_assert(sizeof(wchar_t) == sizeof(char16_t));
 #endif
 #if defined(Q_OS_WIN) || defined(Q_CLANG_QDOC)
-#   if !defined(_WCHAR_T_DEFINED) || defined(_NATIVE_WCHAR_T_DEFINED)
     constexpr Q_IMPLICIT QChar(wchar_t ch) noexcept : ucs(char16_t(ch)) {} // implicit
-#   endif
 #endif
 
 #ifndef QT_NO_CAST_FROM_ASCII
+    // Always implicit -- allow for 'x' => QChar conversions
     QT_ASCII_CAST_WARN constexpr Q_IMPLICIT QChar(char c) noexcept : ucs(uchar(c)) { }
 #ifndef QT_RESTRICTED_CAST_FROM_ASCII
-    QT_ASCII_CAST_WARN constexpr Q_IMPLICIT QChar(uchar c) noexcept : ucs(c) { }
+    QT_ASCII_CAST_WARN constexpr QCHAR_MAYBE_IMPLICIT QChar(uchar c) noexcept : ucs(c) { }
 #endif
 #endif
 
+#undef QCHAR_MAYBE_IMPLICIT
+
     static constexpr QChar fromUcs2(char16_t c) noexcept { return QChar{c}; }
     static constexpr inline auto fromUcs4(char32_t c) noexcept;
 
diff --git a/src/corelib/text/qstring.cpp b/src/corelib/text/qstring.cpp
index 1447b5b21c..c955cb8736 100644
--- a/src/corelib/text/qstring.cpp
+++ b/src/corelib/text/qstring.cpp
@@ -1509,8 +1509,10 @@ inline char qToLower(char ch)
 /*!
   \macro QT_NO_CAST_FROM_ASCII
   \relates QString
+  \relates QChar
 
-  Disables automatic conversions from 8-bit strings (char *) to unicode QStrings
+  Disables automatic conversions from 8-bit strings (char *) to unicode QStrings,
+  as well as from 8-bit char types (char and unsigned char) to QChar.
 
   \sa QT_NO_CAST_TO_ASCII, QT_RESTRICTED_CAST_FROM_ASCII, QT_NO_CAST_FROM_BYTEARRAY
 */
diff --git a/src/gui/text/windows/qwindowsfontdatabase.cpp b/src/gui/text/windows/qwindowsfontdatabase.cpp
index 9ee1c5aba6..79ea08f9b6 100644
--- a/src/gui/text/windows/qwindowsfontdatabase.cpp
+++ b/src/gui/text/windows/qwindowsfontdatabase.cpp
@@ -485,7 +485,7 @@ static QChar *createFontFile(const QString &faceName)
         const int nameLength = qMin(faceName.length(), LF_FACESIZE - 1);
         faceNamePtr = new QChar[nameLength + 1];
         memcpy(static_cast<void *>(faceNamePtr), faceName.utf16(), sizeof(wchar_t) * nameLength);
-        faceNamePtr[nameLength] = 0;
+        faceNamePtr[nameLength] = u'\0';
     }
     return faceNamePtr;
 }
diff --git a/src/gui/text/windows/qwindowsfontengine.cpp b/src/gui/text/windows/qwindowsfontengine.cpp
index 46774fa304..b2855d19dd 100644
--- a/src/gui/text/windows/qwindowsfontengine.cpp
+++ b/src/gui/text/windows/qwindowsfontengine.cpp
@@ -376,7 +376,7 @@ void QWindowsFontEngine::recalcAdvances(QGlyphLayout *glyphs, QFontEngine::Shape
                     oldFont = SelectObject(hdc, hfont);
 
                 if (!ttf) {
-                    QChar ch[2] = { ushort(glyph), 0 };
+                    QChar ch[2] = { ushort(glyph), u'\0' };
                     int chrLen = 1;
                     if (QChar::requiresSurrogates(glyph)) {
                         ch[0] = QChar::highSurrogate(glyph);
diff --git a/src/plugins/platforms/cocoa/qcocoakeymapper.mm b/src/plugins/platforms/cocoa/qcocoakeymapper.mm
index dca7b576f6..caa68ae694 100644
--- a/src/plugins/platforms/cocoa/qcocoakeymapper.mm
+++ b/src/plugins/platforms/cocoa/qcocoakeymapper.mm
@@ -121,7 +121,7 @@ static CarbonModifiers toCarbonModifiers(Qt::KeyboardModifiers qtModifiers)
 }
 
 // Keyboard keys (non-modifiers)
-static QHash<QChar, Qt::Key> standardKeys = {
+static QHash<char16_t, Qt::Key> standardKeys = {
     { kHomeCharCode, Qt::Key_Home },
     { kEnterCharCode, Qt::Key_Enter },
     { kEndCharCode, Qt::Key_End },
@@ -173,7 +173,7 @@ static QHash<QChar, Qt::Key> standardKeys = {
     { '^', Qt::Key_AsciiCircum }
 };
 
-static QHash<QChar, Qt::Key> virtualKeys = {
+static QHash<char16_t, Qt::Key> virtualKeys = {
     { kVK_F1, Qt::Key_F1 },
     { kVK_F2, Qt::Key_F2 },
     { kVK_F3, Qt::Key_F3 },
@@ -202,7 +202,7 @@ static QHash<QChar, Qt::Key> virtualKeys = {
     { kVK_PageDown, Qt::Key_PageDown }
 };
 
-static QHash<QChar, Qt::Key> functionKeys = {
+static QHash<char16_t, Qt::Key> functionKeys = {
     { NSUpArrowFunctionKey, Qt::Key_Up },
     { NSDownArrowFunctionKey, Qt::Key_Down },
     { NSLeftArrowFunctionKey, Qt::Key_Left },
@@ -237,7 +237,7 @@ static int toKeyCode(const QChar &key, int virtualKey, int modifiers)
     qCDebug(lcQpaKeyMapperKeys, "Mapping key: %d (0x%04x) / vk %d (0x%04x)",
         key.unicode(), key.unicode(), virtualKey, virtualKey);
 
-    if (key == kClearCharCode && virtualKey == 0x47)
+    if (key == QChar(kClearCharCode) && virtualKey == 0x47)
         return Qt::Key_Clear;
 
     if (key.isDigit()) {
@@ -254,7 +254,7 @@ static int toKeyCode(const QChar &key, int virtualKey, int modifiers)
         return key.unicode();
     }
 
-    if (auto qtKey = standardKeys.value(key)) {
+    if (auto qtKey = standardKeys.value(key.unicode())) {
         // To work like Qt for X11 we issue Backtab when Shift + Tab are pressed
         if (qtKey == Qt::Key_Tab && (modifiers & Qt::ShiftModifier)) {
             qCDebug(lcQpaKeyMapperKeys, "Got key: Qt::Key_Backtab");
@@ -272,11 +272,11 @@ static int toKeyCode(const QChar &key, int virtualKey, int modifiers)
     }
 
     // Check if they belong to key codes in private unicode range
-    if (key >= NSUpArrowFunctionKey && key <= NSModeSwitchFunctionKey) {
-        if (auto qtKey = functionKeys.value(key)) {
+    if (key >= QChar(NSUpArrowFunctionKey) && key <= QChar(NSModeSwitchFunctionKey)) {
+        if (auto qtKey = functionKeys.value(key.unicode())) {
             qCDebug(lcQpaKeyMapperKeys) << "Got" << qtKey;
             return qtKey;
-        } else if (key >= NSF1FunctionKey && key <= NSF35FunctionKey) {
+        } else if (key >= QChar(NSF1FunctionKey) && key <= QChar(NSF35FunctionKey)) {
             auto functionKey = Qt::Key_F1 + (key.unicode() - NSF1FunctionKey) ;
             qCDebug(lcQpaKeyMapperKeys) << "Got" << functionKey;
             return functionKey;
@@ -291,7 +291,7 @@ static int toKeyCode(const QChar &key, int virtualKey, int modifiers)
 
 static const int NSEscapeCharacter = 27; // not defined by Cocoa headers
 
-static const QHash<QChar, Qt::Key> cocoaKeys = {
+static const QHash<char16_t, Qt::Key> cocoaKeys = {
     { NSEnterCharacter, Qt::Key_Enter },
     { NSBackspaceCharacter, Qt::Key_Backspace },
     { NSTabCharacter, Qt::Key_Tab },
@@ -357,11 +357,11 @@ QChar QCocoaKeyMapper::toCocoaKey(Qt::Key key)
 {
     // Prioritize overloaded keys
     if (key == Qt::Key_Return)
-        return NSNewlineCharacter;
+        return QChar(NSNewlineCharacter);
     if (key == Qt::Key_Backspace)
-        return NSBackspaceCharacter;
+        return QChar(NSBackspaceCharacter);
 
-    static QHash<Qt::Key, QChar> reverseCocoaKeys;
+    static QHash<Qt::Key, char16_t> reverseCocoaKeys;
     if (reverseCocoaKeys.isEmpty()) {
         reverseCocoaKeys.reserve(cocoaKeys.size());
         for (auto it = cocoaKeys.begin(); it != cocoaKeys.end(); ++it)
@@ -373,7 +373,7 @@ QChar QCocoaKeyMapper::toCocoaKey(Qt::Key key)
 
 Qt::Key QCocoaKeyMapper::fromCocoaKey(QChar keyCode)
 {
-    if (auto key = cocoaKeys.value(keyCode))
+    if (auto key = cocoaKeys.value(keyCode.unicode()))
         return key;
 
     return Qt::Key(keyCode.toUpper().unicode());
diff --git a/src/plugins/platforms/cocoa/qcocoamenuitem.mm b/src/plugins/platforms/cocoa/qcocoamenuitem.mm
index 511c72988a..4806b244c3 100644
--- a/src/plugins/platforms/cocoa/qcocoamenuitem.mm
+++ b/src/plugins/platforms/cocoa/qcocoamenuitem.mm
@@ -362,7 +362,7 @@ NSMenuItem *QCocoaMenuItem::sync()
         // Similar to qt_mac_removePrivateUnicode change the delete key,
         // so the symbol is correctly seen in native menu bar.
         if (cocoaKey.unicode() == NSDeleteFunctionKey)
-            cocoaKey = NSDeleteCharacter;
+            cocoaKey = QChar(NSDeleteCharacter);
 
         m_native.keyEquivalent = QStringView(&cocoaKey, 1).toNSString();
         m_native.keyEquivalentModifierMask = QCocoaKeyMapper::toCocoaModifiers(modifiers);
diff --git a/src/plugins/styles/mac/qmacstyle_mac.mm b/src/plugins/styles/mac/qmacstyle_mac.mm
index 34568ee44a..8c3c6f4413 100644
--- a/src/plugins/styles/mac/qmacstyle_mac.mm
+++ b/src/plugins/styles/mac/qmacstyle_mac.mm
@@ -676,7 +676,7 @@ static inline bool isTreeView(const QWidget *widget)
 
 static QString qt_mac_removeMnemonics(const QString &original)
 {
-    QString returnText(original.size(), 0);
+    QString returnText(original.size(), QChar(0));
     int finalDest = 0;
     int currPos = 0;
     int l = original.length();