Improve manual implementations of up- and downcasting (#1204)

* cxx-qt: use qobject_cast for downcastPtr

* cxx-qt-lib: implement Upcast<QVector<QPoint>> for QPolygon

* cxx-qt-lib: implement Upcast<QVector<QPointF>> for QPolygonF

* cxx-qt-lib: implement Deref for QQmlApplicationEngine

* cxx-qt-lib: remove null-pointer dereference from QStringList's downcast

* cxx-qt-lib: implement Upcast<QCoreApplication> for QGuiApplication

* cxx-qt-lib: implement Upcast<QGuiApplication> for QApplication

* Revert "cxx-qt-lib: implement Upcast<QGuiApplication> for QApplication"

This reverts commit 311ddd9.

* Revert "cxx-qt-lib: implement Upcast<QCoreApplication> for QGuiApplication"

This reverts commit 8e829ef.

* Revert "cxx-qt-lib: implement Deref for QQmlApplicationEngine"

This reverts commit b939a8d.

* cxx-qt: use qobject_cast for downcastPtr

* cxx-qt-lib: implement Upcast<QVector<QPoint>> for QPolygon

* cxx-qt-lib: implement Upcast<QVector<QPointF>> for QPolygonF

* cxx-qt-lib: remove null-pointer dereference from QStringList's downcast

* Improve downcast C++ implementations

Add detection whether to use qobject_cast or dynamic_cast via template
specialization.

Also add additional checks that a static_cast downcast is sane to do
(i.e. at least the size of the types must be the same).

---------

Co-authored-by: Leon Matthes <[email protected]>

Author: jnbooth

crates/cxx-qt-lib/include/gui/qpolygon.h

@@ -21,14 +21,3 @@ struct IsRelocatable<QPolygon> : ::std::true_type
 {};
 
 } // namespace rust
-
-namespace rust {
-namespace cxxqtlib1 {
-
-const QVector<QPoint>&
-qpolygonAsQVectorQPointRef(const QPolygon& shape);
-QVector<QPoint>&
-qpolygonAsQVectorQPointRef(QPolygon& shape);
-
-}
-}

crates/cxx-qt-lib/include/gui/qpolygonf.h

@@ -21,14 +21,3 @@ struct IsRelocatable<QPolygonF> : ::std::true_type
 {};
 
 } // namespace rust
-
-namespace rust {
-namespace cxxqtlib1 {
-
-const QVector<QPointF>&
-qpolygonfAsQVectorQPointFRef(const QPolygonF& shape);
-QVector<QPointF>&
-qpolygonfAsQVectorQPointFRef(QPolygonF& shape);
-
-}
-}

crates/cxx-qt-lib/src/core/qstringlist.rs

@@ -2,7 +2,6 @@
 // SPDX-FileContributor: Andrew Hayzen <[email protected]>
 //
 // SPDX-License-Identifier: MIT OR Apache-2.0
-use crate::core::qstringlist::ffi::QList_QString;
 use crate::{QList, QString};
 use core::mem::MaybeUninit;
 use cxx::{type_id, ExternType};
@@ -28,21 +27,6 @@ mod ffi {
         include!("cxx-qt-lib/qstringlist.h");
         type QStringList = super::QStringList;
 
-        include!("cxx-qt/casting.h");
-
-        #[doc(hidden)]
-        #[rust_name = "upcast_qstringlist"]
-        #[cxx_name = "upcastPtr"]
-        #[namespace = "rust::cxxqt1"]
-        unsafe fn upcast(thiz: *const QStringList) -> *const QList_QString;
-
-        #[doc(hidden)]
-        #[rust_name = "downcast_qlist_qstring"]
-        #[cxx_name = "downcastPtr"]
-        #[namespace = "rust::cxxqt1"]
-        #[cfg(cxxqt_qt_version_at_least_6)]
-        unsafe fn downcast(base: *const QList_QString) -> *const QStringList;
-
         /// Returns true if the list contains the string str; otherwise returns false.
         fn contains(self: &QStringList, str: &QString, cs: CaseSensitivity) -> bool;
 
@@ -67,6 +51,19 @@ mod ffi {
         ) -> &mut QStringList;
     }
 
