Check supported extern_rust_function types.

Relates to #1166

engine/src/ast_discoverer.rs

@@ -103,7 +103,7 @@ impl<'b> PerModDiscoveries<'b> {
                     self.discoveries.extern_rust_funs.push(RustFun {
                         path: self.deeper_path(&fun.sig.ident),
                         sig: fun.sig.clone(),
-                        receiver: None,
+                        has_receiver: false,
                     });
                 }
             }
@@ -347,7 +347,7 @@ impl<'b> PerModDiscoveries<'b> {
                     self.discoveries.extern_rust_funs.push(RustFun {
                         path: self.deeper_path(&itm.sig.ident),
                         sig,
-                        receiver: Some(receiver.get_final_ident().clone()),
+                        has_receiver: true,
                     });
                     self.discoveries.extern_rust_types.push(receiver.clone())
                 } else {

engine/src/conversion/analysis/deps.rs

@@ -52,7 +52,7 @@ impl HasDependencies for Api<FnPrePhase1> {
                 superclass,
             } => Box::new(std::iter::once(superclass)),
             Api::RustSubclassFn { details, .. } => Box::new(details.dependencies.iter()),
-            Api::RustFn { receiver, .. } => Box::new(receiver.iter()),
+            Api::RustFn { deps, .. } => Box::new(deps.iter()),
             _ => Box::new(std::iter::empty()),
         }
     }
@@ -105,7 +105,7 @@ impl HasDependencies for Api<FnPhase> {
                 superclass,
             } => Box::new(std::iter::once(superclass)),
             Api::RustSubclassFn { details, .. } => Box::new(details.dependencies.iter()),
-            Api::RustFn { receiver, .. } => Box::new(receiver.iter()),
+            Api::RustFn { deps, .. } => Box::new(deps.iter()),
             _ => Box::new(std::iter::empty()),
         }
     }

engine/src/conversion/api.rs

