Remove the full qualification prefix ::

This is conflicting with Qt5s Meta Object system. E.g.: In Qt5 only `QPoint` is a valid type for a Q_PROPERTY, `::QPoint` is not!
KDAB · Mar 1, 2024 · 74beaa7 · 74beaa7
1 parent a825c29
commit 74beaa7
Show file tree

Hide file tree

Showing 20 changed files with 391 additions and 375 deletions.
diff --git a/crates/cxx-qt-gen/src/generator/cpp/method.rs b/crates/cxx-qt-gen/src/generator/cpp/method.rs
@@ -411,15 +411,12 @@ mod tests {
         } else {
             panic!("Expected pair")
         };
-        assert_str_eq!(
-            header,
-            "Q_INVOKABLE ::B2 trivialInvokable(::A1 param) const;"
-        );
+        assert_str_eq!(header, "Q_INVOKABLE B2 trivialInvokable(A1 param) const;");
         assert_str_eq!(
             source,
             indoc! {r#"
-            ::B2
-            MyObject::trivialInvokable(::A1 param) const
+            B2
+            MyObject::trivialInvokable(A1 param) const
             {
                 const ::rust::cxxqt1::MaybeLockGuard<MyObject> guard(*this);
                 return trivialInvokableWrapper(param);
@@ -437,7 +434,7 @@ mod tests {
         };
         assert_str_eq!(
             header,
-            "::B2 trivialInvokableWrapper(::A1 param) const noexcept;"
+            "B2 trivialInvokableWrapper(A1 param) const noexcept;"
         );
     }
 }
diff --git a/crates/cxx-qt-gen/src/generator/cpp/signal.rs b/crates/cxx-qt-gen/src/generator/cpp/signal.rs
@@ -639,10 +639,10 @@ mod tests {
             header,
             indoc! {
             r#"
-            namespace rust::cxxqtgen1::mynamespace {
+            namespace mynamespace::rust::cxxqtgen1 {
             ::QMetaObject::Connection
-            ObjRust_signalCxxNameConnect(::mynamespace::ObjCpp& self, ::rust::cxxqtgen1::mynamespace::ObjRustCxxQtSignalHandlersignalCxxName closure, ::Qt::ConnectionType type);
-            } // namespace rust::cxxqtgen1::mynamespace
+            ObjRust_signalCxxNameConnect(mynamespace::ObjCpp& self, ::mynamespace::rust::cxxqtgen1::ObjRustCxxQtSignalHandlersignalCxxName closure, ::Qt::ConnectionType type);
+            } // namespace mynamespace::rust::cxxqtgen1
             "#}
         );
         assert_str_eq!(
@@ -652,7 +652,7 @@ mod tests {
             // https://gcc.gnu.org/bugzilla/show_bug.cgi?id=56480
             namespace rust::cxxqt1 {
             template <>
-            SignalHandler<::rust::cxxqtgen1::mynamespace::ObjRustCxxQtSignalParamssignalCxxName *>::~SignalHandler() noexcept
+            SignalHandler<::mynamespace::rust::cxxqtgen1::ObjRustCxxQtSignalParamssignalCxxName *>::~SignalHandler() noexcept
             {
                 if (data[0] == nullptr && data[1] == nullptr)
                 {
@@ -664,30 +664,30 @@ mod tests {
 
             template <>
             template <>
-            void SignalHandler<::rust::cxxqtgen1::mynamespace::ObjRustCxxQtSignalParamssignalCxxName *>::operator()<::mynamespace::ObjCpp&>(::mynamespace::ObjCpp& self)
+            void SignalHandler<::mynamespace::rust::cxxqtgen1::ObjRustCxxQtSignalParamssignalCxxName *>::operator()<mynamespace::ObjCpp&>(mynamespace::ObjCpp& self)
             {
                 call_ObjRust_signal_handler_signalCxxName(*this, self);
             }
 
-            static_assert(alignof(SignalHandler<::rust::cxxqtgen1::mynamespace::ObjRustCxxQtSignalParamssignalCxxName *>) <= alignof(::std::size_t), "unexpected aligment");
-            static_assert(sizeof(SignalHandler<::rust::cxxqtgen1::mynamespace::ObjRustCxxQtSignalParamssignalCxxName *>) == sizeof(::std::size_t[2]), "unexpected size");
+            static_assert(alignof(SignalHandler<::mynamespace::rust::cxxqtgen1::ObjRustCxxQtSignalParamssignalCxxName *>) <= alignof(::std::size_t), "unexpected aligment");
+            static_assert(sizeof(SignalHandler<::mynamespace::rust::cxxqtgen1::ObjRustCxxQtSignalParamssignalCxxName *>) == sizeof(::std::size_t[2]), "unexpected size");
             } // namespace rust::cxxqt1
 
-            namespace rust::cxxqtgen1::mynamespace {
+            namespace mynamespace::rust::cxxqtgen1 {
             ::QMetaObject::Connection
-            ObjRust_signalCxxNameConnect(::mynamespace::ObjCpp& self, ::rust::cxxqtgen1::mynamespace::ObjRustCxxQtSignalHandlersignalCxxName closure, ::Qt::ConnectionType type)
+            ObjRust_signalCxxNameConnect(mynamespace::ObjCpp& self, ::mynamespace::rust::cxxqtgen1::ObjRustCxxQtSignalHandlersignalCxxName closure, ::Qt::ConnectionType type)
             {
                 return ::QObject::connect(
                     &self,
-                    &::mynamespace::ObjCpp::signalCxxName,
+                    &mynamespace::ObjCpp::signalCxxName,
                     &self,
                     [&, closure = ::std::move(closure)]() mutable {
-                        const ::rust::cxxqt1::MaybeLockGuard<::mynamespace::ObjCpp> guard(self);
-                        closure.template operator()<::mynamespace::ObjCpp&>(self);
+                        const ::rust::cxxqt1::MaybeLockGuard<mynamespace::ObjCpp> guard(self);
+                        closure.template operator()<mynamespace::ObjCpp&>(self);
                     },
                     type);
             }
-            } // namespace rust::cxxqtgen1::mynamespace
+            } // namespace mynamespace::rust::cxxqtgen1
             "#}
         );
     }

diff --git a/crates/cxx-qt-gen/src/generator/naming/signals.rs b/crates/cxx-qt-gen/src/generator/naming/signals.rs
@@ -56,10 +56,22 @@ impl QSignalHelperName {
         let signal_ident = &idents.name.cpp;
         let handler_alias = format_ident!("{qobject_ident}CxxQtSignalHandler{signal_ident}");
         let namespace = {
-            let mut namespace = vec!["rust::cxxqtgen1".to_owned()];
-            if let Some(qobject_namespace) = type_names.namespace(qobject_ident) {
-                namespace.push(qobject_namespace);
-            }
+            // This namespace will take the form of:
+            // qobject_namespace::rust::cxxqtgen1
+            //
+            // We experimented with using rust::cxxqtgen1::qobject_namespace.
+            // However, this currently doesn't work, as we can't fully-qualify all C++ access.
+            // Therefore when refering to the QObject type (e.g. qobject_namespace::QObject),
+            // It would fail, as it would look up in this helper namespace, instead of the actual
+            // qobject_namespace.
+            //
+            // See the comment on TypeNames::cxx_qualified for why fully qualifying is
+            // unfortunately not possible.
+            let qobject_namespace = type_names.namespace(qobject_ident);
+            let namespace: Vec<_> = qobject_namespace
+                .into_iter()
+                .chain(vec!["rust::cxxqtgen1".to_owned()])
+                .collect();
 
             namespace.join("::")
         };

diff --git a/crates/cxx-qt-gen/src/naming/cpp.rs b/crates/cxx-qt-gen/src/naming/cpp.rs
@@ -373,15 +373,15 @@ mod tests {
         let ty = parse_quote! { A };
         let mut type_names = TypeNames::default();
         type_names.insert("A", None, Some("A1"), None);
-        assert_eq!(syn_type_to_cpp_type(&ty, &type_names).unwrap(), "::A1");
+        assert_eq!(syn_type_to_cpp_type(&ty, &type_names).unwrap(), "A1");
     }
 
     #[test]
     fn test_syn_type_to_cpp_type_mapped_with_namespace() {
         let ty = parse_quote! { A };
         let mut type_names = TypeNames::default();
         type_names.insert("A", None, Some("A1"), Some("N1"));
-        assert_eq!(syn_type_to_cpp_type(&ty, &type_names).unwrap(), "::N1::A1");
+        assert_eq!(syn_type_to_cpp_type(&ty, &type_names).unwrap(), "N1::A1");
     }
 
     #[test]

diff --git a/crates/cxx-qt-gen/src/naming/type_names.rs b/crates/cxx-qt-gen/src/naming/type_names.rs
@@ -178,6 +178,15 @@ impl TypeNames {
     }
 
     /// For a given rust ident return the CXX name with its namespace
+    ///
+    /// Ideally we'd want this type name to always be **fully** qualified, staring with `::`.
+    /// Unfortunately, this isn't always possible, as the Qt5 meta object system doesn't register
+    /// types with the fully qualified path :(
+    /// E.g. it will recognize `QString`, but not `::QString` from QML.
+    ///
+    /// This needs to be considered in many places (properties, signals, invokables, etc.)
+    /// Therefore, for now we'll use the qualified, but not fully qualified version of `namespace::type`.
+    /// This should work in most cases, but it's not perfect.
     pub fn cxx_qualified(&self, ident: &Ident) -> String {
         // Check if there is a cxx_name or namespace to handle
         let name = self.names.get(ident);
@@ -190,9 +199,9 @@ impl TypeNames {
         let cxx_name = name.cxx.clone().unwrap_or_else(|| name.rust.to_string());
 
         if let Some(namespace) = &name.namespace {
-            format!("::{namespace}::{cxx_name}")
+            format!("{namespace}::{cxx_name}")
         } else {
-            format!("::{cxx_name}")
+            cxx_name
         }
     }
 
@@ -284,7 +293,7 @@ mod tests {
 
         assert_eq!(types.num_types(), 1);
         assert_eq!(types.rust_qualified(&ident), parse_quote! { ffi::A });
-        assert_eq!(types.cxx_qualified(&ident), "::A");
+        assert_eq!(types.cxx_qualified(&ident), "A");
         assert!(types.namespace(&ident).is_none());
     }
 
@@ -302,7 +311,7 @@ mod tests {
             .is_ok());
 
         assert_eq!(types.num_types(), 1);
-        assert_eq!(types.cxx_qualified(&ident), "::B");
+        assert_eq!(types.cxx_qualified(&ident), "B");
         assert!(types.namespace(&ident).is_none());
         assert_eq!(types.rust_qualified(&ident), parse_quote! { ffi::A });
     }
@@ -354,7 +363,7 @@ mod tests {
             .populate(&ident, &[], Some("bridge_namespace"), &format_ident!("ffi"))
             .is_ok());
 
-        assert_eq!(types.cxx_qualified(&ident), "::bridge_namespace::A");
+        assert_eq!(types.cxx_qualified(&ident), "bridge_namespace::A");
         assert_eq!(types.namespace(&ident).unwrap(), "bridge_namespace");
         assert_eq!(types.num_types(), 1);
         assert_eq!(
@@ -397,7 +406,7 @@ mod tests {
         let type_names = parse_cxx_item(item);
         let ident = format_ident!("A");
         assert_eq!(type_names.num_types(), 1);
-        assert_eq!(type_names.cxx_qualified(&ident), "::B");
+        assert_eq!(type_names.cxx_qualified(&ident), "B");
 
         assert_eq!(type_names.rust_qualified(&ident), parse_quote! { ffi::A });
     }
@@ -431,15 +440,15 @@ mod tests {
 
         assert_eq!(
             &types.cxx_qualified(&format_ident!("A")),
-            "::type_namespace::B"
+            "type_namespace::B"
         );
         assert_eq!(
             &types.cxx_qualified(&format_ident!("C")),
-            "::extern_namespace::D"
+            "extern_namespace::D"
         );
         assert_eq!(
             &types.cxx_qualified(&format_ident!("E")),
-            "::bridge_namespace::E"
+            "bridge_namespace::E"
         );
 
         assert_eq!(
@@ -485,7 +494,7 @@ mod tests {
         assert_eq!(type_names.num_types(), 1);
         assert_eq!(type_names.cxx_unqualified(&ident).unwrap(), "EnumB");
         assert_eq!(type_names.namespace(&ident).unwrap(), "enum_namespace");
-        assert_eq!(type_names.cxx_qualified(&ident), "::enum_namespace::EnumB");
+        assert_eq!(type_names.cxx_qualified(&ident), "enum_namespace::EnumB");
         assert_eq!(
             type_names.rust_qualified(&ident),
             parse_quote! { ffi::EnumA }
@@ -507,7 +516,7 @@ mod tests {
 
         assert_eq!(types.num_types(), 1);
         assert_eq!(types.cxx_unqualified(&ident).unwrap(), "StructB");
-        assert_eq!(types.cxx_qualified(&ident), "::struct_namespace::StructB");
+        assert_eq!(types.cxx_qualified(&ident), "struct_namespace::StructB");
         assert_eq!(types.namespace(&ident).unwrap(), "struct_namespace");
         assert_eq!(types.rust_qualified(&ident), parse_quote! { ffi::StructA });
     }

diff --git a/crates/cxx-qt-gen/test_outputs/inheritance.cpp b/crates/cxx-qt-gen/test_outputs/inheritance.cpp
@@ -1,14 +1,14 @@
 #include "cxx-qt-gen/inheritance.cxxqt.h"
 
-::QVariant
-MyObject::data(::QModelIndex const& _index, ::std::int32_t _role) const
+QVariant
+MyObject::data(QModelIndex const& _index, ::std::int32_t _role) const
 {
   const ::rust::cxxqt1::MaybeLockGuard<MyObject> guard(*this);
   return dataWrapper(_index, _role);
 }
 
 bool
-MyObject::hasChildren(::QModelIndex const& _parent) const
+MyObject::hasChildren(QModelIndex const& _parent) const
 {
   const ::rust::cxxqt1::MaybeLockGuard<MyObject> guard(*this);
   return hasChildrenWrapper(_parent);

diff --git a/crates/cxx-qt-gen/test_outputs/inheritance.h b/crates/cxx-qt-gen/test_outputs/inheritance.h
@@ -18,9 +18,9 @@ class MyObject
   virtual ~MyObject() = default;
 
 public:
-  Q_INVOKABLE ::QVariant data(::QModelIndex const& _index,
-                              ::std::int32_t _role) const override;
-  Q_INVOKABLE bool hasChildren(::QModelIndex const& _parent) const override;
+  Q_INVOKABLE QVariant data(QModelIndex const& _index,
+                            ::std::int32_t _role) const override;
+  Q_INVOKABLE bool hasChildren(QModelIndex const& _parent) const override;
   template<class... Args>
   bool hasChildrenCxxQtInherit(Args... args) const
   {
@@ -34,9 +34,9 @@ class MyObject
   explicit MyObject(QObject* parent = nullptr);
 
 private:
-  ::QVariant dataWrapper(::QModelIndex const& _index,
-                         ::std::int32_t _role) const noexcept;
-  bool hasChildrenWrapper(::QModelIndex const& _parent) const noexcept;
+  QVariant dataWrapper(QModelIndex const& _index,
+                       ::std::int32_t _role) const noexcept;
+  bool hasChildrenWrapper(QModelIndex const& _parent) const noexcept;
 };
 
 static_assert(::std::is_base_of<QObject, MyObject>::value,

diff --git a/crates/cxx-qt-gen/test_outputs/invokables.cpp b/crates/cxx-qt-gen/test_outputs/invokables.cpp
@@ -23,8 +23,8 @@ MyObject::invokableMutable()
 }
 
 void
-MyObject::invokableParameters(::QColor const& opaque,
-                              ::QPoint const& trivial,
+MyObject::invokableParameters(QColor const& opaque,
+                              QPoint const& trivial,
                               ::std::int32_t primitive) const
 {
   const ::rust::cxxqt1::MaybeLockGuard<MyObject> guard(*this);
@@ -38,7 +38,7 @@ MyObject::invokableReturnOpaque()
   return invokableReturnOpaqueWrapper();
 }
 
-::QPoint
+QPoint
 MyObject::invokableReturnTrivial()
 {
   const ::rust::cxxqt1::MaybeLockGuard<MyObject> guard(*this);

diff --git a/crates/cxx-qt-gen/test_outputs/invokables.h b/crates/cxx-qt-gen/test_outputs/invokables.h
@@ -26,11 +26,11 @@ class MyObject
   void cppMethod() const;
   Q_INVOKABLE void invokable() const;
   Q_INVOKABLE void invokableMutable();
-  Q_INVOKABLE void invokableParameters(::QColor const& opaque,
-                                       ::QPoint const& trivial,
+  Q_INVOKABLE void invokableParameters(QColor const& opaque,
+                                       QPoint const& trivial,
                                        ::std::int32_t primitive) const;
   Q_INVOKABLE ::std::unique_ptr<Opaque> invokableReturnOpaque();
-  Q_INVOKABLE ::QPoint invokableReturnTrivial();
+  Q_INVOKABLE QPoint invokableReturnTrivial();
   Q_INVOKABLE void invokableFinal() const final;
   Q_INVOKABLE void invokableOverride() const override;
   Q_INVOKABLE virtual void invokableVirtual() const;
@@ -43,11 +43,11 @@ class MyObject
   void cppMethodWrapper() const noexcept;
   void invokableWrapper() const noexcept;
   void invokableMutableWrapper() noexcept;
-  void invokableParametersWrapper(::QColor const& opaque,
-                                  ::QPoint const& trivial,
+  void invokableParametersWrapper(QColor const& opaque,
+                                  QPoint const& trivial,
                                   ::std::int32_t primitive) const noexcept;
   ::std::unique_ptr<Opaque> invokableReturnOpaqueWrapper() noexcept;
-  ::QPoint invokableReturnTrivialWrapper() noexcept;
+  QPoint invokableReturnTrivialWrapper() noexcept;
   void invokableFinalWrapper() const noexcept;
   void invokableOverrideWrapper() const noexcept;
   void invokableVirtualWrapper() const noexcept;