From 621c05e3b1869a524c1617fe9d1d178af3f7f227 Mon Sep 17 00:00:00 2001 From: Lars Knoll Date: Mon, 9 Nov 2020 17:27:04 +0100 Subject: Don't allow storing types that throw in the destructor in our containers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Types that throw in their destructors are strongly discouraged in C++, and even the STL doesn't define what happens if such types are stored in their containers. Make this more explicit for Qt and disallow storing those types in our containers. This will hopefully preempty any potential future bug reports about us not handling such a case. It also helps simplify some code in QList and other cases and makes it possible to explicitly mark more methods as noexcept. Some care needs to be taken where to add the static asserts, so that we don't disallow forward declarations of types stored in containers. Place the static assert into the destructor of the container where possible or otherwise into the templated d-pointer. Change-Id: If3aa40888f668d0f1b6c6b3ad4862b169d31280e Reviewed-by: Thiago Macieira Reviewed-by: Andrei Golubev Reviewed-by: MÃ¥rten Nordheim --- src/corelib/tools/qarraydataops.h | 6 ++++++ src/corelib/tools/qcache.h | 11 +++++++++-- src/corelib/tools/qcontiguouscache.h | 2 ++ src/corelib/tools/qflatmap_p.h | 2 ++ src/corelib/tools/qhash.h | 6 ++++++ src/corelib/tools/qmap.h | 25 +++++++++++++++---------- src/corelib/tools/qvarlengtharray.h | 2 ++ 7 files changed, 42 insertions(+), 12 deletions(-) (limited to 'src') diff --git a/src/corelib/tools/qarraydataops.h b/src/corelib/tools/qarraydataops.h index 183b6b0410..6e16d9280c 100644 --- a/src/corelib/tools/qarraydataops.h +++ b/src/corelib/tools/qarraydataops.h @@ -223,6 +223,8 @@ template struct QPodArrayOps : public QArrayDataPointer { + static_assert (std::is_nothrow_destructible_v, "Types with throwing destructors are not supported in Qt containers."); + protected: typedef QTypedArrayData Data; @@ -462,6 +464,8 @@ template struct QGenericArrayOps : public QArrayDataPointer { + static_assert (std::is_nothrow_destructible_v, "Types with throwing destructors are not supported in Qt containers."); + protected: typedef QTypedArrayData Data; @@ -892,6 +896,8 @@ template struct QMovableArrayOps : QGenericArrayOps { + static_assert (std::is_nothrow_destructible_v, "Types with throwing destructors are not supported in Qt containers."); + protected: typedef QTypedArrayData Data; diff --git a/src/corelib/tools/qcache.h b/src/corelib/tools/qcache.h index 868b4a5854..4cfe4a4a48 100644 --- a/src/corelib/tools/qcache.h +++ b/src/corelib/tools/qcache.h @@ -193,8 +193,15 @@ class QCache public: inline explicit QCache(qsizetype maxCost = 100) noexcept : mx(maxCost) - {} - inline ~QCache() { clear(); } + { + } + inline ~QCache() + { + static_assert(std::is_nothrow_destructible_v, "Types with throwing destructors are not supported in Qt containers."); + static_assert(std::is_nothrow_destructible_v, "Types with throwing destructors are not supported in Qt containers."); + + clear(); + } inline qsizetype maxCost() const noexcept { return mx; } void setMaxCost(qsizetype m) noexcept(std::is_nothrow_destructible_v) diff --git a/src/corelib/tools/qcontiguouscache.h b/src/corelib/tools/qcontiguouscache.h index e4d493cc52..d830c31891 100644 --- a/src/corelib/tools/qcontiguouscache.h +++ b/src/corelib/tools/qcontiguouscache.h @@ -73,6 +73,8 @@ struct QContiguousCacheTypedData : public QContiguousCacheData template class QContiguousCache { + static_assert(std::is_nothrow_destructible_v, "Types with throwing destructors are not supported in Qt containers."); + typedef QContiguousCacheTypedData Data; Data *d; public: diff --git a/src/corelib/tools/qflatmap_p.h b/src/corelib/tools/qflatmap_p.h index bf0efb2543..1b3eaea01c 100644 --- a/src/corelib/tools/qflatmap_p.h +++ b/src/corelib/tools/qflatmap_p.h @@ -109,6 +109,8 @@ template, class KeyContainer class MappedContainer = QList> class QFlatMap : private QFlatMapValueCompare { + static_assert(std::is_nothrow_destructible_v, "Types with throwing destructors are not supported in Qt containers."); + using full_map_t = QFlatMap; template diff --git a/src/corelib/tools/qhash.h b/src/corelib/tools/qhash.h index ce8982416d..5dc88943fc 100644 --- a/src/corelib/tools/qhash.h +++ b/src/corelib/tools/qhash.h @@ -725,6 +725,9 @@ public: } ~QHash() { + static_assert(std::is_nothrow_destructible_v, "Types with throwing destructors are not supported in Qt containers."); + static_assert(std::is_nothrow_destructible_v, "Types with throwing destructors are not supported in Qt containers."); + if (d && !d->ref.deref()) delete d; } @@ -1191,6 +1194,9 @@ public: } ~QMultiHash() { + static_assert(std::is_nothrow_destructible_v, "Types with throwing destructors are not supported in Qt containers."); + static_assert(std::is_nothrow_destructible_v, "Types with throwing destructors are not supported in Qt containers."); + if (d && !d->ref.deref()) delete d; } diff --git a/src/corelib/tools/qmap.h b/src/corelib/tools/qmap.h index 7b59688fa4..e63eb85bca 100644 --- a/src/corelib/tools/qmap.h +++ b/src/corelib/tools/qmap.h @@ -68,6 +68,9 @@ public: using iterator = typename Map::iterator; using const_iterator = typename Map::const_iterator; + static_assert(std::is_nothrow_destructible_v, "Types with throwing destructors are not supported in Qt containers."); + static_assert(std::is_nothrow_destructible_v, "Types with throwing destructors are not supported in Qt containers."); + Map m; QMapData() = default; @@ -217,7 +220,8 @@ public: template class QMap { - using MapData = QMapData>; + using Map = std::map; + using MapData = QMapData; QtPrivate::QExplicitlySharedDataPointerV2 d; friend class QMultiMap; @@ -446,8 +450,8 @@ public: friend class QMap; friend class const_iterator; - typename MapData::Map::iterator i; - explicit iterator(typename MapData::Map::iterator it) : i(it) {} + typename Map::iterator i; + explicit iterator(typename Map::iterator it) : i(it) {} public: typedef std::bidirectional_iterator_tag iterator_category; typedef qptrdiff difference_type; @@ -491,8 +495,8 @@ public: class const_iterator { friend class QMap; - typename MapData::Map::const_iterator i; - explicit const_iterator(typename MapData::Map::const_iterator it) : i(it) {} + typename Map::const_iterator i; + explicit const_iterator(typename Map::const_iterator it) : i(it) {} public: typedef std::bidirectional_iterator_tag iterator_category; @@ -745,7 +749,8 @@ Q_DECLARE_MUTABLE_ASSOCIATIVE_ITERATOR(Map) template class QMultiMap { - using MapData = QMapData>; + using Map = std::multimap; + using MapData = QMapData; QtPrivate::QExplicitlySharedDataPointerV2 d; public: @@ -1062,8 +1067,8 @@ public: friend class QMultiMap; friend class const_iterator; - typename MapData::Map::iterator i; - explicit iterator(typename MapData::Map::iterator it) : i(it) {} + typename Map::iterator i; + explicit iterator(typename Map::iterator it) : i(it) {} public: typedef std::bidirectional_iterator_tag iterator_category; typedef qptrdiff difference_type; @@ -1107,8 +1112,8 @@ public: class const_iterator { friend class QMultiMap; - typename MapData::Map::const_iterator i; - explicit const_iterator(typename MapData::Map::const_iterator it) : i(it) {} + typename Map::const_iterator i; + explicit const_iterator(typename Map::const_iterator it) : i(it) {} public: typedef std::bidirectional_iterator_tag iterator_category; diff --git a/src/corelib/tools/qvarlengtharray.h b/src/corelib/tools/qvarlengtharray.h index b538cf292d..975d088a48 100644 --- a/src/corelib/tools/qvarlengtharray.h +++ b/src/corelib/tools/qvarlengtharray.h @@ -60,6 +60,8 @@ QT_BEGIN_NAMESPACE template class QVarLengthArray { + static_assert(std::is_nothrow_destructible_v, "Types with throwing destructors are not supported in Qt containers."); + public: QVarLengthArray() : QVarLengthArray(0) {} -- cgit v1.2.3