@@ -531,7 +531,7 @@ pub(crate) enum Api<T: AnalysisPhase> {
     RustFn {
         name: ApiName,
         details: RustFun,
-        receiver: Option<QualifiedName>,
+        deps: Vec<QualifiedName>,
     },
     /// Some function for the extern "Rust" block.
     RustSubclassFn {

engine/src/conversion/codegen_rs/mod.rs

@@ -685,34 +685,28 @@ impl<'a> RsCodeGenerator<'a> {
                 details:
                     RustFun {
                         path,
-                        sig,
-                        receiver: None,
+                        mut sig,
+                        has_receiver,
                         ..
                     },
                 ..
-            } => RsCodegenResult {
-                global_items: vec![parse_quote! {
-                    use super::#path;
-                }],
-                extern_rust_mod_items: vec![parse_quote! {
-                    #sig;
-                }],
-                ..Default::default()
-            },
-            Api::RustFn {
-                details:
-                    RustFun {
-                        sig,
-                        receiver: Some(_),
-                        ..
+            } => {
+                sig.inputs = unqualify_params(sig.inputs);
+                sig.output = unqualify_ret_type(sig.output);
+                RsCodegenResult {
+                    global_items: if has_receiver {
+                        vec![parse_quote! {
+                            use super::#path;
+                        }]
+                    } else {
+                        Vec::new()
                     },
-                ..
-            } => RsCodegenResult {
-                extern_rust_mod_items: vec![parse_quote! {
-                    #sig;
-                }],
-                ..Default::default()
-            },
+                    extern_rust_mod_items: vec![parse_quote! {
+                        #sig;
+                    }],
+                    ..Default::default()
+                }
+            }
             Api::RustSubclassFn {
                 details, subclass, ..
             } => Self::generate_subclass_fn(id, *details, subclass),

engine/src/conversion/convert_error.rs

@@ -131,6 +131,14 @@ pub enum ConvertError {
     ReferringToGenericTypeParam,
     #[error("This forward declaration was nested within another struct/class. autocxx is unable to represent inner types if they are forward declarations.")]
     ForwardDeclaredNestedType,
+    #[error("extern_rust_function only supports limited parameter and return types. This is not such a supported type")]
+    UnsupportedTypeForExternFun,
+    #[error("extern_rust_function requires a fully qualified receiver, that is: fn a(self: &SomeType) as opposed to fn a(&self)")]
+    ExternRustFunRequiresFullyQualifiedReceiver,
+    #[error("extern_rust_function cannot support &mut T references; instead use Pin<&mut T> (see cxx documentation for more details")]
+    PinnedReferencesRequiredForExternFun,
+    #[error("extern_rust_function cannot currently support qualified type paths (that is, foo::bar::Baz). All type paths must be within the current module, imported using 'use'. This restriction may be lifted in future.")]
+    NamespacesNotSupportedForExternFun,
 }
 
 /// Ensures that error contexts are always created using the constructors in this

engine/src/conversion/error_reporter.rs

@@ -127,11 +127,11 @@ pub(crate) fn convert_apis<FF, SF, EF, TF, A, B: 'static>(
             Api::RustFn {
                 name,
                 details,
-                receiver,
+                deps,
             } => Ok(Box::new(std::iter::once(Api::RustFn {
                 name,
                 details,
-                receiver,
+                deps,
             }))),
             Api::RustSubclassFn {
                 name,

engine/src/conversion/parse/parse_bindgen.rs

@@ -26,7 +26,10 @@ use crate::{
     types::validate_ident_ok_for_cxx,
 };
 use autocxx_parser::{IncludeCppConfig, RustPath};
-use syn::{parse_quote, Fields, Ident, Item, Type, TypePath, UseTree};
+use syn::{
+    parse_quote, AngleBracketedGenericArguments, Fields, GenericArgument, Ident, Item, PatType,
+    PathArguments, PathSegment, ReturnType, Signature, Type, TypePath, TypeReference, UseTree,
+};
 
 use super::{
     super::utilities::generate_utilities, bindgen_semantic_attributes::BindgenSemanticAttributes,
@@ -76,7 +79,7 @@ impl<'a> ParseBindgen<'a> {
         if !self.config.exclude_utilities() {
             generate_utilities(&mut self.apis, self.config);
         }
-        self.add_apis_from_config();
+        self.add_apis_from_config()?;
         let root_ns = Namespace::new();
         self.parse_mod_items(items, root_ns);
         self.confirm_all_generate_directives_obeyed()?;
@@ -86,23 +89,20 @@ impl<'a> ParseBindgen<'a> {
 
     /// Some API items are not populated from bindgen output, but instead
     /// directly from items in the config.
-    fn add_apis_from_config(&mut self) {
+    fn add_apis_from_config(&mut self) -> Result<(), ConvertError> {
         self.apis
             .extend(self.config.subclasses.iter().map(|sc| Api::Subclass {
                 name: SubclassName::new(sc.subclass.clone()),
                 superclass: QualifiedName::new_from_cpp_name(&sc.superclass),
             }));
-        self.apis
-            .extend(self.config.extern_rust_funs.iter().map(|fun| {
-                let id = fun.sig.ident.clone();
-                Api::RustFn {
-                    name: ApiName::new_in_root_namespace(id),
-                    details: fun.clone(),
-                    receiver: fun.receiver.as_ref().map(|receiver_id| {
-                        QualifiedName::new(&Namespace::new(), receiver_id.clone())
-                    }),
-                }
-            }));
+        for fun in &self.config.extern_rust_funs {
+            let id = fun.sig.ident.clone();
+            self.apis.push(Api::RustFn {
+                name: ApiName::new_in_root_namespace(id),
+                details: fun.clone(),
+                deps: assemble_extern_fun_deps(&fun.sig)?,
+            })
+        }
         let unique_rust_types: HashSet<&RustPath> = self.config.rust_types.iter().collect();
         self.apis.extend(unique_rust_types.into_iter().map(|path| {
             let id = path.get_final_ident();
@@ -125,6 +125,7 @@ impl<'a> ParseBindgen<'a> {
                     }
                 }),
         );
+        Ok(())
     }
 
     /// We do this last, _after_ we've parsed all the APIs, because we might want to actually
@@ -394,3 +395,86 @@ impl<'a> ParseBindgen<'a> {
         Ok(())
     }
 }
+
+fn assemble_extern_fun_deps(sig: &Signature) -> Result<Vec<QualifiedName>, ConvertError> {
+    let mut deps = HashSet::new();
+    // It's possible that this will need to be implemented using TypeConverter
+    // and the encountered_types field on its annotated results.
+    // But the design of that code is intended to convert from C++ types
+    // (via bindgen) to cxx types, and instead here we're starting with pure
+    // Rust types as written by a Rustacean human. It may therefore not
+    // be quite right to go via TypeConverter.
+    // Also, by doing it ourselves here, we're in a better place to emit
+    // meaningful errors about types which can't be supported within
+    // extern_rust_fun.
+    if let ReturnType::Type(_, ty) = &sig.output {
+        add_type_to_deps(&*ty, &mut deps)?;
+    }
+    for input in &sig.inputs {
+        match input {
+            syn::FnArg::Receiver(_) => {
+                return Err(ConvertError::ExternRustFunRequiresFullyQualifiedReceiver)
+            }
+            syn::FnArg::Typed(PatType { ty, .. }) => add_type_to_deps(&*ty, &mut deps)?,
+        }
+    }
+    Ok(deps.into_iter().collect())
+}
+
+/// For all types within an extern_rust_function signature, add them to the deps
+/// hash, or raise an appropriate error.
+fn add_type_to_deps(ty: &Type, deps: &mut HashSet<QualifiedName>) -> Result<(), ConvertError> {
+    match ty {
+        Type::Reference(TypeReference {
+            mutability: Some(_),
+            ..
+        }) => return Err(ConvertError::PinnedReferencesRequiredForExternFun),
+        Type::Reference(TypeReference { elem, .. }) => match &**elem {
+            Type::Path(tp) => add_path_to_deps(tp, deps)?,
+            _ => return Err(ConvertError::UnsupportedTypeForExternFun),
+        },
+        Type::Path(tp) => {
+            if let Some(PathSegment {
+                ident,
+                arguments:
+                    PathArguments::AngleBracketed(AngleBracketedGenericArguments { args, .. }),
+            }) = tp.path.segments.last()
+            {
+                if ident.to_string() != "Pin" {
+                    return Err(ConvertError::UnsupportedTypeForExternFun);
+                }
+                if args.len() != 1 {
+                    return Err(ConvertError::UnsupportedTypeForExternFun);
+                }
+                if let Some(GenericArgument::Type(ty)) = args.first() {
+                    add_type_to_deps(&ty, deps)?
+                } else {
+                    return Err(ConvertError::UnsupportedTypeForExternFun);
+                }
+            } else {
+                add_path_to_deps(tp, deps)?
+            }
+        }
+        _ => return Err(ConvertError::UnsupportedTypeForExternFun),
+    };
+    Ok(())
+}
+
+fn add_path_to_deps(
+    type_path: &TypePath,
+    deps: &mut HashSet<QualifiedName>,
+) -> Result<(), ConvertError> {
+    if let Some(PathSegment {
+        arguments: PathArguments::AngleBracketed(..) | PathArguments::Parenthesized(..),
+        ..
+    }) = type_path.path.segments.last()
+    {
+        return Err(ConvertError::UnsupportedTypeForExternFun);
+    }
+    let qn = QualifiedName::from_type_path(type_path);
+    if !qn.get_namespace().is_empty() {
+        return Err(ConvertError::NamespacesNotSupportedForExternFun);
+    }
+    deps.insert(qn);
+    Ok(())
+}

integration-tests/tests/integration_test.rs

@@ -7052,15 +7052,14 @@ fn test_extern_rust_fn_callback() {
         #[autocxx::extern_rust::extern_rust_function]
         pub fn called_from_cpp(a: &mut ffi::a) {}
     };
-    do_run_test_manual(
-        "",
-        hdr,
-        rs,
-        None,
-        None
-    ).unwrap();
+    do_run_test_manual("", hdr, rs, None, None).unwrap();
 }
 
+// TODO: there are various other tests for extern_rust_fn we should add:
+// 1) taking mutable and immutable references
+// 2) ensuring that types on which the signature depends as receiver,
+//    parameters and return are not garbage collected
+
 #[test]
 fn test_rust_reference_no_autodiscover() {
     let hdr = indoc! {"

parser/src/config.rs

@@ -165,7 +165,7 @@ pub struct Subclass {
 pub struct RustFun {
     pub path: RustPath,
     pub sig: Signature,
-    pub receiver: Option<Ident>,
+    pub has_receiver: bool,
 }
 
 impl std::fmt::Debug for RustFun {

parser/src/directives.rs

@@ -472,7 +472,7 @@ impl Directive for ExternRustFun {
         config.extern_rust_funs.push(RustFun {
             path,
             sig,
-            receiver: None,
+            has_receiver: false,
         });
         Ok(())
     }