Merge pull request #16 from google/pass_nonpod_by_value

Pass nonpod by value

Cargo.toml

@@ -14,7 +14,7 @@
 
 [package]
 name = "autocxx"
-version = "0.3.1"
+version = "0.3.2"
 authors = ["Adrian Taylor <adetaylor@chromium.org>"]
 license = "MIT OR Apache-2.0"
 description = "Safe autogenerated interop between Rust and C++"
@@ -27,7 +27,7 @@ categories = ["development-tools::ffi", "api-bindings"]
 proc-macro = true
 
 [dependencies]
-autocxx-engine = { path="engine", version="0.3.1" }
+autocxx-engine = { path="engine", version="0.3.2" }
 proc-macro-error = "1.0"
 
 [dependencies.syn]

README.md

@@ -77,8 +77,8 @@ The project also contains test code which does this end-to-end, for all sorts of
 | Typedefs | - |
 | Structs containing UniquePtr | Works |
 | Structs containing strings | Works (opaque only) |
-| Passing opaque structs (owned by UniquePtr) into C++ APIs which take them by value | - |
-| Constructors/make_unique | Sort of. A function called {type}_make_unique is generated which is not the eventual intended syntax. |
+| Passing opaque structs (owned by UniquePtr) into C++ APIs which take them by value | Works, not yet for methods |
+| Constructors/make_unique | Works, though probably many problems |
 | Field access to opaque objects via UniquePtr | - |
 | Reference counting | - |
 | std::optional | - |

demo/Cargo.toml

@@ -19,8 +19,8 @@ authors = ["Adrian Taylor <adetaylor@chromium.org>"]
 edition = "2018"
 
 [dependencies]
-cxx = { version="0.5.0", path="../../cxx" }
-autocxx = { path = "..", version="0.3.1" }
+cxx = { version="0.5.1", path="../../cxx" }
+autocxx = { path = "..", version="0.3.2" }
 
 [build-dependencies]
-autocxx-build = { path = "../gen/build", version="0.3.1" }
+autocxx-build = { path = "../gen/build", version="0.3.2" }

engine/Cargo.toml

@@ -14,7 +14,7 @@
 
 [package]
 name = "autocxx-engine"
-version = "0.3.1"
+version = "0.3.2"
 authors = ["Adrian Taylor <adetaylor@chromium.org>"]
 license = "MIT OR Apache-2.0"
 description = "Safe autogenerated interop between Rust and C++"
@@ -32,7 +32,7 @@ lazy_static = "1.4"
 indoc = "1.0"
 bindgen = "0.55"
 itertools = "0.9"
-cxx-gen = { version="0.5.0", path="../../cxx/gen/lib" }
+cxx-gen = { version="0.5.1", path="../../cxx/gen/lib" }
 
 [dependencies.syn]
 version = "1.0.39"
@@ -50,4 +50,4 @@ autocxx-build = { path="../gen/build" }
 # by the trybuild test system...
 autocxx = { path=".." }
 link-cplusplus = "1.0"
-cxx = { version="0.5.0", path="../../cxx" }
+cxx = { version="0.5.1", path="../../cxx" }

engine/src/additional_cpp_generator.rs

@@ -12,25 +12,109 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-use crate::TypeName;
+use crate::types::TypeName;
 use itertools::Itertools;
+use std::collections::HashMap;
+use syn::{Ident, Type};
+
+enum ArgumentConversionType {
+    None,
+    FromUniquePtrToValue,
+    FromValueToUniquePtr,
+}
+
+pub(crate) struct ArgumentConversion {
+    unwrapped_type: Type,
+    conversion: ArgumentConversionType,
+}
+
+impl ArgumentConversion {
+    pub(crate) fn unconverted(ty: Type) -> Self {
+        ArgumentConversion {
+            unwrapped_type: ty,
+            conversion: ArgumentConversionType::None,
+        }
+    }
+
+    pub(crate) fn to_unique_ptr(ty: Type) -> Self {
+        ArgumentConversion {
+            unwrapped_type: ty,
+            conversion: ArgumentConversionType::FromValueToUniquePtr,
+        }
+    }
+
+    pub(crate) fn from_unique_ptr(ty: Type) -> Self {
+        ArgumentConversion {
+            unwrapped_type: ty,
+            conversion: ArgumentConversionType::FromUniquePtrToValue,
+        }
+    }
+
+    pub(crate) fn work_needed(&self) -> bool {
+        match self.conversion {
+            ArgumentConversionType::None => false,
+            _ => true,
+        }
+    }
+
+    fn unconverted_type(&self) -> String {
+        match self.conversion {
+            ArgumentConversionType::FromUniquePtrToValue => self.wrapped_type(),
+            _ => self.unwrapped_type_as_string(),
+        }
+    }
+
+    fn converted_type(&self) -> String {
+        match self.conversion {
+            ArgumentConversionType::FromValueToUniquePtr => self.wrapped_type(),
+            _ => self.unwrapped_type_as_string(),
+        }
+    }
+
+    fn unwrapped_type_as_string(&self) -> String {
+        TypeName::from_type(&self.unwrapped_type)
+            .to_cpp_name()
+            .to_string()
+    }
+
+    fn wrapped_type(&self) -> String {
+        format!("std::unique_ptr<{}>", self.unwrapped_type_as_string())
+    }
+
+    fn conversion(&self, var_name: &str) -> String {
+        match self.conversion {
+            ArgumentConversionType::None => var_name.to_string(),
+            ArgumentConversionType::FromUniquePtrToValue => format!("std::move(*{})", var_name),
+            ArgumentConversionType::FromValueToUniquePtr => format!(
+                "std::make_unique<{}>({})",
+                self.unconverted_type(),
+                var_name
+            ),
+        }
+    }
+}
 
 /// Instructions for new C++ which we need to generate.
 pub(crate) enum AdditionalNeed {
     MakeUnique(TypeName, Vec<TypeName>),
+    ByValueWrapper(Ident, Option<ArgumentConversion>, Vec<ArgumentConversion>),
 }
 
 struct AdditionalFunction {
     declaration: String,
     definition: String,
     name: String,
+    suppress_older: Option<String>,
+    rename: Option<(String, String)>,
 }
 
 /// Details of additional generated C++.
 pub(crate) struct AdditionalCpp {
     pub(crate) declarations: String,
     pub(crate) definitions: String,
     pub(crate) extra_allowlist: Vec<String>,
+    pub(crate) extra_blocklist: Vec<String>,
+    pub(crate) renames: HashMap<String, String>,
 }
 
 /// Generates additional C++ glue functions needed by autocxx.
@@ -58,6 +142,9 @@ impl AdditionalCppGenerator {
         for need in additions {
             match need {
                 AdditionalNeed::MakeUnique(ty, args) => self.generate_make_unique(&ty, &args),
+                AdditionalNeed::ByValueWrapper(id, ret, tys) => {
+                    self.generate_by_value_wrapper(&id, &ret, &tys)
+                }
             }
         }
     }
@@ -75,10 +162,22 @@ impl AdditionalCppGenerator {
                 .iter()
                 .map(|x| x.name.to_string())
                 .collect();
+            let extra_blocklist = self
+                .additional_functions
+                .iter()
+                .filter_map(|x| x.suppress_older.clone())
+                .collect();
+            let renames = self
+                .additional_functions
+                .iter()
+                .filter_map(|x| x.rename.clone())
+                .collect();
             Some(AdditionalCpp {
                 declarations,
                 definitions,
                 extra_allowlist,
+                extra_blocklist,
+                renames,
             })
         }
     }
@@ -98,11 +197,11 @@ impl AdditionalCppGenerator {
     }
 
     fn generate_make_unique(&mut self, ty: &TypeName, constructor_arg_types: &[TypeName]) {
-        let name = format!("{}_make_unique", ty.to_cxx_name());
+        let name = format!("{}_make_unique", ty.to_cpp_name());
         let constructor_args = constructor_arg_types
             .iter()
             .enumerate()
-            .map(|(counter, ty)| format!("{} arg{}", ty.to_cxx_name(), counter))
+            .map(|(counter, ty)| format!("{} arg{}", ty.to_cpp_name(), counter))
             .join(", ");
         let declaration = format!("std::unique_ptr<{}> {}({})", ty, name, constructor_args);
         let arg_list = constructor_arg_types
@@ -119,6 +218,45 @@ impl AdditionalCppGenerator {
             name,
             declaration,
             definition,
+            suppress_older: None,
+            rename: None,
+        })
+    }
+
+    fn generate_by_value_wrapper(
+        &mut self,
+        ident: &Ident,
+        ret: &Option<ArgumentConversion>,
+        arg_types: &[ArgumentConversion],
+    ) {
+        let name = format!("{}_up_wrapper", ident.to_string());
+        let args = arg_types
+            .iter()
+            .enumerate()
+            .map(|(counter, ty)| format!("{} arg{}", ty.unconverted_type(), counter))
+            .join(", ");
+        let ret_type = ret
+            .as_ref()
+            .map_or("void".to_string(), |x| x.converted_type());
+        let declaration = format!("{} {}({})", ret_type, name, args);
+        let arg_list = arg_types
+            .iter()
+            .enumerate()
+            .map(|(counter, conv)| conv.conversion(&format!("arg{}", counter)))
+            .join(", ");
+        let underlying_function_call = format!("{}({})", ident.to_string(), arg_list);
+        let underlying_function_call = match ret {
+            None => underlying_function_call,
+            Some(ret) => format!("return {}", ret.conversion(&underlying_function_call)),
+        };
+        let definition = format!("{} {{ {}; }}", declaration, underlying_function_call,);
+        let declaration = format!("{};", declaration);
+        self.additional_functions.push(AdditionalFunction {
+            name: name.clone(),
+            declaration,
+            definition,
+            suppress_older: Some(ident.to_string()),
+            rename: Some((name, ident.to_string())),
         })
     }
 }