Spot deleted move/copy constructors.

This also generally improves tracking of special members which will be important
as we come towards supporting more moveit functionality.

Fixes #489.

engine/Cargo.toml

@@ -36,8 +36,8 @@ log = "0.4"
 proc-macro2 = "1.0.11"
 quote = "1.0"
 indoc = "1.0"
-autocxx-bindgen = "=0.59.10"
-#autocxx-bindgen = { git = "https://github.com/adetaylor/rust-bindgen", branch = "rvalue-references" }
+autocxx-bindgen = "=0.59.11"
+#autocxx-bindgen = { git = "https://github.com/adetaylor/rust-bindgen", branch = "denote-deleted-move-constructors" }
 itertools = "0.10"
 cc = { version = "1.0", optional = true }
 unzip-n = "0.1.2"

engine/src/conversion/analysis/casts.rs

@@ -109,7 +109,7 @@ fn create_cast(from: &QualifiedName, to: &QualifiedName, mutable: CastMutability
             vis: parse_quote! { pub },
             virtualness: crate::conversion::api::Virtualness::None,
             cpp_vis: crate::conversion::api::CppVisibility::Public,
-            is_move_constructor: false,
+            special_member: None,
             unused_template_param: false,
             references: References::new_with_this_and_return_as_reference(),
             original_name: None,
@@ -119,6 +119,7 @@ fn create_cast(from: &QualifiedName, to: &QualifiedName, mutable: CastMutability
                 to_type: to.clone(),
                 mutable,
             }),
+            is_deleted: false,
         }),
         analysis: (),
     }

engine/src/conversion/analysis/fun/mod.rs

@@ -25,8 +25,8 @@ use crate::{
             type_converter::{self, add_analysis, TypeConversionContext, TypeConverter},
         },
         api::{
-            ApiName, CastMutability, CppVisibility, FuncToConvert, References, SubclassName,
-            Synthesis, Virtualness,
+            ApiName, CastMutability, CppVisibility, FuncToConvert, References, SpecialMemberKind,
+            SubclassName, Synthesis, Virtualness,
         },
         convert_error::ConvertErrorWithContext,
         convert_error::ErrorContext,
@@ -705,7 +705,7 @@ impl<'a> FnAnalyzer<'a> {
             FnKind::Method(_, MethodKind::Static) => {}
             FnKind::Method(ref self_ty, _) => {
                 // Reject move constructors.
-                if fun.is_move_constructor {
+                if matches!(fun.special_member, Some(SpecialMemberKind::MoveConstructor)) {
                     self.has_unrepresentable_constructors
                         .insert(self_ty.clone());
                     return Err(contextualize_error(
@@ -1208,12 +1208,13 @@ impl<'a> FnAnalyzer<'a> {
                         vis: parse_quote! { pub },
                         virtualness: Virtualness::None,
                         cpp_vis: CppVisibility::Public,
-                        is_move_constructor: false,
+                        special_member: None,
                         unused_template_param: false,
                         references: References::default(),
                         original_name: None,
                         synthesized_this_type: None,
                         synthesis: None,
+                        is_deleted: false,
                     }),
                 )
             });

engine/src/conversion/analysis/fun/subclass.rs

@@ -65,11 +65,12 @@ pub(super) fn create_subclass_fn_wrapper(
         vis: fun.vis.clone(),
         virtualness: Virtualness::None,
         cpp_vis: CppVisibility::Public,
-        is_move_constructor: false,
+        special_member: None,
         unused_template_param: fun.unused_template_param,
         original_name: None,
         references: fun.references.clone(),
         synthesis: fun.synthesis.clone(),
+        is_deleted: fun.is_deleted,
     })
 }
 