+    #[namespace = "rust::cxxqt1"]
+    unsafe extern "C++" {
+        include!("cxx-qt/casting.h");
+
+        #[doc(hidden)]
+        #[rust_name = "upcast_qstringlist"]
+        unsafe fn upcastPtr(thiz: *const QStringList) -> *const QList_QString;
+
+        #[doc(hidden)]
+        #[rust_name = "downcast_qlist_qstring"]
+        unsafe fn downcastPtrStatic(base: *const QList_QString) -> *const QStringList;
+    }
+
     #[namespace = "rust::cxxqtlib1"]
     unsafe extern "C++" {
         include!("cxx-qt-lib/common.h");
@@ -207,20 +204,14 @@ impl DerefMut for QStringList {
     }
 }
 
-impl Upcast<QList_QString> for QStringList {
-    unsafe fn upcast_ptr(this: *const Self) -> *const QList_QString {
+impl Upcast<QList<QString>> for QStringList {
+    unsafe fn upcast_ptr(this: *const Self) -> *const QList<QString> {
         ffi::upcast_qstringlist(this)
     }
 
-    #[cfg(cxxqt_qt_version_at_least_6)]
-    unsafe fn from_base_ptr(base: *const QList_QString) -> *const Self {
+    unsafe fn from_base_ptr(base: *const QList<QString>) -> *const Self {
         ffi::downcast_qlist_qstring(base)
     }
-
-    #[cfg(cxxqt_qt_version_major = "5")]
-    unsafe fn from_base_ptr(_base: *const QList_QString) -> *const Self {
-        std::ptr::null()
-    }
 }
 
 // Safety:

crates/cxx-qt-lib/src/gui/qpolygon.cpp

@@ -32,20 +32,3 @@ static_assert(!::std::is_trivially_copy_constructible<QPolygon>::value);
 static_assert(!::std::is_trivially_destructible<QPolygon>::value);
 
 static_assert(QTypeInfo<QPolygon>::isRelocatable);
-
-namespace rust {
-namespace cxxqtlib1 {
-const QVector<QPoint>&
-qpolygonAsQVectorQPointRef(const QPolygon& shape)
-{
-  return static_cast<const QVector<QPoint>&>(shape);
-}
-
-QVector<QPoint>&
-qpolygonAsQVectorQPointRef(QPolygon& shape)
-{
-  return static_cast<QVector<QPoint>&>(shape);
-}
-
-}
-}

crates/cxx-qt-lib/src/gui/qpolygon.rs

@@ -5,6 +5,7 @@
 use crate::{QPoint, QRect, QVector};
 use core::mem::MaybeUninit;
 use cxx::{type_id, ExternType};
+use cxx_qt::Upcast;
 use std::fmt;
 use std::ops::{Deref, DerefMut};
 
@@ -74,6 +75,19 @@ mod ffi {
         fn united(self: &QPolygon, r: &QPolygon) -> QPolygon;
     }
 
+    #[namespace = "rust::cxxqt1"]
+    unsafe extern "C++" {
+        include!("cxx-qt/casting.h");
+
+        #[doc(hidden)]
+        #[rust_name = "upcast_qpolygon"]
+        unsafe fn upcastPtr(thiz: *const QPolygon) -> *const QVector_QPoint;
+
+        #[doc(hidden)]
+        #[rust_name = "downcast_qvector_qpoint"]
+        unsafe fn downcastPtrStatic(base: *const QVector_QPoint) -> *const QPolygon;
+    }
+
     #[namespace = "rust::cxxqtlib1"]
     unsafe extern "C++" {
         include!("cxx-qt-lib/common.h");
@@ -102,15 +116,6 @@ mod ffi {
         #[rust_name = "qpolygon_to_debug_qstring"]
         fn toDebugQString(value: &QPolygon) -> QString;
     }
-
-    #[namespace = "rust::cxxqtlib1"]
-    unsafe extern "C++" {
-        #[doc(hidden)]
-        #[rust_name = "qpolygon_as_qvector_qpoint_ref"]
-        fn qpolygonAsQVectorQPointRef(shape: &QPolygon) -> &QVector_QPoint;
-        #[rust_name = "qpolygon_as_qvector_qpoint_ref_mut"]
-        fn qpolygonAsQVectorQPointRef(shape: &mut QPolygon) -> &mut QVector_QPoint;
-    }
 }
 
 /// The QPolygon class provides a list of QPoint.
@@ -178,13 +183,23 @@ impl Deref for QPolygon {
     type Target = QVector<QPoint>;
 
     fn deref(&self) -> &Self::Target {
-        ffi::qpolygon_as_qvector_qpoint_ref(self)
+        self.upcast()
     }
 }
 
 impl DerefMut for QPolygon {
     fn deref_mut(&mut self) -> &mut Self::Target {
-        ffi::qpolygon_as_qvector_qpoint_ref_mut(self)
+        self.upcast_mut()
+    }
+}
+
+impl Upcast<QVector<QPoint>> for QPolygon {
+    unsafe fn upcast_ptr(this: *const Self) -> *const QVector<QPoint> {
+        ffi::upcast_qpolygon(this)
+    }
+
+    unsafe fn from_base_ptr(base: *const QVector<QPoint>) -> *const Self {
+        ffi::downcast_qvector_qpoint(base)
     }
 }
 

crates/cxx-qt-lib/src/gui/qpolygonf.cpp

@@ -32,20 +32,3 @@ static_assert(!::std::is_trivially_copy_constructible<QPolygonF>::value);
 static_assert(!::std::is_trivially_destructible<QPolygonF>::value);
 
 static_assert(QTypeInfo<QPolygonF>::isRelocatable);
-
-namespace rust {
-namespace cxxqtlib1 {
-const QVector<QPointF>&
-qpolygonfAsQVectorQPointFRef(const QPolygonF& shape)
-{
-  return static_cast<const QVector<QPointF>&>(shape);
-}
-
-QVector<QPointF>&
-qpolygonfAsQVectorQPointFRef(QPolygonF& shape)
-{
-  return static_cast<QVector<QPointF>&>(shape);
-}
-
-}
-}

crates/cxx-qt-lib/src/gui/qpolygonf.rs

@@ -4,6 +4,7 @@
 // SPDX-License-Identifier: MIT OR Apache-2.0
 use core::mem::MaybeUninit;
 use cxx::{type_id, ExternType};
+use cxx_qt::Upcast;
 use std::fmt;
 use std::ops::{Deref, DerefMut};
 
@@ -69,6 +70,19 @@ mod ffi {
         fn united(self: &QPolygonF, r: &QPolygonF) -> QPolygonF;
     }
 
+    #[namespace = "rust::cxxqt1"]
+    unsafe extern "C++" {
+        include!("cxx-qt/casting.h");
+
+        #[doc(hidden)]
+        #[rust_name = "upcast_qpolygonf"]
+        unsafe fn upcastPtr(thiz: *const QPolygonF) -> *const QVector_QPointF;
+
+        #[doc(hidden)]
+        #[rust_name = "downcast_qvector_qpointf"]
+        unsafe fn downcastPtrStatic(base: *const QVector_QPointF) -> *const QPolygonF;
+    }
+
     #[namespace = "rust::cxxqtlib1"]
     unsafe extern "C++" {
         include!("cxx-qt-lib/common.h");
@@ -93,15 +107,6 @@ mod ffi {
         #[rust_name = "qpolygonf_to_debug_qstring"]
         fn toDebugQString(value: &QPolygonF) -> QString;
     }
-
-    #[namespace = "rust::cxxqtlib1"]
-    unsafe extern "C++" {
-        #[doc(hidden)]
-        #[rust_name = "qpolygonf_as_qvector_qpointf_ref"]
-        fn qpolygonfAsQVectorQPointFRef(shape: &QPolygonF) -> &QVector_QPointF;
-        #[rust_name = "qpolygonf_as_qvector_qpointf_ref_mut"]
-        fn qpolygonfAsQVectorQPointFRef(shape: &mut QPolygonF) -> &mut QVector_QPointF;
-    }
 }
 
 /// The QPolygonF class provides a list of QPointF.
@@ -160,13 +165,23 @@ impl Deref for QPolygonF {
     type Target = QVector<QPointF>;
 
     fn deref(&self) -> &Self::Target {
-        ffi::qpolygonf_as_qvector_qpointf_ref(self)
+        self.upcast()
     }
 }
 
 impl DerefMut for QPolygonF {
     fn deref_mut(&mut self) -> &mut Self::Target {
-        ffi::qpolygonf_as_qvector_qpointf_ref_mut(self)
+        self.upcast_mut()
+    }
+}
+
+impl Upcast<QVector<QPointF>> for QPolygonF {
+    unsafe fn upcast_ptr(this: *const Self) -> *const QVector<QPointF> {
+        ffi::upcast_qpolygonf(this)
+    }
+
+    unsafe fn from_base_ptr(base: *const QVector<QPointF>) -> *const Self {
+        ffi::downcast_qvector_qpointf(base)
     }
 }
 

crates/cxx-qt/include/casting.h

@@ -5,6 +5,7 @@
 //
 // SPDX-License-Identifier: MIT OR Apache-2.0
 #pragma once
+#include <QtCore/QObject>
 #include <type_traits>
 
 namespace rust::cxxqt1 {
@@ -17,12 +18,44 @@ upcastPtr(const Sub* sub)
   return static_cast<const Base*>(sub);
 }
 
-template<typename Sub, typename Base>
+// Main downcasting function, for non-QObject types
+template<typename Sub,
+         typename Base,
+         std::enable_if_t<!std::is_base_of_v<QObject, Sub>, bool> = true>
 const Sub*
 downcastPtr(const Base* base)
 {
   static_assert(std::is_base_of_v<Base, Sub>);
+  static_assert(std::is_polymorphic_v<Sub>,
+                "Downcasting requires a polymorphic type (e.g. a type with at "
+                "least one virtual method)!");
   return dynamic_cast<const Sub*>(base);
 }
 
+// Downcasting function for QObject types, enabled via SFINAE
+template<typename Sub,
+         typename Base,
+         std::enable_if_t<std::is_base_of_v<QObject, Sub>, bool> = true>
+const Sub*
+downcastPtr(const Base* base)
+{
+  static_assert(std::is_base_of_v<Base, Sub>);
+  return qobject_cast<const Sub*>(base);
+}
+
+// Warning: This function is highly unsafe, use with caution!
+// It is only safe to use if you are 100% sure that the Sub and Base types are
+// of the same size and that all Base instances are also valid Sub instances!
+//
+// This is mostly the case for types like QStringList, that are just an
+// extension of their parent types and don't add any data members.
+template<typename Sub, typename Base>
+const Sub*
+downcastPtrStatic(const Base* base)
+{
+  static_assert(std::is_base_of_v<Base, Sub>);
+  static_assert(sizeof(Base) == sizeof(Sub));
+  return static_cast<const Sub*>(base);
+}
+
 }

crates/cxx-qt/src/lib.rs

@@ -236,7 +236,7 @@ pub trait Upcast<T> {
     /// Automatically available for types in RustQt blocks in [cxx_qt::bridge](bridge)s.
     /// Downcasts a pointer to base class `T` to a pointer to `Self`.
     /// Return a null pointer if `Self` is not actually a child of base.
-    /// > Note: Internal implementation uses `dynamic_cast`.
+    /// > Note: Internal implementation uses `qobject_cast`.
     unsafe fn from_base_ptr(base: *const T) -> *const Self;
 
     /// Upcast a reference to self to a reference to the base class