QPixmapCache: deprecate replace()

The replace() implementation overwrites the passed Key key with a new
version, const_cast'ing away the const from the key passed by
reference-to-const. This is UB if the Key was originally declared
const.

Deprecate the function.

Also inline the const_cast, so compilers can readily detect the UB
even if users don't enable deprecation warnings. Due to the severity
of the issue (UB), immediate deprecation is warranted. There appear to
be no in-tree user of the API outside of tst_qpixmapcache.cpp.

[ChangeLog][Deprecation Notice][QtGui][QPixmapCache] The `replace(key,
pixmap)` function has been deprecated, because passing a `const Key`
to it results in undefined behavior. Use `remove(key, pixmap)`
followed by `key = insert(pixmap)` instead.

Pick-to: 6.6
Change-Id: Ic5060ce3271f2a1b6dc561da8716b452a2355d4c
Reviewed-by: Volker Hilsheimer <volker.hilsheimer@qt.io>

src/gui/compat/removed_api.cpp

@@ -9,6 +9,8 @@ QT_USE_NAMESPACE
 
 #if QT_GUI_REMOVED_SINCE(6, 6)
 
+#include "qpixmapcache.h" // inlined API
+
 // #include "qotherheader.h"
 // // implement removed functions from qotherheader.h
 // order sections alphabetically

src/gui/image/qpixmapcache.cpp

@@ -184,7 +184,6 @@ public:
     void timerEvent(QTimerEvent *) override;
     bool insert(const QString& key, const QPixmap &pixmap, int cost);
     QPixmapCache::Key insert(const QPixmap &pixmap, int cost);
-    bool replace(const QPixmapCache::Key &key, const QPixmap &pixmap, int cost);
     bool remove(const QString &key);
     bool remove(const QPixmapCache::Key &key);
 
@@ -351,25 +350,6 @@ QPixmapCache::Key QPMCache::insert(const QPixmap &pixmap, int cost)
     return cacheKey;
 }
 
-bool QPMCache::replace(const QPixmapCache::Key &key, const QPixmap &pixmap, int cost)
-{
-    Q_ASSERT(key.isValid());
-    //If for the same key we had already an entry so we should delete the pixmap and use the new one
-    QCache<QPixmapCache::Key, QPixmapCacheEntry>::remove(key);
-
-    QPixmapCache::Key cacheKey = createKey();
-
-    bool success = QCache<QPixmapCache::Key, QPixmapCacheEntry>::insert(cacheKey, new QPixmapCacheEntry(cacheKey, pixmap), cost);
-    if (success) {
-        if (!theid) {
-            theid = startTimer(flush_time);
-            t = false;
-        }
-        const_cast<QPixmapCache::Key&>(key) = cacheKey;
-    }
-    return success;
-}
-
 bool QPMCache::remove(const QString &key)
 {
     auto cacheKey = cacheKeys.constFind(key);
@@ -553,24 +533,25 @@ QPixmapCache::Key QPixmapCache::insert(const QPixmap &pixmap)
     return pm_cache()->insert(pixmap, cost(pixmap));
 }
 
+#if QT_DEPRECATED_SINCE(6, 6)
 /*!
+    \fn bool QPixmapCache::replace(const Key &key, const QPixmap &pixmap)
+
+    \deprecated [6.6] Use \c{remove(key); key = insert(pixmap);} instead.
+
     Replaces the pixmap associated with the given \a key with the \a pixmap
     specified. Returns \c true if the \a pixmap has been correctly inserted into
     the cache; otherwise returns \c false.
 
+    The passed \a key is updated to reference \a pixmap now. Other copies of \a
+    key, if any, still refer to the old pixmap, which is, however, removed from
+    the cache by this function.
+
     \sa setCacheLimit(), insert()
 
     \since 4.6
 */
-bool QPixmapCache::replace(const Key &key, const QPixmap &pixmap)
-{
-    if (!qt_pixmapcache_thread_test())
-        return false;
-    //The key is not valid anymore, a flush happened before probably
-    if (!key.d || !key.d->isValid)
-        return false;
-    return pm_cache()->replace(key, pixmap, cost(pixmap));
-}
+#endif // QT_DEPRECATED_SINCE(6, 6)
 
 /*!
     Returns the cache limit (in kilobytes).

src/gui/image/qpixmapcache.h

@@ -42,13 +42,30 @@ public:
     static bool find(const Key &key, QPixmap *pixmap);
     static bool insert(const QString &key, const QPixmap &pixmap);
     static Key insert(const QPixmap &pixmap);
+#if QT_DEPRECATED_SINCE(6, 6)
+    QT_DEPRECATED_VERSION_X_6_6("Use remove(key), followed by key = insert(pixmap).")
+    QT_GUI_INLINE_SINCE(6, 6)
     static bool replace(const Key &key, const QPixmap &pixmap);
+#endif
     static void remove(const QString &key);
     static void remove(const Key &key);
     static void clear();
 };
 Q_DECLARE_SHARED(QPixmapCache::Key)
 
+#if QT_DEPRECATED_SINCE(6, 6)
+#if QT_GUI_INLINE_IMPL_SINCE(6, 6)
+bool QPixmapCache::replace(const Key &key, const QPixmap &pixmap)
+{
+    if (!key.isValid())
+        return false;
+    remove(key);
+    const_cast<Key&>(key) = insert(pixmap);
+    return key.isValid();
+}
+#endif // QT_GUI_INLINE_IMPL_SINCE(6, 6)
+#endif // QT_DEPRECATED_SINCE(6, 6)
+
 QT_END_NAMESPACE
 
 #endif // QPIXMAPCACHE_H

tests/auto/gui/image/qpixmapcache/tst_qpixmapcache.cpp

@@ -32,7 +32,9 @@ private slots:
     void find();
     void insert();
     void failedInsertReturnsInvalidKey();
+#if QT_DEPRECATED_SINCE(6, 6)
     void replace();
+#endif
     void remove();
     void clear();
     void pixmapKey();
@@ -113,11 +115,15 @@ void tst_QPixmapCache::setCacheLimit()
     QVERIFY(!QPixmapCache::find(key, p1));
 
     QPixmapCache::setCacheLimit(1000);
+#if QT_DEPRECATED_SINCE(6, 6)
+    QT_WARNING_PUSH
+    QT_WARNING_DISABLE_DEPRECATED
     p1 = new QPixmap(2, 3);
     QVERIFY(!QPixmapCache::replace(key, *p1));
     QVERIFY(!QPixmapCache::find(key, p1));
 
     delete p1;
+#endif // QT_DEPRECATED_SINCE(6, 6)
 
     //Let check if keys are released when the pixmap cache is
     //full or has been flushed.
@@ -310,6 +316,9 @@ void tst_QPixmapCache::failedInsertReturnsInvalidKey()
     QVERIFY(!success);       // QString API
 }
 
+#if QT_DEPRECATED_SINCE(6, 6)
+QT_WARNING_PUSH
+QT_WARNING_DISABLE_DEPRECATED
 void tst_QPixmapCache::replace()
 {
     //The int part of the API
@@ -338,6 +347,8 @@ void tst_QPixmapCache::replace()
     //Broken keys
     QCOMPARE(QPixmapCache::replace(QPixmapCache::Key(), p2), false);
 }
+QT_WARNING_POP
+#endif // QT_DEPRECATED_SINCE(6, 6)
 
 void tst_QPixmapCache::remove()
 {