@@ -201,13 +202,14 @@ pub(super) fn create_subclass_constructor(
         vis: fun.vis.clone(),
         virtualness: Virtualness::None,
         cpp_vis: CppVisibility::Public,
-        is_move_constructor: false,
+        special_member: fun.special_member.clone(),
         original_name: None,
         unused_template_param: fun.unused_template_param,
         references: fun.references.clone(),
         synthesized_this_type: Some(cpp.clone()),
         self_ty: Some(cpp),
         synthesis,
+        is_deleted: fun.is_deleted,
     });
     let subclass_constructor_name = ApiName::new_with_cpp_name(
         &Namespace::new(),

engine/src/conversion/analysis/pod/mod.rs

@@ -18,12 +18,17 @@ use std::collections::{HashMap, HashSet};
 
 use autocxx_parser::IncludeCppConfig;
 use byvalue_checker::ByValueChecker;
-use syn::{ItemEnum, ItemStruct, Type, Visibility};
+use syn::{FnArg, ItemEnum, ItemStruct, Type, TypePtr, Visibility};
+
+use itertools::{Either, Itertools};
 
 use crate::{
     conversion::{
         analysis::type_converter::{add_analysis, TypeConversionContext, TypeConverter},
-        api::{AnalysisPhase, Api, ApiName, CppVisibility, StructDetails, TypeKind, UnanalyzedApi},
+        api::{
+            AnalysisPhase, Api, ApiName, CppVisibility, FuncToConvert, SpecialMemberKind,
+            StructDetails, TypeKind, UnanalyzedApi,
+        },
         convert_error::{ConvertErrorWithContext, ErrorContext},
         error_reporter::convert_apis,
         parse::BindgenSemanticAttributes,
@@ -43,6 +48,7 @@ pub(crate) struct PodAnalysis {
     /// abstract or not.
     pub(crate) castable_bases: HashSet<QualifiedName>,
     pub(crate) field_deps: HashSet<QualifiedName>,
+    pub(crate) movable: bool,
 }
 
 pub(crate) struct PodPhase;
@@ -67,6 +73,8 @@ pub(crate) fn analyze_pod_apis(
     // a type contains a std::string or some other type which can't be
     // held safely by value in Rust.
     let byvalue_checker = ByValueChecker::new_from_apis(&apis, config)?;
+    // We'll also note which types have deleted move constructors.
+    let (deleted_move_constructors, apis) = find_deleted_move_and_copy_constructors(apis);
     let mut extra_apis = Vec::new();
     let mut type_converter = TypeConverter::new(config, &apis);
     let mut results = Vec::new();
@@ -82,6 +90,7 @@ pub(crate) fn analyze_pod_apis(
                 name,
                 details,
                 config,
+                &deleted_move_constructors,
             )
         },
         analyze_enum,
@@ -103,6 +112,7 @@ pub(crate) fn analyze_pod_apis(
                 name,
                 details,
                 config,
+                &deleted_move_constructors,
             )
         },
         analyze_enum,
@@ -128,7 +138,9 @@ fn analyze_struct(
     name: ApiName,
     mut details: Box<StructDetails>,
     config: &IncludeCppConfig,
+    deleted_move_constructors: &HashSet<QualifiedName>,
 ) -> Result<Box<dyn Iterator<Item = Api<PodPhase>>>, ConvertErrorWithContext> {
+    let movable = !deleted_move_constructors.contains(&name.name);
     let id = name.name.get_final_ident();
     if details.vis != CppVisibility::Public {
         return Err(ConvertErrorWithContext(
@@ -169,6 +181,7 @@ fn analyze_struct(
             bases: bases.into_keys().collect(),
             castable_bases,
             field_deps,
+            movable,
         },
     })))
 }
@@ -206,3 +219,43 @@ fn get_bases(item: &ItemStruct) -> HashMap<QualifiedName, bool> {
         })
         .collect()
 }
