Cease to use bindgen in a procedural macro

Cargo.toml

@@ -29,7 +29,7 @@ autocxx-engine = { path="engine", version="0.5.0" }
 cxx = "1.0.20" # needed because expansion of type_id refers to ::cxx
 
 [workspace]
-members = ["engine", "gen/cmd", "gen/build", "macro", "demo"]
+members = ["parser", "engine", "gen/cmd", "gen/build", "macro", "demo"]
 
 # [patch.crates-io]
 # cxx = { path="../cxx" }

README.md

@@ -35,22 +35,19 @@ See [demo/src/main.rs](demo/src/main.rs) for a real example.
 
 The existing cxx facilities are used to allow safe ownership of C++ types from Rust; specifically things like `std::unique_ptr` and `std::string` - so the Rust code should not typically require use of unsafe code, unlike with normal `bindgen` bindings.
 
-The macro and code generator will both need to know the include path to be passed to bindgen. At the moment, this is passed in via an
-environment variable, `AUTOCXX_INC`. See the [demo/build.rs](demo/build.rs) file for details.
-
 # How it works
 
-It is effectively a three-stage procedural macro, which:
+Before building the Rust code, you must run a code generator (typically run in a `build.rs` for a Cargo setup.)
+
+This:
 
 * First, runs `bindgen` to generate some bindings (with all the usual `unsafe`, `#[repr(C)]` etc.)
 * Second, interprets and converts them to bindings suitable for `cxx::bridge`.
-* Thirdly, runs `cxx::bridge` to convert them to Rust code.
+* Thirdly, runs `cxx::bridge` to create the C++ bindings.
+* Fourthly, writes out a `.rs` file with the Rust bindings.
 
-The same code can be passed through tools that generate .cc and .h bindings too:
-
-* First, runs `bindgen` to generate some bindings (with all the usual `unsafe`, `#[repr(C)]` etc.) - in exactly the same way as above.
-* Second, interprets and converts them to bindings suitable for `cxx::bridge` - in the same way as above.
-* Thirdly, runs the codegen code from `cxx` to generate .cc and .h files
+When building your Rust code, the procedural macro boils down to an `include!` macro that pulls in the
+generated Rust code.
 
 # Current state of affairs
 
@@ -107,30 +104,37 @@ This crate mostly intends to follow the lead of the `cxx` crate in where and whe
 
 If your project is 90% Rust code, with small bits of C++, don't use this crate. You need something where all C++ interaction is marked with big red "this is terrifying" flags. This crate is aimed at cases where there's 90% C++ and small bits of Rust, and so we want the Rust code to be pragmatically reviewable without the signal:noise ratio of `unsafe` in the Rust code becoming so bad that `unsafe` loses all value.
 
