Restrict QVariant::isNull() behavior

isNull() would forward to the contained type and check that type's
isNull() method for some of the builtin types. Remove that behavior
and only return true in isNull(), if the variant is invalid, doesn't
contain data or contains a null pointer.

In addition, implement more consistent behavior when constructing
a QVariant using the internal API taking a copy from a void *.
isNull() should return true in both cases. This mainly changes behavior
for some corner cases and when using our internal API.

[ChangeLog][Important Behavior Changes] QVariant::isNull()
no longer returns true when the variant contains an object of some
type with an isNull() method, that returns true for the object;
QVariant::isNull() now only returns true when the variant contains
no object or a null pointer.

Change-Id: I3125041c4f8f8618a04aa375aa0a56b19c02dcf5
Reviewed-by: Fabian Kosmale <fabian.kosmale@qt.io>

5 files changed, 18 insertions, 180 deletions

diff --git a/src/corelib/kernel/qvariant.cpp b/src/corelib/kernel/qvariant.cpp
index 94a87297f0..d9fafca0b7 100644
--- a/src/corelib/kernel/qvariant.cpp
+++ b/src/corelib/kernel/qvariant.cpp
@@ -117,12 +117,6 @@ struct CoreTypesFilter {
 
 namespace { // annonymous used to hide QVariant handlers
 
-static bool isNull(const QVariant::Private *d)
-{
-    QVariantIsNull<CoreTypesFilter> isNull(d);
-    return QMetaTypeSwitcher::switcher<bool>(isNull, d->type().id());
-}
-
 /*!
   \internal
  */
@@ -1394,7 +1388,6 @@ static void streamDebug(QDebug dbg, const QVariant &v)
 #endif
 
 const QVariant::Handler qt_kernel_variant_handler = {
-    isNull,
     convert,
 #if !defined(QT_NO_DEBUG_STREAM)
     streamDebug
@@ -1403,13 +1396,11 @@ const QVariant::Handler qt_kernel_variant_handler = {
 #endif
 };
 
-static bool dummyIsNull(const QVariant::Private *d) { Q_ASSERT_X(false, "QVariant::isNull", "Trying to call isNull on an unknown type"); return d->is_null; }
 static bool dummyConvert(const QVariant::Private *, int, void *, bool *) { Q_ASSERT_X(false, "QVariant", "Trying to convert an unknown type"); return false; }
 #if !defined(QT_NO_DEBUG_STREAM)
 static void dummyStreamDebug(QDebug, const QVariant &) { Q_ASSERT_X(false, "QVariant", "Trying to convert an unknown type"); }
 #endif
 const QVariant::Handler qt_dummy_variant_handler = {
-    dummyIsNull,
     dummyConvert,
 #if !defined(QT_NO_DEBUG_STREAM)
     dummyStreamDebug
@@ -1445,7 +1436,9 @@ static void customConstruct(QVariant::Private *d, const void *copy)
         d->is_shared = true;
         d->data.shared = new (data) QVariant::PrivateShared(ptr);
     }
-    d->is_null = !copy;
+    // need to check for nullptr_t here, as this can get called by fromValue(nullptr). fromValue() uses
+    // std::addressof(value) which in this case returns the address of the nullptr object.
+    d->is_null = !copy || type == QMetaType::fromType<std::nullptr_t>();
 }
 
 static void customClear(QVariant::Private *d)
@@ -1459,17 +1452,6 @@ static void customClear(QVariant::Private *d)
     }
 }
 