+
+fn find_deleted_move_and_copy_constructors(
+    apis: Vec<Api<TypedefPhase>>,
+) -> (HashSet<QualifiedName>, Vec<Api<TypedefPhase>>) {
+    // Remove any deleted move + copy constructors from the API list and list the types
+    // that they construct.
+    apis.into_iter().partition_map(|api| match api {
+        Api::Function { ref fun, .. } => match &**fun {
+            FuncToConvert {
+                special_member:
+                    Some(SpecialMemberKind::MoveConstructor | SpecialMemberKind::CopyConstructor),
+                is_deleted: true,
+                inputs,
+                ..
+             } => match is_a_pointer_arg(inputs.iter().next()) {
+                    Some(ty) => Either::Left(ty),
+                    _ => panic!("found special constructor member with something other than a pointer first arg"),
+                },
+            _ => Either::Right(api),
+        },
+        _ => Either::Right(api),
+    })
+}
+
+/// Determine if a function argument is a pointer, and if so, to what.
+/// It's unfortunate that we need to do this during the POD analysis but
+/// for now, it's the best way to identify special constructor members.
+fn is_a_pointer_arg(arg: Option<&FnArg>) -> Option<QualifiedName> {
+    arg.map(|arg| match arg {
+        FnArg::Receiver(..) => None,
+        FnArg::Typed(pt) => match &*pt.ty {
+            Type::Ptr(TypePtr { elem, .. }) => match &**elem {
+                Type::Path(typ) => Some(QualifiedName::from_type_path(typ)),
+                _ => None,
+            },
+            _ => None,
+        },
+    })
+    .flatten()
+}

engine/src/conversion/api.rs

@@ -136,6 +136,14 @@ impl References {
     }
 }
 
+#[derive(Clone)]
+pub(crate) enum SpecialMemberKind {
+    DefaultConstructor,
+    CopyConstructor,
+    MoveConstructor,
+    Destructor,
+}
+
 /// A C++ function for which we need to generate bindings, but haven't
 /// yet analyzed in depth. This is little more than a `ForeignItemFn`
 /// broken down into its constituent parts, plus some metadata from the