+See [safety!] in the documentation for more details.
+
 # Build environment
 
 Because this uses `bindgen`, and `bindgen` may depend on the state of your system C++ headers, it is somewhat sensitive. It requires [llvm to be installed due to bindgen](https://rust-lang.github.io/rust-bindgen/requirements.html)
 
-As with `cxx`, this generates both Rust and C++ side bindings code. The Rust code is simply
-and transparently generated at build time by the `include_cpp!` procedural macro. But you'll
+As with `cxx`, this generates both Rust and C++ side bindings code. You'll
 need to take steps to generate the C++ code: either by using the `build.rs` integration within
-`autocxx_build`, or the command line utility within `autocxx_gen`.
-
-# Configuring the build
-
-This runs `bindgen` within a procedural macro. There are limited opportunities to pass information into procedural macros, yet bindgen needs to know a lot about the build environment.
-
-The plan is:
-* The Rust code itself will specify the include file(s) and allowlist by passing them into the macro. This is the sort of thing that developers within an existing C++ codebase would specify in C++ (give or take) so it makes sense for it to be specific in the Rust code.
-* However, all build settings (e.g. bindgen compiler configuration, include path etc.) will be passed into the macro by means of environment variables. The build environment should set these before building the code. (An alternative means will be provided to pass these into the C++ code generator tools.)
+`autocxx_build`, or the command line utility within `autocxx_gen`. Either way, you'll need
+to specify the Rust file(s) which have `include_cpp` macros in place, and suitable corresponding
+C++ and Rust code will be generated.
+
+When you come to build your Rust code, it will expand to an `include!` macro which will pull
+in the generated Rust code. For this to work, you need to specify an `AUTOCXX_RS` environment
+variable such that the macro can discover the location of the .rs file which was generated.
+If you use the `build.rs` cargo integration, this happens automatically. You'll also need
+to ensure that you build and link against the C++ code. Again, if you use the Cargo integrationm
+and follow the pattern of the `demo` example, this is fairly automatic because we use
+`cc` for this.
+
+You'll also want to ensure that the code generation (both Rust and C++ code) happens whenever
+any included header file changes. This is _not_ yet handled automatically even by our
+`build.rs` integration, but is coming soon.
 
 # Directory structure
 
 * `demo` - a demo example
-* `engine` - all the core code. Currently a Rust library, but we wouldn't want to support
-  these APIs for external users, so maybe it needs to be a directory of code symlinked
-  into all the other sub-crates. All the following three sub-crates are thin wrappers
-  for part of this engine. This also contains the test code.
+* `parser` - code which parses a single `include_cpp!` macro. Used by both the macro
+  (which doesn't do much) and the code generator (which does much more, by means of
+  `engine` below)
+* `engine` - all the core code for actual code generation.
 * `macro` - the procedural macro which expands the Rust code.
 * `gen/build` - a library to be used from `build.rs` scripts to generate .cc and .h
   files from an `include_cxx` section.

engine/Cargo.toml

@@ -39,6 +39,7 @@ cc = { version = "1.0", optional = true }
 unzip-n = "0.1.2"
 cxx-gen = "0.7.20"
 cxx = "1.0.20"
+autocxx-parser = { path="../parser" }
 
 [dependencies.syn]
 version = "1.0.39"

engine/src/additional_cpp_generator.rs

@@ -14,7 +14,7 @@
 
 use crate::{
     function_wrapper::{FunctionWrapper, FunctionWrapperPayload},
-    type_database::TypeDatabase,
+    type_to_cpp::type_to_cpp,
     types::TypeName,
 };
 use itertools::Itertools;
@@ -88,20 +88,16 @@ impl AdditionalCppGenerator {
         }
     }
 
-    pub(crate) fn add_needs(
-        &mut self,
-        additions: Vec<AdditionalNeed>,
-        type_database: &TypeDatabase,
-    ) {
+    pub(crate) fn add_needs(&mut self, additions: Vec<AdditionalNeed>) {
         for need in additions {
             match need {
                 AdditionalNeed::MakeStringConstructor => self.generate_string_constructor(),
                 AdditionalNeed::FunctionWrapper(by_value_wrapper) => {
-                    self.generate_by_value_wrapper(*by_value_wrapper, type_database)
+                    self.generate_by_value_wrapper(*by_value_wrapper)
                 }
                 AdditionalNeed::CTypeTypedef(tn) => self.generate_ctype_typedef(tn),
                 AdditionalNeed::ConcreteTemplatedTypeTypedef(tn, def) => {
-                    self.generate_typedef(tn, type_database.type_to_cpp(&def))
+                    self.generate_typedef(tn, type_to_cpp(&def))
                 }
             }
         }
@@ -166,11 +162,7 @@ impl AdditionalCppGenerator {
         })
     }
 