-static bool customIsNull(const QVariant::Private *d)
-{
-    if (d->is_null)
-        return true;
-    if (d->type().flags() & QMetaType::IsPointer) {
-        const void *d_ptr = d->is_shared ? d->data.shared->ptr : &(d->data);
-        return *static_cast<void *const *>(d_ptr) == nullptr;
-    }
-    return false;
-}
-
 static bool customConvert(const QVariant::Private *d, int t, void *result, bool *ok)
 {
     if (d->type().id() >= QMetaType::User || t >= QMetaType::User) {
@@ -1496,7 +1478,6 @@ static void customStreamDebug(QDebug dbg, const QVariant &variant) {
 #endif
 
 const QVariant::Handler qt_custom_variant_handler = {
-    customIsNull,
     customConvert,
 #if !defined(QT_NO_DEBUG_STREAM)
     customStreamDebug
@@ -2061,7 +2042,6 @@ QVariant::QVariant(int typeId, const void *copy, uint flags)
     } else {
         create(typeId, copy);
     }
-    d.is_null = false;
 }
 
 /*!
@@ -2071,7 +2051,6 @@ QVariant::QVariant(int typeId, const void *copy, uint flags)
 QVariant::QVariant(QMetaType type, const void *copy) : d(type)
 {
     customConstruct(&d, copy);
-    d.is_null = false;
 }
 
 QVariant::QVariant(int val)
@@ -3972,25 +3951,32 @@ const void *QVariant::constData() const
 void* QVariant::data()
 {
     detach();
+    // set is_null to false, as the caller is likely to write some data into this variant
+    d.is_null = false;
     return const_cast<void *>(constData());
 }
 
 
 /*!
-    Returns \c true if this is a null variant, false otherwise. A variant is
-    considered null if it contains no initialized value, or the contained value
-    is \nullptr or is an instance of a built-in type that has an isNull
-    method, in which case the result would be the same as calling isNull on the
-    wrapped object.
+    Returns \c true if this is a null variant, false otherwise.
+
+    A variant is considered null if it contains no initialized value or a null pointer.
 
-    \warning Null variants is not a single state and two null variants may easily
-    return \c false on the == operator if they do not contain similar null values.
+    \note This behavior has been changed from Qt 5, where isNull() would also
+    return true if the variant contained an object of a builtin type with an isNull()
+    method that returned true for that object.
 
     \sa convert(int)
 */
 bool QVariant::isNull() const
 {
-    return handlerManager[d.type().id()]->isNull(&d);
+    if (d.is_null || !metaType().isValid())
+        return true;
+    if (metaType().flags() & QMetaType::IsPointer) {
+        const void *d_ptr = d.is_shared ? d.data.shared->ptr : &(d.data);
+        return *static_cast<void *const *>(d_ptr) == nullptr;
+    }
+    return false;
 }
 
 #ifndef QT_NO_DEBUG_STREAM
diff --git a/src/corelib/kernel/qvariant.h b/src/corelib/kernel/qvariant.h
index 498ba57399..4690ce1d07 100644
--- a/src/corelib/kernel/qvariant.h
+++ b/src/corelib/kernel/qvariant.h
@@ -469,11 +469,9 @@ class Q_CORE_EXPORT QVariant
         }
     };
  public:
-    typedef bool (*f_null)(const Private *);
     typedef bool (*f_convert)(const QVariant::Private *d, int t, void *, bool *);
     typedef void (*f_debugStream)(QDebug, const QVariant &);
     struct Handler {
-        f_null isNull;
         f_convert convert;
         f_debugStream debugStream;
     };
diff --git a/src/corelib/kernel/qvariant_p.h b/src/corelib/kernel/qvariant_p.h
index 2cec8ff65b..56fd715382 100644
--- a/src/corelib/kernel/qvariant_p.h
+++ b/src/corelib/kernel/qvariant_p.h
@@ -182,127 +182,8 @@ inline void v_clear(QVariant::Private *d, T* = nullptr)
 
 }
 
-template <typename T>
-struct PrimitiveIsNull
-{
-public:
-    static bool isNull(const QVariant::Private *d)
-    {
-        return d->is_null;
-    }
-};
-
-template <typename T>
-struct PrimitiveIsNull<T*>
-{
-public:
-    static bool isNull(const QVariant::Private *d)
-    {
-        return d->is_null || d->data.ptr == nullptr;
-    }
-};
-
-template <>
-struct PrimitiveIsNull<std::nullptr_t>
-{
-public:
-    static bool isNull(const QVariant::Private *)
-    {
-        return true;
-    }
-};
-
 Q_CORE_EXPORT const QVariant::Handler *qcoreVariantHandler();
 