@@ -155,7 +163,7 @@ pub(crate) struct FuncToConvert {
     pub(crate) vis: Visibility,
     pub(crate) virtualness: Virtualness,
     pub(crate) cpp_vis: CppVisibility,
-    pub(crate) is_move_constructor: bool,
+    pub(crate) special_member: Option<SpecialMemberKind>,
     pub(crate) unused_template_param: bool,
     pub(crate) references: References,
     pub(crate) original_name: Option<String>,
@@ -169,6 +177,7 @@ pub(crate) struct FuncToConvert {
     /// If Some, this function didn't really exist in the original
     /// C++ and instead we're synthesizing it.
     pub(crate) synthesis: Option<Synthesis>,
+    pub(crate) is_deleted: bool,
 }
 
 /// Layers of analysis which may be applied to decorate each API.

engine/src/conversion/codegen_rs/impl_item_creator.rs

@@ -15,11 +15,11 @@
 use autocxx_parser::IncludeCppConfig;
 use syn::{parse_quote, Ident, Item};
 
-pub(crate) fn create_impl_items(id: &Ident, config: &IncludeCppConfig) -> Vec<Item> {
+pub(crate) fn create_impl_items(id: &Ident, movable: bool, config: &IncludeCppConfig) -> Vec<Item> {
     if config.exclude_impls {
         return vec![];
     }
-    vec![
+    let mut results = vec![
         Item::Impl(parse_quote! {
             impl UniquePtr<#id> {}
         }),
@@ -29,8 +29,11 @@ pub(crate) fn create_impl_items(id: &Ident, config: &IncludeCppConfig) -> Vec<It
         Item::Impl(parse_quote! {
             impl WeakPtr<#id> {}
         }),
-        Item::Impl(parse_quote! {
+    ];
+    if movable {
+        results.push(Item::Impl(parse_quote! {
             impl CxxVector<#id> {}
-        }),
-    ]
+        }))
+    }
+    results
 }

engine/src/conversion/codegen_rs/mod.rs

@@ -521,6 +521,7 @@ impl<'a> RsCodeGenerator<'a> {
                     &name,
                     id,
                     analysis.kind,
+                    analysis.movable,
                     || Some((Item::Struct(details.item), doc_attr)),
                     associated_methods,
                     layout,
@@ -532,6 +533,7 @@ impl<'a> RsCodeGenerator<'a> {
                     &name,
                     id,
                     TypeKind::Pod,
+                    true,
                     || Some((Item::Enum(item), doc_attr)),
                     associated_methods,
                     None,
@@ -541,6 +543,7 @@ impl<'a> RsCodeGenerator<'a> {
                 &name,
                 id,
                 TypeKind::Abstract,
+                false, // assume for now that these types can't be kept in a Vector
                 || None,
                 associated_methods,
                 None,
@@ -719,7 +722,9 @@ impl<'a> RsCodeGenerator<'a> {
         });
         RsCodegenResult {
             extern_c_mod_items,
-            bridge_items: create_impl_items(&cpp_id, self.config),
+            // For now we just assume we can't keep subclasses in vectors.
+            // That's the reason for the 'false'
+            bridge_items: create_impl_items(&cpp_id, false, self.config),
             bindgen_mod_items,
             materializations: vec![Use::Custom(Box::new(parse_quote! {
                 pub use cxxbridge::#cpp_id;
@@ -810,6 +815,7 @@ impl<'a> RsCodeGenerator<'a> {
         name: &QualifiedName,
         id: Ident,
         type_kind: TypeKind,
+        movable: bool,
         item_creator: F,
         associated_methods: &HashMap<QualifiedName, Vec<SuperclassMethod>>,
         layout: Option<Layout>,
@@ -857,7 +863,7 @@ impl<'a> RsCodeGenerator<'a> {
                 RsCodegenResult {
                     global_items: self.generate_extern_type_impl(type_kind, name),
                     impl_entry: None,
-                    bridge_items: create_impl_items(&id, self.config),
+                    bridge_items: create_impl_items(&id, movable, self.config),
                     extern_c_mod_items: vec![self.generate_cxxbridge_type(name, true, None)],
                     bindgen_mod_items,
                     materializations,

engine/src/conversion/parse/bindgen_semantic_attributes.rs

@@ -20,7 +20,7 @@ use syn::{
 };
 
 use crate::conversion::{
-    api::{CppVisibility, Layout, References, Virtualness},
+    api::{CppVisibility, Layout, References, SpecialMemberKind, Virtualness},
     convert_error::{ConvertErrorWithContext, ErrorContext},
     ConvertError,
 };
@@ -121,14 +121,16 @@ impl BindgenSemanticAttributes {
         self.string_if_present("original_name")
     }
 
-    fn get_bindgen_special_member_annotation(&self) -> Option<String> {
+    /// Whether this is a move constructor or other special member.
+    pub(super) fn special_member_kind(&self) -> Option<SpecialMemberKind> {
         self.string_if_present("special_member")
-    }
-
-    /// Whether this is a move constructor.
-    pub(super) fn is_move_constructor(&self) -> bool {
-        self.get_bindgen_special_member_annotation()
-            .map_or(false, |val| val == "move_ctor")
+            .map(|kind| match kind.as_str() {
+                "default_ctor" => SpecialMemberKind::DefaultConstructor,
+                "copy_ctor" => SpecialMemberKind::CopyConstructor,
+                "move_ctor" => SpecialMemberKind::MoveConstructor,
+                "dtor" => SpecialMemberKind::Destructor,
+                _ => panic!("unexpected special_member_kind"),
+            })
     }
 
     /// Any reference parameters or return values.

engine/src/conversion/parse/parse_foreign_mod.rs

@@ -83,13 +83,14 @@ impl ParseForeignMod {
                     vis: item.vis,
                     virtualness: annotations.get_virtualness(),
                     cpp_vis: annotations.get_cpp_visibility(),
-                    is_move_constructor: annotations.is_move_constructor(),
+                    special_member: annotations.special_member_kind(),
                     unused_template_param: annotations
                         .has_attr("unused_template_param_in_arg_or_return"),
                     references: annotations.get_reference_parameters_and_return(),
                     original_name: annotations.get_original_name(),
                     synthesized_this_type: None,
                     synthesis: None,
+                    is_deleted: annotations.has_attr("deleted"),
                 });
                 Ok(())
             }
@@ -113,13 +114,9 @@ impl ParseForeignMod {
         };
         for i in imp.items {
             if let ImplItem::Method(itm) = i {
-                let effective_fun_name = if itm.sig.ident == "new" {
-                    ty_id.clone()
-                } else {
-                    match get_called_function(&itm.block) {
-                        Some(id) => id.clone(),
-                        None => itm.sig.ident,
-                    }
+                let effective_fun_name = match get_called_function(&itm.block) {
+                    Some(id) => id.clone(),
+                    None => itm.sig.ident,
                 };
                 self.method_receivers.insert(
                     effective_fun_name,