Q{Shared,Weak}Pointer: Reduce overload sets in implicit conversions

Only allow implicit conversions when the types involved are compatible.
That means, only allow construction and copy assignment when the type
X* is convertible to type T*. This is done using SFINAE and the
std::is_convertible type trait, which makes the previous
QSHAREDPOINTER_VERIFY_AUTO_CAST obsolete.

This patch fixes compilation when a function is overloaded with
Q{Shared,Weak}Pointer of different, incompatible types. Previously, this
resulted in a compilation error due to an ambiguous overload.

Change-Id: I069d22f3582e69842f14284d4f27827326597ca2
Fixes: QTBUG-75222
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>

src/corelib/tools/qsharedpointer_impl.h

@@ -69,21 +69,6 @@ QT_END_NAMESPACE
 
 QT_BEGIN_NAMESPACE
 
-
-// Macro QSHAREDPOINTER_VERIFY_AUTO_CAST
-//  generates a compiler error if the following construct isn't valid:
-//    T *ptr1;
-//    X *ptr2 = ptr1;
-//
-#ifdef QT_NO_DEBUG
-# define QSHAREDPOINTER_VERIFY_AUTO_CAST(T, X)          qt_noop()
-#else
-
-template<typename T> inline void qt_sharedpointer_cast_check(T *) { }
-# define QSHAREDPOINTER_VERIFY_AUTO_CAST(T, X)          \
-    qt_sharedpointer_cast_check<T>(static_cast<X *>(0))
-#endif
-
 //
 // forward declarations
 //
@@ -293,6 +278,9 @@ template <class T> class QSharedPointer
 {
     typedef T *QSharedPointer:: *RestrictedBool;
     typedef QtSharedPointer::ExternalRefCountData Data;
+    template <typename X>
+    using IfCompatible = typename std::enable_if<std::is_convertible<X*, T*>::value, bool>::type;
+
 public:
     typedef T Type;
     typedef T element_type;
@@ -316,11 +304,11 @@ public:
 
     Q_DECL_CONSTEXPR QSharedPointer(std::nullptr_t) Q_DECL_NOTHROW : value(nullptr), d(nullptr) { }
 
-    template <class X>
+    template <class X, IfCompatible<X> = true>
     inline explicit QSharedPointer(X *ptr) : value(ptr) // noexcept
     { internalConstruct(ptr, QtSharedPointer::NormalDeleter()); }
 
-    template <class X, typename Deleter>
+    template <class X, typename Deleter, IfCompatible<X> = true>
     inline QSharedPointer(X *ptr, Deleter deleter) : value(ptr) // throws
     { internalConstruct(ptr, deleter); }
 
@@ -349,7 +337,7 @@ public:
         return *this;
     }
 
-    template <class X>
+    template <class X, IfCompatible<X> = true>
     QSharedPointer(QSharedPointer<X> &&other) Q_DECL_NOTHROW
         : value(other.value), d(other.d)
     {
@@ -357,7 +345,7 @@ public:
         other.value = nullptr;
     }
 