-    fn generate_by_value_wrapper(
-        &mut self,
-        details: FunctionWrapper,
-        type_database: &TypeDatabase,
-    ) {
+    fn generate_by_value_wrapper(&mut self, details: FunctionWrapper) {
         // Even if the original function call is in a namespace,
         // we generate this wrapper in the global namespace.
         // We could easily do this the other way round, and when
@@ -198,24 +190,18 @@ impl AdditionalCppGenerator {
             .argument_conversion
             .iter()
             .enumerate()
-            .map(|(counter, ty)| {
-                format!(
-                    "{} {}",
-                    ty.unconverted_type(type_database),
-                    get_arg_name(counter)
-                )
-            })
+            .map(|(counter, ty)| format!("{} {}", ty.unconverted_type(), get_arg_name(counter)))
             .join(", ");
         let ret_type = details
             .return_conversion
             .as_ref()
-            .map_or("void".to_string(), |x| x.converted_type(type_database));
+            .map_or("void".to_string(), |x| x.converted_type());
         let declaration = format!("{} {}({})", ret_type, name, args);
         let mut arg_list = details
             .argument_conversion
             .iter()
             .enumerate()
-            .map(|(counter, conv)| conv.conversion(&get_arg_name(counter), type_database));
+            .map(|(counter, conv)| conv.conversion(&get_arg_name(counter)));
         let receiver = if is_a_method { arg_list.next() } else { None };
         let arg_list = arg_list.join(", ");
         let mut underlying_function_call = match details.payload {
@@ -241,10 +227,8 @@ impl AdditionalCppGenerator {
             }
         };
         if let Some(ret) = details.return_conversion {
-            underlying_function_call = format!(
-                "return {}",
-                ret.conversion(&underlying_function_call, type_database)
-            );
+            underlying_function_call =
+                format!("return {}", ret.conversion(&underlying_function_call));
         };
         let definition = format!("{} {{ {}; }}", declaration, underlying_function_call,);
         let declaration = format!("{};", declaration);

engine/src/builder.rs

@@ -12,7 +12,8 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-use crate::Error as EngineError;
+use proc_macro2::TokenStream;
+
 use crate::{ParseError, ParsedFile};
 use std::io::Write;
 use std::path::{Path, PathBuf};
@@ -32,9 +33,6 @@ pub enum BuilderError {
     FileWriteFail(std::io::Error, PathBuf),
     /// No `include_cxx` macro was found anywhere.
     NoIncludeCxxMacrosFound,
-    /// Problem converting the `AUTOCXX_INC` environment variable
-    /// to a set of canonical paths.
-    IncludeDirProblem(EngineError),
     /// Unable to create one of the directories to which we need to write
     UnableToCreateDirectory(std::io::Error, PathBuf),
 }
@@ -46,14 +44,15 @@ impl Display for BuilderError {
             BuilderError::InvalidCxx(ee) => write!(f, "cxx was unable to understand the code generated by autocxx (likely a bug in autocxx; please report.) {}", ee)?,
             BuilderError::FileWriteFail(ee, pb) => write!(f, "Unable to write to {}: {}", pb.to_string_lossy(), ee)?,
             BuilderError::NoIncludeCxxMacrosFound => write!(f, "No include_cpp! macro found")?,
-            BuilderError::IncludeDirProblem(ee) =>  write!(f, "Unable to set up C++ include direcories as requested: {}", ee)?,
             BuilderError::UnableToCreateDirectory(ee, pb) => write!(f, "Unable to create directory {}: {}", pb.to_string_lossy(), ee)?,
         }
         Ok(())
     }
 }
 
-pub type BuilderSuccess = cc::Build;
+pub type BuilderBuild = cc::Build;
+
+pub struct BuilderSuccess(pub BuilderBuild, pub Vec<PathBuf>);
 
 /// Results of a build.
 pub type BuilderResult = Result<BuilderSuccess, BuilderError>;
@@ -73,7 +72,7 @@ where
 
 /// Builds successfully, or exits the process displaying a suitable
 /// message.
-pub fn expect_build<P1, I, T>(rs_file: P1, autocxx_incs: I) -> cc::Build
+pub fn expect_build<P1, I, T>(rs_file: P1, autocxx_incs: I) -> BuilderSuccess
 where
     P1: AsRef<Path>,
     I: IntoIterator<Item = T>,
@@ -106,6 +105,8 @@ where
     ensure_created(&incdir)?;
     let cxxdir = gendir.join("cxx");
     ensure_created(&cxxdir)?;
+    let rsdir = gendir.join("rs");
+    ensure_created(&rsdir)?;
     // We are incredibly unsophisticated in our directory arrangement here
     // compared to cxx. I have no doubt that we will need to replicate just
     // about everything cxx does, in due course...
@@ -114,31 +115,29 @@ where
     write_to_file(&incdir, "cxx.h", crate::HEADER.as_bytes())?;
 
     let autocxx_inc = build_autocxx_inc(autocxx_incs, &incdir);
-    // Configure cargo to give the same set of include paths to autocxx
-    // when expanding the macro.
-    println!("cargo:rustc-env=AUTOCXX_INC={}", autocxx_inc);
+    // Configure cargo to tell the procedural macro how to find the
+    // Rust files we may have written out.
+    println!("cargo:rustc-env=AUTOCXX_RS={}", rsdir.to_str().unwrap());
 
-    let mut parsed_file =
-        crate::parse_file(rs_file, Some(&autocxx_inc)).map_err(BuilderError::ParseError)?;
+    let mut parsed_file = crate::parse_file(rs_file).map_err(BuilderError::ParseError)?;
     parsed_file
-        .resolve_all()
+        .resolve_all(&autocxx_inc)
         .map_err(BuilderError::ParseError)?;
-    build_with_existing_parsed_file(parsed_file, cxxdir, incdir)
+    build_with_existing_parsed_file(parsed_file, cxxdir, incdir, rsdir)
 }
 
 pub(crate) fn build_with_existing_parsed_file(
     parsed_file: ParsedFile,
     cxxdir: PathBuf,
     incdir: PathBuf,
-) -> Result<cc::Build, BuilderError> {
+    rsdir: PathBuf,
+) -> BuilderResult {
     let mut counter = 0;
     let mut builder = cc::Build::new();
     builder.cpp(true);
+    let mut generated_rs = Vec::new();
     for include_cpp in parsed_file.get_autocxxes() {
-        for inc_dir in include_cpp
-            .include_dirs()
-            .map_err(BuilderError::IncludeDirProblem)?
-        {
+        for inc_dir in include_cpp.include_dirs() {
             builder.include(inc_dir);
         }
         let generated_code = include_cpp
@@ -151,12 +150,15 @@ pub(crate) fn build_with_existing_parsed_file(
             builder.file(gen_cxx_path);
 
             write_to_file(&incdir, &filepair.header_name, &filepair.header)?;
+            let fname = include_cpp.get_rs_filename();
+            let rs = include_cpp.generate_rs();
+            generated_rs.push(write_rs_to_file(&rsdir, &fname, rs)?);
         }
     }
     if counter == 0 {
         Err(BuilderError::NoIncludeCxxMacrosFound)
     } else {
-        Ok(builder)
+        Ok(BuilderSuccess(builder, generated_rs))
     }
 }
 
@@ -195,3 +197,11 @@ fn try_write_to_file(path: &PathBuf, content: &[u8]) -> std::io::Result<()> {
     let mut f = File::create(path)?;
     f.write_all(content)
 }
+
+fn write_rs_to_file(
+    dir: &PathBuf,
+    filename: &str,
+    content: TokenStream,
+) -> Result<PathBuf, BuilderError> {
+    write_to_file(dir, filename, content.to_string().as_bytes())
+}

engine/src/byvalue_checker.rs

@@ -177,7 +177,7 @@ impl ByValueChecker {
 #[cfg(test)]
 mod tests {
     use super::ByValueChecker;
-    use crate::{types::Namespace, TypeName};
+    use crate::types::{Namespace, TypeName};
     use syn::{parse_quote, ItemStruct};
 
     #[test]

engine/src/byvalue_scanner.rs

@@ -12,10 +12,11 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
+use autocxx_parser::TypeDatabase;
 use syn::Item;
 
 use crate::{
-    byvalue_checker::ByValueChecker, conversion::ConvertError, type_database::TypeDatabase,
+    byvalue_checker::ByValueChecker, conversion::ConvertError,
     typedef_analyzer::analyze_typedef_target, typedef_analyzer::TypedefTarget, types::Namespace,
     types::TypeName,
 };
@@ -75,14 +76,14 @@ impl<'a> ByValueScanner<'a> {
 
     fn find_nested_pod_types(&mut self, items: &[Item]) -> Result<(), ConvertError> {
         self.find_nested_pod_types_in_mod(items, &Namespace::new())?;
+        let pod_requests = self
+            .type_database
+            .get_pod_requests()
+            .iter()
+            .map(|ty| TypeName::new_from_user_input(ty))
+            .collect();
         self.byvalue_checker
-            .satisfy_requests(
-                self.type_database
-                    .get_pod_requests()
-                    .iter()
-                    .cloned()
-                    .collect(),
-            )
+            .satisfy_requests(pod_requests)
             .map_err(ConvertError::UnsafePODType)
     }
 }

engine/src/conversion/gc.rs

@@ -14,7 +14,9 @@
 
 use std::collections::{HashMap, HashSet};
 
-use crate::{type_database::TypeDatabase, types::TypeName};
+use autocxx_parser::TypeDatabase;
+
+use crate::types::TypeName;
 
 use super::api::Api;
 
@@ -43,7 +45,7 @@ pub(crate) fn filter_apis_by_following_edges_from_allowlist(
         .iter()
         .filter(|api| {
             let tnforal = api.typename_for_allowlist();
-            type_database.is_on_allowlist(&tnforal)
+            type_database.is_on_allowlist(&tnforal.to_cpp_name())
         })
         .map(Api::typename)
         .collect();

engine/src/conversion/mod.rs

@@ -20,11 +20,10 @@ mod parse;
 mod utilities;
 
 pub(crate) use api::ConvertError;
+use autocxx_parser::TypeDatabase;
 use syn::{Item, ItemMod};
 
-use crate::{
-    byvalue_scanner::identify_byvalue_safe_types, type_database::TypeDatabase, UnsafePolicy,
-};
+use crate::{byvalue_scanner::identify_byvalue_safe_types, UnsafePolicy};
 
 use self::{
     codegen::{CodeGenerator, CodegenResults},