cxx-qt-gen: allow for safe / unsafe extern "RustQt"

crates/cxx-qt-gen/src/generator/cpp/inherit.rs

@@ -63,14 +63,14 @@ mod tests {
 
     use super::*;
     use crate::generator::cpp::property::tests::require_header;
+    use crate::parser::inherit::ParsedInheritedMethod;
     use crate::parser::CaseConversion;
-    use crate::{parser::inherit::ParsedInheritedMethod, syntax::safety::Safety};
 
     fn generate_from_foreign(
         method: ForeignItemFn,
         base_class: Option<&str>,
     ) -> Result<GeneratedCppQObjectBlocks> {
-        let method = ParsedInheritedMethod::parse(method, Safety::Safe, CaseConversion::none())?;
+        let method = ParsedInheritedMethod::parse(method, CaseConversion::none())?;
         let inherited_methods = vec![&method];
         let base_class = base_class.map(|s| s.to_owned());
         generate(
@@ -93,8 +93,7 @@ mod tests {
             #[cfg(test_cfg_disabled)]
             fn test(self: &T, a: B, b: C);
         };
-        let method =
-            ParsedInheritedMethod::parse(method, Safety::Safe, CaseConversion::none()).unwrap();
+        let method = ParsedInheritedMethod::parse(method, CaseConversion::none()).unwrap();
         let inherited_methods = vec![&method];
         let base_class = Some("TestBaseClass".to_owned());
         let opt = GeneratedOpt {

crates/cxx-qt-gen/src/generator/cpp/property/signal.rs

@@ -7,7 +7,6 @@ use syn::ForeignItemFn;
 
 use crate::naming::Name;
 use crate::parser::CaseConversion;
-use crate::syntax::safety::Safety;
 use crate::{
     generator::naming::property::{NameState, QPropertyNames},
     parser::signals::ParsedSignal,
@@ -26,7 +25,7 @@ pub fn generate(idents: &QPropertyNames, qobject_name: &Name) -> Option<ParsedSi
             fn #notify_rust(self: Pin<&mut #cpp_class_rust>);
         };
 
-        Some(ParsedSignal::parse(method, Safety::Safe, CaseConversion::none()).unwrap())
+        Some(ParsedSignal::parse(method, CaseConversion::none()).unwrap())
     } else {
         None
     }

crates/cxx-qt-gen/src/generator/rust/externcxxqt.rs

@@ -80,7 +80,13 @@ impl GeneratedRustFragment {
         // Build the signals
         for signal in &extern_cxxqt_block.signals {
             let qobject_name = type_names.lookup(&signal.qobject_ident)?;
-            generated.push(generate_rust_signal(signal, qobject_name, type_names)?);
+            generated.push(generate_rust_signal(
+                signal,
+                qobject_name,
+                type_names,
+                // Copy the same safety as the extern C++Qt block into the generated extern C++
+                extern_cxxqt_block.unsafety.map(|_| quote! { unsafe }),
+            )?);
         }
 
         Ok(GeneratedRustFragment::flatten(generated))

crates/cxx-qt-gen/src/generator/rust/inherit.rs

@@ -73,29 +73,20 @@ pub fn generate(
 mod tests {
     use super::*;
     use crate::parser::CaseConversion;
-    use crate::{
-        generator::naming::qobject::tests::create_qobjectname, syntax::safety::Safety,
-        tests::assert_tokens_eq,
-    };
+    use crate::{generator::naming::qobject::tests::create_qobjectname, tests::assert_tokens_eq};
     use syn::{parse_quote, ForeignItemFn};
 
-    fn generate_from_foreign(
-        method: ForeignItemFn,
-        safety: Safety,
-    ) -> Result<GeneratedRustFragment> {
-        let method = ParsedInheritedMethod::parse(method, safety, CaseConversion::none())?;
+    fn generate_from_foreign(method: ForeignItemFn) -> Result<GeneratedRustFragment> {
+        let method = ParsedInheritedMethod::parse(method, CaseConversion::none())?;
         let inherited_methods = vec![&method];
         generate(&create_qobjectname(), &inherited_methods)
     }
 
     #[test]
     fn test_mutable() {
-        let generated = generate_from_foreign(
-            parse_quote! {
-                fn test(self: Pin<&mut MyObject>, a: B, b: C);
-            },
-            Safety::Safe,
-        )
+        let generated = generate_from_foreign(parse_quote! {
+            fn test(self: Pin<&mut MyObject>, a: B, b: C);
+        })
         .unwrap();
 
         assert_eq!(generated.cxx_mod_contents.len(), 1);
@@ -114,12 +105,9 @@ mod tests {
 
     #[test]
     fn test_immutable() {
-        let generated = generate_from_foreign(
-            parse_quote! {
-                fn test(self: &MyObject, a: B, b: C);
-            },
-            Safety::Safe,
-        )
+        let generated = generate_from_foreign(parse_quote! {
+            fn test(self: &MyObject, a: B, b: C);
+        })
         .unwrap();
 
         assert_eq!(generated.cxx_mod_contents.len(), 1);
@@ -138,12 +126,9 @@ mod tests {
 
     #[test]
     fn test_unsafe() {
-        let generated = generate_from_foreign(
-            parse_quote! {
-                unsafe fn test(self: &MyObject);
-            },
-            Safety::Unsafe,
-        )
+        let generated = generate_from_foreign(parse_quote! {
+            unsafe fn test(self: &MyObject);
+        })
         .unwrap();
 
         assert_eq!(generated.cxx_mod_contents.len(), 1);

crates/cxx-qt-gen/src/generator/rust/method.rs

@@ -33,11 +33,6 @@ pub fn generate_rust_methods(
 
             let return_type = &invokable.method.sig.output;
 
-            let mut unsafe_call = Some(quote! { unsafe });
-            if invokable.safe {
-                std::mem::swap(&mut unsafe_call, &mut None);
-            }
-
             let cfgs = &invokable.cfgs;
             let cxx_namespace = qobject_names.namespace_tokens();
 
@@ -46,6 +41,18 @@ pub fn generate_rust_methods(
             } else {
                 ("Rust", None)
             };
+            // When generating extern Rust forward the block unsafe to fn
+            // This allows for then defining pointer args when the whole block
+            // is unsafe, as CXX does not allow for unsafe Rust
+            let unsafe_call = if invokable.safe {
+                if block_safety.is_none() && invokable.unsafe_block {
+                    Some(quote! { unsafe })
+                } else {
+                    None
+                }
+            } else {
+                Some(quote! { unsafe })
+            };
 
             GeneratedRustFragment::from_cxx_item(parse_quote_spanned! {
                 invokable.method.span() =>

crates/cxx-qt-gen/src/generator/rust/property/signal.rs

@@ -6,7 +6,6 @@
 use syn::ForeignItemFn;
 
 use crate::parser::CaseConversion;
-use crate::syntax::safety::Safety;
 use crate::{
     generator::naming::{
         property::{NameState, QPropertyNames},
@@ -29,7 +28,7 @@ pub fn generate(idents: &QPropertyNames, qobject_names: &QObjectNames) -> Option
             fn #notify_rust(self: Pin<&mut #cpp_class_rust>);
         };
 
-        Some(ParsedSignal::parse(method, Safety::Safe, CaseConversion::none()).unwrap())
+        Some(ParsedSignal::parse(method, CaseConversion::none()).unwrap())
     } else {
         None
     }

crates/cxx-qt-gen/src/generator/rust/signals.rs

@@ -15,6 +15,7 @@ use crate::{
     naming::{rust::syn_type_cxx_bridge_to_qualified, Name, TypeNames},
     parser::signals::ParsedSignal,
 };
+use proc_macro2::TokenStream;
 use quote::{quote, quote_spanned};
 use syn::spanned::Spanned;
 use syn::{parse_quote, parse_quote_spanned, FnArg, Ident, Result, Type};
@@ -23,6 +24,7 @@ pub fn generate_rust_signal(
     signal: &ParsedSignal,
     qobject_name: &Name,
     type_names: &TypeNames,
+    unsafety_block: Option<TokenStream>,
 ) -> Result<GeneratedRustFragment> {
     let span = signal.method.span();
     let idents = QSignalNames::from(signal);
@@ -90,16 +92,14 @@ pub fn generate_rust_signal(
     let self_type_qualified = syn_type_cxx_bridge_to_qualified(&self_type_cxx, type_names)?;
     let qualified_impl = qobject_name.rust_qualified();
 
-    let mut unsafe_block = None;
-    let mut unsafe_call = Some(quote! { unsafe });
-    if signal.safe {
-        std::mem::swap(&mut unsafe_call, &mut unsafe_block);
-    }
-
     let rust_class_name = qobject_name.rust_unqualified();
-
     let cpp_ident = idents.name.cxx_unqualified();
 
+    let unsafe_call = if signal.safe {
+        None
+    } else {
+        Some(quote! { unsafe })
+    };
     let doc_comments = &signal.docs;
     let cfgs = &signal.cfgs;
     let namespace = if let Some(namespace) = qobject_name.namespace() {
@@ -127,7 +127,7 @@ pub fn generate_rust_signal(
     if !signal.private {
         cxx_mod_contents.push(parse_quote_spanned! {
             span=>
-            #unsafe_block extern "C++" {
+            #unsafety_block extern "C++" {
                 #[cxx_name = #cpp_ident]
                 #(#cfgs)*
                 #(#doc_comments)*
@@ -254,7 +254,20 @@ pub fn generate_rust_signals(
 ) -> Result<GeneratedRustFragment> {
     let generated = signals
         .iter()
-        .map(|signal| generate_rust_signal(signal, &qobject_names.name, type_names))
+        .map(|signal| {
+            generate_rust_signal(
+                signal,
+                &qobject_names.name,
+                type_names,
+                // When generating from a RustQt block we use the opposite of the signal safety
+                // This means that a safe signal has unsafe block, and the reverse
+                if signal.safe {
+                    Some(quote! { unsafe })
+                } else {
+                    None
+                },
+            )
+        })
         .collect::<Result<Vec<_>>>()?;
 
     Ok(GeneratedRustFragment::flatten(generated))
@@ -402,7 +415,13 @@ mod tests {
             generate_rust_signals(&vec![&qsignal], &qobject_names, &type_names).unwrap();
 
         let qobject_name = type_names.lookup(&qsignal.qobject_ident).unwrap().clone();
-        let other_generated = generate_rust_signal(&qsignal, &qobject_name, &type_names).unwrap();
+        let other_generated = generate_rust_signal(
+            &qsignal,
+            &qobject_name,
+            &type_names,
+            Some(quote! { unsafe }),
+        )
+        .unwrap();
 
         assert_eq!(generated, other_generated);
 
@@ -865,7 +884,13 @@ mod tests {
         let type_names = TypeNames::mock();
 
         let qobject_name = type_names.lookup(&qsignal.qobject_ident).unwrap().clone();
-        let generated = generate_rust_signal(&qsignal, &qobject_name, &type_names).unwrap();
+        let generated = generate_rust_signal(
+            &qsignal,
+            &qobject_name,
+            &type_names,
+            Some(quote! { unsafe }),
+        )
+        .unwrap();
 
         common_asserts(&generated.cxx_mod_contents, &generated.cxx_qt_mod_contents);
     }

crates/cxx-qt-gen/src/parser/cxxqtdata.rs

@@ -14,10 +14,13 @@ use crate::{
     },
     syntax::{
         attribute::attribute_get_path, expr::expr_to_string, foreignmod::ForeignTypeIdentAlias,
-        path::path_compare_str, safety::Safety,
+        path::path_compare_str,
     },
 };
-use syn::{ForeignItem, Ident, Item, ItemEnum, ItemForeignMod, ItemImpl, ItemMacro, Meta, Result};
+use syn::{
+    spanned::Spanned, Error, ForeignItem, Ident, Item, ItemEnum, ItemForeignMod, ItemImpl,
+    ItemMacro, Meta, Result,
+};
 
 pub struct ParsedCxxQtData {
     /// Map of the QObjects defined in the module that will be used for code generation
@@ -142,32 +145,44 @@ impl ParsedCxxQtData {
             .transpose()?
             .or_else(|| self.namespace.clone());
 
-        let safe_call = if foreign_mod.unsafety.is_some() {
-            Safety::Safe
-        } else {
-            Safety::Unsafe
-        };
-
         for item in foreign_mod.items.drain(..) {
             match item {
                 ForeignItem::Fn(foreign_fn) => {
                     // Test if the function is a signal
                     if attribute_get_path(&foreign_fn.attrs, &["qsignal"]).is_some() {
                         let parsed_signal_method =
-                            ParsedSignal::parse(foreign_fn, safe_call, auto_case)?;
+                            ParsedSignal::parse(foreign_fn.clone(), auto_case)?;
+                        if parsed_signal_method.inherit
+                            && foreign_fn.sig.unsafety.is_none()
+                            && foreign_mod.unsafety.is_none()
+                        {
+                            return Err(Error::new(foreign_fn.span(), "block must be declared `unsafe extern \"RustQt\"` if it contains any safe-to-call #[inherit] qsignals"));
+                        }
+
                         self.signals.push(parsed_signal_method);
 
                         // Test if the function is an inheritance method
                         //
                         // Note that we need to test for qsignal first as qsignals have their own inherit meaning
                     } else if attribute_get_path(&foreign_fn.attrs, &["inherit"]).is_some() {
+                        // We need to check that any safe functions are defined inside an unsafe block
+                        // as with inherit we cannot fully prove the implementation and we can then
+                        // directly copy the unsafetyness into the generated extern C++ block
+                        if foreign_fn.sig.unsafety.is_none() && foreign_mod.unsafety.is_none() {
+                            return Err(Error::new(foreign_fn.span(), "block must be declared `unsafe extern \"RustQt\"` if it contains any safe-to-call #[inherit] functions"));
+                        }
+
                         let parsed_inherited_method =
-                            ParsedInheritedMethod::parse(foreign_fn, safe_call, auto_case)?;
+                            ParsedInheritedMethod::parse(foreign_fn, auto_case)?;
 
                         self.inherited_methods.push(parsed_inherited_method);
                         // Remaining methods are either C++ methods or invokables
                     } else {
-                        let parsed_method = ParsedMethod::parse(foreign_fn, safe_call, auto_case)?;
+                        let parsed_method = ParsedMethod::parse(
+                            foreign_fn,
+                            auto_case,
+                            foreign_mod.unsafety.is_some(),
+                        )?;
                         self.methods.push(parsed_method);
                     }
                 }
@@ -246,20 +261,28 @@ mod tests {
                 }
             }
             {
-                // Not unsafe
-                extern "RustQt" {
+                // Namespaces aren't allowed on qinvokables
+                unsafe extern "RustQt" {
                     #[qinvokable]
+                    #[namespace = "disallowed"]
                     fn invokable(self: &MyObject);
                 }
             }
             {
-                // Namespaces aren't allowed on qinvokables
-                unsafe extern "RustQt" {
-                    #[qinvokable]
-                    #[namespace = "disallowed"]
+                // Block or fn must be unsafe for inherit methods
+                extern "RustQt" {
+                    #[inherit]
                     fn invokable(self: &MyObject);
                 }
             }
+            {
+                // Block or fn must be unsafe for inherit qsignals
+                extern "RustQt" {
+                    #[inherit]
+                    #[qsignal]
+                    fn signal(self: Pin<&mut MyObject>);
+                }
+            }
             {
                 // Qenum without namespace
                 #[qenum]