-    template <class X>
+    template <class X, IfCompatible<X> = true>
     QSharedPointer &operator=(QSharedPointer<X> &&other) Q_DECL_NOTHROW
     {
         QSharedPointer moved(std::move(other));
@@ -367,11 +355,11 @@ public:
 
 #endif
 
-    template <class X>
+    template <class X, IfCompatible<X> = true>
     QSharedPointer(const QSharedPointer<X> &other) Q_DECL_NOTHROW : value(other.value), d(other.d)
     { if (d) ref(); }
 
-    template <class X>
+    template <class X, IfCompatible<X> = true>
     inline QSharedPointer &operator=(const QSharedPointer<X> &other)
     {
         QSharedPointer copy(other);
@@ -379,11 +367,11 @@ public:
         return *this;
     }
 
-    template <class X>
+    template <class X, IfCompatible<X> = true>
     inline QSharedPointer(const QWeakPointer<X> &other) : value(nullptr), d(nullptr)
     { *this = other; }
 
-    template <class X>
+    template <class X, IfCompatible<X> = true>
     inline QSharedPointer<T> &operator=(const QWeakPointer<X> &other)
     { internalSet(other.d, other.value); return *this; }
 
@@ -553,6 +541,8 @@ class QWeakPointer
 {
     typedef T *QWeakPointer:: *RestrictedBool;
     typedef QtSharedPointer::ExternalRefCountData Data;
+    template <typename X>
+    using IfCompatible = typename std::enable_if<std::is_convertible<X*, T*>::value, bool>::type;
 
 public:
     typedef T element_type;
@@ -574,14 +564,14 @@ public:
 #ifndef QT_NO_QOBJECT
     // special constructor that is enabled only if X derives from QObject
 #if QT_DEPRECATED_SINCE(5, 0)
-    template <class X>
+    template <class X, IfCompatible<X> = true>
     QT_DEPRECATED inline QWeakPointer(X *ptr) : d(ptr ? Data::getAndRef(ptr) : nullptr), value(ptr)
     { }
 #endif
 #endif
 
 #if QT_DEPRECATED_SINCE(5, 0)
-    template <class X>
+    template <class X, IfCompatible<X> = true>
     QT_DEPRECATED inline QWeakPointer &operator=(X *ptr)
     { return *this = QWeakPointer(ptr); }
 #endif
@@ -619,11 +609,11 @@ public:
         return *this;
     }
 
-    template <class X>
+    template <class X, IfCompatible<X> = true>
     inline QWeakPointer(const QWeakPointer<X> &o) : d(nullptr), value(nullptr)
     { *this = o; }
 
-    template <class X>
+    template <class X, IfCompatible<X> = true>
     inline QWeakPointer &operator=(const QWeakPointer<X> &o)
     {
         // conversion between X and T could require access to the virtual table
@@ -640,14 +630,13 @@ public:
     bool operator!=(const QWeakPointer<X> &o) const Q_DECL_NOTHROW
     { return !(*this == o); }
 
-    template <class X>
+    template <class X, IfCompatible<X> = true>
     inline QWeakPointer(const QSharedPointer<X> &o) : d(nullptr), value(nullptr)
     { *this = o; }
 
-    template <class X>
+    template <class X, IfCompatible<X> = true>
     inline QWeakPointer &operator=(const QSharedPointer<X> &o)
     {
-        QSHAREDPOINTER_VERIFY_AUTO_CAST(T, X); // if you get an error in this line, the cast is invalid
         internalSet(o.d, o.data());
         return *this;
     }
@@ -684,7 +673,7 @@ public:
     { return *this = QWeakPointer<X>(ptr, true); }
 
 #ifndef QT_NO_QOBJECT
-    template <class X>
+    template <class X, IfCompatible<X> = true>
     inline QWeakPointer(X *ptr, bool) : d(ptr ? Data::getAndRef(ptr) : nullptr), value(ptr)
     { }
 #endif

tests/auto/corelib/tools/qsharedpointer/tst_qsharedpointer.cpp

@@ -40,6 +40,7 @@
 #include "nontracked.h"
 #include "wrapper.h"
 
+#include <array>
 #include <memory>
 #include <stdlib.h>
 #include <time.h>
@@ -105,12 +106,15 @@ private slots:
     void sharedFromThis();
 
     void constructorThrow();
+    void overloads();
 
     void threadStressTest_data();
     void threadStressTest();
     void validConstructs();
     void invalidConstructs_data();
     void invalidConstructs();
+
+
     // let invalidConstructs be the last test, because it's the slowest;
     // add new tests above this block
 public slots:
@@ -2250,6 +2254,11 @@ void tst_QSharedPointer::invalidConstructs_data()
         << &QTest::QExternalTest::tryCompileFail
         << "QSharedPointer<Data> ptr(new Data, [](int *) {});\n";
 #endif
+
+    QTest::newRow("incompatible-overload")
+        << &QTest::QExternalTest::tryCompileFail
+        << "void foo(QSharedPointer<DerivedData>) {}\n"
+           "void bar() { foo(QSharedPointer<Data>()); }\n";
 }
 
 void tst_QSharedPointer::invalidConstructs()
@@ -2748,5 +2757,50 @@ void tst_QSharedPointer::reentrancyWhileDestructing()
     ReentrancyWhileDestructing::A obj;
 }
 
+namespace {
+struct Base1 {};
+struct Base2 {};
+
+struct Child1 : Base1 {};
+struct Child2 : Base2 {};
+
+template<template<typename> class SmartPtr>
+struct Overloaded
+{
+    std::array<int, 1> call(const SmartPtr<const Base1> &)
+    {
+        return {};
+    }
+    std::array<int, 2> call(const SmartPtr<const Base2> &)
+    {
+        return {};
+    }
+    static const Q_CONSTEXPR uint base1Called = sizeof(std::array<int, 1>);
+    static const Q_CONSTEXPR uint base2Called = sizeof(std::array<int, 2>);
+
+    void test()
+    {
+#define QVERIFY_CALLS(expr, base) Q_STATIC_ASSERT(sizeof(call(expr)) == base##Called)
+        QVERIFY_CALLS(SmartPtr<Base1>{}, base1);
+        QVERIFY_CALLS(SmartPtr<Base2>{}, base2);
+        QVERIFY_CALLS(SmartPtr<const Base1>{}, base1);
+        QVERIFY_CALLS(SmartPtr<const Base2>{}, base2);
+        QVERIFY_CALLS(SmartPtr<Child1>{}, base1);
+        QVERIFY_CALLS(SmartPtr<Child2>{}, base2);
+        QVERIFY_CALLS(SmartPtr<const Child1>{}, base1);
+        QVERIFY_CALLS(SmartPtr<const Child2>{}, base2);
+#undef QVERIFY_CALLS
+    }
+};
+}
+
+void tst_QSharedPointer::overloads()
+{
+    Overloaded<QSharedPointer> sharedOverloaded;
+    sharedOverloaded.test();
+    Overloaded<QWeakPointer> weakOverloaded;
+    weakOverloaded.test();
+}
+
 QTEST_MAIN(tst_QSharedPointer)
 #include "tst_qsharedpointer.moc"