From c12950ef2a58983dff4fb6fac5e9fee357b7cb2e Mon Sep 17 00:00:00 2001
From: Martin Boehme <mboehme@google.com>
Date: Fri, 30 Apr 2021 15:21:30 +0200
Subject: [PATCH] Don't generate bindings for move constructors.

For details, see https://github.com/google/autocxx/issues/456.

I'm duplicating some code from `get_bindgen_original_name_annotation` in
`get_bindgen_special_member_annotation` because I plan to move the
former function to a different module when I implement support for
nested types.
---
 engine/src/conversion/analysis/fun/mod.rs | 28 +++++++++++++++++++++++
 engine/src/conversion/convert_error.rs    |  2 ++
 engine/src/integration_tests.rs           | 17 ++++++++++----
 3 files changed, 42 insertions(+), 5 deletions(-)

diff --git a/engine/src/conversion/analysis/fun/mod.rs b/engine/src/conversion/analysis/fun/mod.rs
index 5e1e672be..1fea760d4 100644
--- a/engine/src/conversion/analysis/fun/mod.rs
+++ b/engine/src/conversion/analysis/fun/mod.rs
@@ -482,6 +482,13 @@ impl<'a> FnAnalyzer<'a> {
             return Err(contextualize_error(ConvertError::UnusedTemplateParam));
         }
 
+        // Reject move constructors.
+        if Self::is_move_constructor(&fun) {
+            return Err(contextualize_error(
+                ConvertError::MoveConstructorUnsupported,
+            ));
+        }
+
         // Analyze the return type, just as we previously did for the
         // parameters.
         let mut return_analysis = if let FnKind::Method(ref self_ty, MethodKind::Constructor) = kind
@@ -818,6 +825,27 @@ impl<'a> FnAnalyzer<'a> {
             .next()
     }
 
+    fn get_bindgen_special_member_annotation(fun: &ForeignItemFn) -> Option<String> {
+        fun.attrs
+            .iter()
+            .filter_map(|a| {
+                if a.path.is_ident("bindgen_special_member") {
+                    let r: Result<LitStr, syn::Error> = a.parse_args();
+                    match r {
+                        Ok(ls) => Some(ls.value()),
+                        Err(_) => None,
+                    }
+                } else {
+                    None
+                }
+            })
+            .next()
+    }
+
+    fn is_move_constructor(fun: &ForeignItemFn) -> bool {
+        Self::get_bindgen_special_member_annotation(fun).map_or(false, |val| val == "move_ctor")
+    }
+
     fn get_reference_parameters_and_return(fun: &ForeignItemFn) -> (HashSet<Ident>, bool) {
         let mut ref_params = HashSet::new();
         let mut ref_return = false;
diff --git a/engine/src/conversion/convert_error.rs b/engine/src/conversion/convert_error.rs
index fae4e8843..3b03345b1 100644
--- a/engine/src/conversion/convert_error.rs
+++ b/engine/src/conversion/convert_error.rs
@@ -47,6 +47,7 @@ pub enum ConvertError {
     TooManyUnderscores,
     UnknownDependentType,
     IgnoredDependent,
+    MoveConstructorUnsupported,
 }
 
 fn format_maybe_identifier(id: &Option<Ident>) -> String {
@@ -86,6 +87,7 @@ impl Display for ConvertError {
             ConvertError::TooManyUnderscores => write!(f, "Names containing __ are reserved by C++ so not acceptable to cxx")?,
             ConvertError::UnknownDependentType => write!(f, "This item relies on a type not known to autocxx.")?,
             ConvertError::IgnoredDependent => write!(f, "This item depends on some other type which autocxx could not generate.")?,
+            ConvertError::MoveConstructorUnsupported => write!(f, "This is a move constructor, for which we currently cannot generate bindings.")?,
         }
         Ok(())
     }
diff --git a/engine/src/integration_tests.rs b/engine/src/integration_tests.rs
index 6a3810544..54a1e511b 100644
--- a/engine/src/integration_tests.rs
+++ b/engine/src/integration_tests.rs
@@ -5065,11 +5065,18 @@ fn test_deleted_function() {
 }
 
 #[test]
-#[ignore] // https://github.com/google/autocxx/issues/426
-fn test_implicitly_deleted_copy_constructor() {
-    // We shouldn't generate bindings for implicitly deleted functions.
-    // The test is successful if the bindings compile, i.e. if autocxx doesn't
-    // attempt to call the implicitly deleted copy constructor.
+fn test_ignore_move_constructor() {
+    // Test that we don't generate bindings for move constructors and,
+    // specifically, don't erroneously import them as copy constructors.
+    // We used to do this because bindgen creates the same Rust signature for
+    // move constructors and for copy constructors.
+    // The way this tests works is a bit subtle.  Declaring a move constructor
+    // causes the copy constructor to be implicitly deleted (unless it is]
+    // explicitly declared). If we erroneously tried to create a binding for the
+    // move constructor (because the signature led us to believe it is a copy
+    // constructor), the generated C++ code would therefore try to call the
+    // deleted copy constructor, which would result in a compile error.
+    // The test is therefore successful if the bindings compile.
     let hdr = indoc! {"
         class A {
         public: