Merge pull request #1164 from google/cppref-changes

Rewrite reference_wrapper.rs to be structs.

.github/workflows/ci.yml

@@ -142,6 +142,7 @@ jobs:
         run: cargo build
       - name: Build reference-wrappers example
         working-directory: ./examples/reference-wrappers
+        if: matrix.rust == 'nightly'
         run: cargo build
         # We do not build the LLVM example because even 'apt-get install llvm-13-dev'
         # does not work to install the LLVM 13 headers.

engine/Cargo.toml

@@ -51,6 +51,9 @@ regex = "1.5"
 indexmap = "1.8"
 prettyplease = { version = "0.1.15", features = ["verbatim"] }
 
+[build-dependencies]
+rustc_version = "0.4"
+
 [dependencies.syn]
 version = "1.0.39"
 features = [ "full", "printing" ]

engine/build.rs

@@ -0,0 +1,15 @@
+// Copyright 2022 Google LLC
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// https://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or https://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+use rustc_version::{version_meta, Channel};
+
+fn main() {
+    if version_meta().unwrap().channel == Channel::Nightly {
+        println!("cargo:rustc-cfg=nightly");
+    }
+}

engine/src/conversion/codegen_rs/fun_codegen.rs

@@ -389,30 +389,23 @@ impl<'a> FnGenerator<'a> {
             .map(|pd| pd.conversion.is_a_pointer())
             .unwrap_or(Pointerness::Not);
         let ty = impl_block_type_name.get_final_ident();
-        let ty = if self.reference_wrappers {
-            match receiver_pointerness {
-                Pointerness::MutPtr => ImplBlockKey {
-                    ty: parse_quote! {
-                        CppMutRef< 'a, #ty>
-                    },
-                    lifetime: Some(parse_quote! { 'a }),
-                },
-                Pointerness::ConstPtr => ImplBlockKey {
-                    ty: parse_quote! {
-                        CppRef< 'a, #ty>
-                    },
-                    lifetime: Some(parse_quote! { 'a }),
+        let ty = match receiver_pointerness {
+            Pointerness::MutPtr if self.reference_wrappers => ImplBlockKey {
+                ty: parse_quote! {
+                    #ty
                 },
-                Pointerness::Not => ImplBlockKey {
-                    ty: parse_quote! { # ty },
-                    lifetime: None,
+                lifetime: Some(parse_quote! { 'a }),
+            },
+            Pointerness::ConstPtr if self.reference_wrappers => ImplBlockKey {
+                ty: parse_quote! {
+                    #ty
                 },
-            }
-        } else {
-            ImplBlockKey {
+                lifetime: Some(parse_quote! { 'a }),
+            },
+            _ => ImplBlockKey {
                 ty: parse_quote! { # ty },
                 lifetime: None,
-            }
+            },
         };
         Box::new(ImplBlockDetails {
             item: ImplItem::Method(parse_quote! {

engine/src/conversion/codegen_rs/function_wrapper_rs.rs

@@ -172,16 +172,16 @@ impl TypeConversionPolicy {
                     _ => panic!("Not a pointer"),
                 };
                 let (ty, wrapper_name) = if is_mut {
-                    (parse_quote! { CppMutRef<'a, #ty> }, "CppMutRef")
+                    (parse_quote! { autocxx::CppMutRef<'a, #ty> }, "CppMutRef")
                 } else {
-                    (parse_quote! { CppRef<'a, #ty> }, "CppRef")
+                    (parse_quote! { autocxx::CppRef<'a, #ty> }, "CppRef")
                 };
                 let wrapper_name = make_ident(wrapper_name);
                 RustParamConversion::Param {
                     ty,
                     local_variables: Vec::new(),
                     conversion: quote! {
-                        #wrapper_name (#var, std::marker::PhantomData)
+                        autocxx::#wrapper_name::from_ptr (#var)
                     },
                     conversion_requires_unsafe: false,
                 }
@@ -194,15 +194,21 @@ impl TypeConversionPolicy {
                     _ => panic!("Not a pointer"),
                 };
                 let ty = if is_mut {
-                    parse_quote! { &mut CppMutRef<'a, #ty> }
+                    parse_quote! { &mut autocxx::CppMutRef<'a, #ty> }
                 } else {
-                    parse_quote! { &CppRef<'a, #ty> }
+                    parse_quote! { &autocxx::CppRef<'a, #ty> }
                 };
                 RustParamConversion::Param {
                     ty,
                     local_variables: Vec::new(),
-                    conversion: quote! {
-                        #var .0
+                    conversion: if is_mut {
+                        quote! {
+                            #var .as_mut_ptr()
+                        }
+                    } else {
+                        quote! {
+                            #var .as_ptr()
+                        }
                     },
                     conversion_requires_unsafe: false,
                 }

engine/src/conversion/codegen_rs/mod.rs

@@ -137,91 +137,6 @@ fn get_string_items() -> Vec<Item> {
     .to_vec()
 }
 
-fn get_cppref_items() -> Vec<Item> {
-    [
-        Item::Struct(parse_quote! {
-            #[repr(transparent)]
-            pub struct CppRef<'a, T>(pub *const T, pub ::std::marker::PhantomData<&'a T>);
-        }),
-        Item::Impl(parse_quote! {
-            impl<'a, T> autocxx::CppRef<'a, T> for CppRef<'a, T> {
-                fn as_ptr(&self) -> *const T {
-                    self.0
-                }
-            }
-        }),
-        Item::Struct(parse_quote! {
-            #[repr(transparent)]
-            pub struct CppMutRef<'a, T>(pub *mut T, pub ::std::marker::PhantomData<&'a T>);
-        }),
-        Item::Impl(parse_quote! {
-            impl<'a, T> autocxx::CppRef<'a, T> for CppMutRef<'a, T> {
-                fn as_ptr(&self) -> *const T {
-                    self.0
-                }
-            }
-        }),
-        Item::Impl(parse_quote! {
-            impl<'a, T> autocxx::CppMutRef<'a, T> for CppMutRef<'a, T> {
-                fn as_mut_ptr(&self) -> *mut T {
-                    self.0
-                }
-            }
-        }),
-        Item::Impl(parse_quote! {
-            impl<'a, T: ::cxx::private::UniquePtrTarget> CppMutRef<'a, T> {
-                /// Create a const C++ reference from this mutable C++ reference.
-                pub fn as_cpp_ref(&self) -> CppRef<'a, T> {
-                    use autocxx::CppRef;
-                    CppRef(self.as_ptr(), ::std::marker::PhantomData)
-                }
-            }
-        }),
-        Item::Struct(parse_quote! {
-            /// "Pins" a `UniquePtr` to an object, so that C++-compatible references can be created.
-            /// See [`::autocxx::CppPin`]
-            #[repr(transparent)]
-            pub struct CppUniquePtrPin<T: ::cxx::private::UniquePtrTarget>(::cxx::UniquePtr<T>);
-        }),
-        Item::Impl(parse_quote! {
-            impl<'a, T: 'a + ::cxx::private::UniquePtrTarget> autocxx::CppPin<'a, T> for CppUniquePtrPin<T>
-            {
-                type CppRef = CppRef<'a, T>;
-                type CppMutRef = CppMutRef<'a, T>;
-                fn as_ptr(&self) -> *const T {
-                    // TODO add as_ptr to cxx to avoid the ephemeral reference
-                    self.0.as_ref().unwrap() as *const T
-                }
-                fn as_mut_ptr(&mut self) -> *mut T {
-                    unsafe { ::std::pin::Pin::into_inner_unchecked(self.0.as_mut().unwrap()) as *mut T  }
-                }
-                fn as_cpp_ref(&self) -> Self::CppRef {
-                    CppRef(self.as_ptr(), ::std::marker::PhantomData)
-                }
-                fn as_cpp_mut_ref(&mut self) -> Self::CppMutRef {
-                    CppMutRef(self.as_mut_ptr(), ::std::marker::PhantomData)
-                }
-            }
-        }),
-        Item::Impl(parse_quote! {
-            impl<T: ::cxx::private::UniquePtrTarget> CppUniquePtrPin<T> {
-                pub fn new(item: ::cxx::UniquePtr<T>) -> Self {
-                    Self(item)
-                }
-            }
-        }),
-        Item::Fn(parse_quote! {
-            /// Pin this item so that we can create C++ references to it.
-            /// This makes it impossible to hold Rust references because Rust
-            /// references are fundamentally incompatible with C++ references.
-            pub fn cpp_pin_uniqueptr<T: ::cxx::private::UniquePtrTarget> (item: ::cxx::UniquePtr<T>) -> CppUniquePtrPin<T> {
-                CppUniquePtrPin::new(item)
-            }
-        })
-    ]
-    .to_vec()
-}
-
 /// Type which handles generation of Rust code.
 /// In practice, much of the "generation" involves connecting together
 /// existing lumps of code within the Api structures.
@@ -314,9 +229,6 @@ impl<'a> RsCodeGenerator<'a> {
         let mut extern_rust_mod_items = extern_rust_mod_items.into_iter().flatten().collect();
         // And a list of global items to include at the top level.
         let mut all_items: Vec<Item> = all_items.into_iter().flatten().collect();
-        if self.config.unsafe_policy.requires_cpprefs() {
-            all_items.append(&mut get_cppref_items())
-        }
         // And finally any C++ we need to generate. And by "we" I mean autocxx not cxx.
         let has_additional_cpp_needs = additional_cpp_needs.into_iter().any(std::convert::identity);
         extern_c_mod_items.extend(self.build_include_foreign_items(has_additional_cpp_needs));
@@ -459,9 +371,6 @@ impl<'a> RsCodeGenerator<'a> {
         if !self.config.exclude_utilities() {
             imports_from_super.push("ToCppString");
         }
-        if self.config.unsafe_policy.requires_cpprefs() {
-            imports_from_super.extend(["CppRef", "CppMutRef"]);
-        }
         let imports_from_super = imports_from_super.into_iter().map(make_ident);
         let super_duper = std::iter::repeat(make_ident("super")); // I'll get my coat
         let supers = super_duper.clone().take(ns.depth() + 2);

engine/src/lib.rs

@@ -125,6 +125,8 @@ pub enum Error {
     NoAutoCxxInc,
     #[error(transparent)]
     Conversion(conversion::ConvertError),
+    #[error("Using `unsafe_references_wrapped` requires the Rust nightly `arbitrary_self_types` feature")]
+    WrappedReferencesButNoArbitrarySelfTypes,
 }
 
 /// Result type.
@@ -398,6 +400,14 @@ impl IncludeCppEngine {
             State::Generated(_) => panic!("Only call generate once"),
         }
 
+        if matches!(
+            self.config.unsafe_policy,
+            UnsafePolicy::ReferencesWrappedAllFunctionsSafe
+        ) && cfg!(not(nightly))
+        {
+            return Err(Error::WrappedReferencesButNoArbitrarySelfTypes);
+        }
+
         let mod_name = self.config.get_mod_name();
         let mut builder = self.make_bindgen_builder(&inc_dirs, extra_clang_args);
         if let Some(dep_recorder) = dep_recorder {

examples/reference-wrappers/build.rs

@@ -10,9 +10,11 @@ fn main() -> miette::Result<()> {
     let path = std::path::PathBuf::from("src");
     let mut b = autocxx_build::Builder::new("src/main.rs", &[&path]).build()?;
     b.flag_if_supported("-std=c++14")
-    .file("src/input.cc").compile("autocxx-reference-wrapper-example");
+        .file("src/input.cc")
+        .compile("autocxx-reference-wrapper-example");
 
     println!("cargo:rerun-if-changed=src/main.rs");
     println!("cargo:rerun-if-changed=src/input.h");
+
     Ok(())
 }

examples/reference-wrappers/src/main.rs

@@ -23,11 +23,16 @@
 // especially in the absence of the Rust "arbitrary self types"
 // feature.
 
+// Necessary to be able to call methods on reference wrappers.
+// For that reason, this example only builds on nightly Rust.
+#![feature(arbitrary_self_types)]
+
 use autocxx::prelude::*;
 
 include_cpp! {
     #include "input.h"
     // This next line enables C++ reference wrappers
+    // This is what requires the 'arbitrary_self_types' feature.
     safety!(unsafe_references_wrapped)
     generate!("Goat")
     generate!("Field")
@@ -47,7 +52,7 @@ fn main() {
     // However, as soon as we want to pass a reference to the field
     // back to C++, we have to ensure we have no Rust references
     // in existence. So: we imprison the object in a "CppPin":
-    let field = ffi::cpp_pin_uniqueptr(field);
+    let field = CppUniquePtrPin::new(field);
     // We can no longer take Rust references to the field...
     //   let _field_rust_ref = field.as_ref();
     // However, we can take C++ references. And use such references
@@ -58,7 +63,7 @@ fn main() {
     assert_eq!(
         another_goat
             .describe() // returns a UniquePtr<CxxString>, there
-                // are no Rust or C++ references involved at this point.
+            // are no Rust or C++ references involved at this point.
             .as_ref()
             .unwrap()
             .to_string_lossy(),

integration-tests/Cargo.toml

@@ -37,6 +37,7 @@ indoc = "1.0"
 log = "0.4"
 cxx = "1.0.78"
 itertools = "0.10"
+rustc_version = "0.4.0"
 
 [dependencies.syn]
 version = "1.0.39"