-template<class Filter>
-class QVariantIsNull
-{
-    /// \internal
-    /// This class checks if a type T has method called isNull. Result is kept in the Value property
-    /// TODO Can we somehow generalize it? A macro version?
-    template<typename T>
-    class HasIsNullMethod {
-        struct Yes { char unused[1]; };
-        struct No { char unused[2]; };
-        static_assert(sizeof(Yes) != sizeof(No));
-
-        template<class C> static decltype(static_cast<const C*>(nullptr)->isNull(), Yes()) test(int);
-        template<class C> static No test(...);
-    public:
-        static const bool Value = (sizeof(test<T>(0)) == sizeof(Yes));
-    };
-
-    // TODO This part should go to autotests during HasIsNullMethod generalization.
-    static_assert(!HasIsNullMethod<bool>::Value);
-    struct SelfTest1 { bool isNull() const; };
-    static_assert(HasIsNullMethod<SelfTest1>::Value);
-    struct SelfTest2 {};
-    static_assert(!HasIsNullMethod<SelfTest2>::Value);
-    struct SelfTest3 : public SelfTest1 {};
-    static_assert(HasIsNullMethod<SelfTest3>::Value);
-    struct SelfTestFinal1 final { bool isNull() const; };
-    static_assert(HasIsNullMethod<SelfTestFinal1>::Value);
-    struct SelfTestFinal2 final {};
-    static_assert(!HasIsNullMethod<SelfTestFinal2>::Value);
-    struct SelfTestFinal3 final : public SelfTest1 {};
-    static_assert(HasIsNullMethod<SelfTestFinal3>::Value);
-
-    template<typename T, bool HasIsNull = HasIsNullMethod<T>::Value>
-    struct CallFilteredIsNull
-    {
-        static bool isNull(const QVariant::Private *d)
-        {
-            return v_cast<T>(d)->isNull();
-        }
-    };
-    template<typename T>
-    struct CallFilteredIsNull<T, /* HasIsNull = */ false>
-    {
-        static bool isNull(const QVariant::Private *d)
-        {
-            return PrimitiveIsNull<T>::isNull(d);
-        }
-    };
-
-    template<typename T, bool IsAcceptedType = Filter::template Acceptor<T>::IsAccepted>
-    struct CallIsNull
-    {
-        static bool isNull(const QVariant::Private *d)
-        {
-            return CallFilteredIsNull<T>::isNull(d);
-        }
-    };
-    template<typename T>
-    struct CallIsNull<T, /* IsAcceptedType = */ false>
-    {
-        static bool isNull(const QVariant::Private *d)
-        {
-            return CallFilteredIsNull<T, false>::isNull(d);
-        }
-    };
-
-public:
-    QVariantIsNull(const QVariant::Private *d)
-        : m_d(d)
-    {}
-    template<typename T>
-    bool delegate(const T*)
-    {
-        return CallIsNull<T>::isNull(m_d);
-    }
-    // we need that as sizof(void) is undefined and it is needed in HasIsNullMethod
-    bool delegate(const void *) { Q_ASSERT(false); return m_d->is_null; }
-    bool delegate(const QMetaTypeSwitcher::UnknownType *) { return m_d->is_null; }
-    bool delegate(const QMetaTypeSwitcher::NotBuiltinType *)
-    {
-        // QVariantIsNull is used only for built-in types
-        Q_ASSERT(false);
-        return m_d->is_null;
-    }
-protected:
-    const QVariant::Private *m_d;
-};
-
 namespace QVariantPrivate {
 Q_CORE_EXPORT void registerHandler(const int /* Modules::Names */ name, const QVariant::Handler *handler);
 }
diff --git a/src/gui/kernel/qguivariant.cpp b/src/gui/kernel/qguivariant.cpp
index efd98f691d..fbcf6494b4 100644
--- a/src/gui/kernel/qguivariant.cpp
+++ b/src/gui/kernel/qguivariant.cpp
@@ -101,26 +101,6 @@ struct GuiTypesFilter {
     };
 };
 
-// This class is a hack that customizes access to QPolygon and QPolygonF
-template<class Filter>
-class QGuiVariantIsNull : public QVariantIsNull<Filter> {
-    typedef QVariantIsNull<Filter> Base;
-public:
-    QGuiVariantIsNull(const QVariant::Private *d)
-        : QVariantIsNull<Filter>(d)
-    {}
-    template<typename T>
-    bool delegate(const T *p) { return Base::delegate(p); }
-    bool delegate(const QPolygon*) { return v_cast<QPolygon>(Base::m_d)->isEmpty(); }
-    bool delegate(const QPolygonF*) { return v_cast<QPolygonF>(Base::m_d)->isEmpty(); }
-    bool delegate(const void *p) { return Base::delegate(p); }
-};
-static bool isNull(const QVariant::Private *d)
-{
-    QGuiVariantIsNull<GuiTypesFilter> isNull(d);
-    return QMetaTypeSwitcher::switcher<bool>(isNull, d->type().id(), nullptr);
-}
-
 static bool convert(const QVariant::Private *d, int t,
                  void *result, bool *ok)
 {
@@ -263,7 +243,6 @@ static void streamDebug(QDebug dbg, const QVariant &v)
 #endif
 
 const QVariant::Handler qt_gui_variant_handler = {
-    isNull,
     convert,
 #if !defined(QT_NO_DEBUG_STREAM)
     streamDebug
diff --git a/src/widgets/kernel/qwidgetsvariant.cpp b/src/widgets/kernel/qwidgetsvariant.cpp
index 31bb92bb2e..0206ed8131 100644
--- a/src/widgets/kernel/qwidgetsvariant.cpp
+++ b/src/widgets/kernel/qwidgetsvariant.cpp
@@ -49,11 +49,6 @@ QT_BEGIN_NAMESPACE
 
 namespace {
 
-static bool isNull(const QVariant::Private *)
-{
-    return false;
-}
-
 static bool convert(const QVariant::Private *d, int type, void *result, bool *ok)
 {
     Q_UNUSED(d);
@@ -79,7 +74,6 @@ static void streamDebug(QDebug dbg, const QVariant &v)
 #endif
 
 static const QVariant::Handler widgets_handler = {
-    isNull,
     convert,
 #if !defined(QT_NO_DEBUG_STREAM)
     streamDebug