Remove AUTOCXX_INC

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.
-
-The same code can be passed through tools that generate .cc and .h bindings too:
+* Thirdly, runs `cxx::bridge` to create the C++ bindings.
+* Fourthly, writes out a `.rs` file.
 
-* 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
 
@@ -116,14 +113,6 @@ and transparently generated at build time by the `include_cpp!` procedural macro
 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.)
-
 # Directory structure
 
 * `demo` - a demo example

engine/src/builder.rs

@@ -14,7 +14,6 @@
 
 use proc_macro2::TokenStream;
 
-use crate::Error as EngineError;
 use crate::{ParseError, ParsedFile};
 use std::io::Write;
 use std::path::{Path, PathBuf};
@@ -34,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),
 }
@@ -48,7 +44,6 @@ 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(())
@@ -118,15 +113,13 @@ 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, rsdir)
 }
@@ -142,10 +135,7 @@ pub(crate) fn build_with_existing_parsed_file(
     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

engine/src/integration_tests.rs

@@ -162,7 +162,6 @@ enum TestError {
     AutoCxx(crate::BuilderError),
     CppBuild(cc::Error),
     RsBuild,
-    IncludeCpp(crate::ParseError),
 }
 
 fn do_run_test(

engine/src/lib.rs

@@ -108,7 +108,7 @@ impl Display for Error {
         match self {
             Error::Bindgen(_) => write!(f, "Bindgen was unable to generate the initial .rs bindings for this file. This may indicate a parsing problem with the C++ headers.")?,
             Error::Parsing(err) => write!(f, "The Rust file could not be parsede: {}", err)?,
-            Error::NoAutoCxxInc => write!(f, "No C++ include directory was provided. Consider setting AUTOCXX_INC.")?,
+            Error::NoAutoCxxInc => write!(f, "No C++ include directory was provided.")?,
             Error::CouldNotCanoncalizeIncludeDir(pb) => write!(f, "One of the C++ include directories provided ({}) did not appear to exist or could otherwise not be made into a canonical path.", pb.to_string_lossy())?,
             Error::Conversion(err) => write!(f, "autocxx could not generate the requested bindings. {}", err)?,
             Error::NoGenerationRequested => write!(f, "No 'generate' or 'generate_pod' directives were found, so we would not generate any Rust bindings despite the inclusion of C++ headers.")?,
@@ -119,12 +119,16 @@ impl Display for Error {
 
 pub type Result<T, E = Error> = std::result::Result<T, E>;
 
-#[allow(clippy::large_enum_variant)] // because this is only used once
+struct GenerationResults {
+    item_mod: ItemMod,
+    additional_cpp_generator: AdditionalCppGenerator,
+    inc_dirs: Vec<PathBuf>,
+}
 enum State {
     NotGenerated,
     ParseOnly,
     NothingGenerated,
-    Generated(ItemMod, AdditionalCppGenerator),
+    Generated(Box<GenerationResults>),
 }
 
 /// Core of the autocxx engine. See `generate` for most details
@@ -136,7 +140,6 @@ enum State {
 pub struct IncludeCpp {
     config: IncludeCppConfig,
     state: State,
-    preconfigured_inc_dirs: Option<std::ffi::OsString>,
 }
 
 impl Parse for IncludeCpp {
@@ -147,11 +150,7 @@ impl Parse for IncludeCpp {
         } else {
             State::NotGenerated
         };
-        Ok(Self {
-            config,
-            state,
-            preconfigured_inc_dirs: None,
-        })
+        Ok(Self { config, state })
     }
 }
 
@@ -160,10 +159,6 @@ impl IncludeCpp {
         mac.parse_body::<IncludeCpp>().map_err(Error::Parsing)
     }
 
-    pub fn set_include_dirs<P: AsRef<std::ffi::OsStr>>(&mut self, include_dirs: P) {
-        self.preconfigured_inc_dirs = Some(include_dirs.as_ref().into());
-    }
-
     fn build_header(&self) -> String {
         join(
             self.config.inclusions.iter().map(|incl| match incl {
@@ -174,26 +169,7 @@ impl IncludeCpp {
         )
     }
 
-    fn determine_incdirs(&self) -> Result<Vec<PathBuf>> {
-        let inc_dirs = match &self.preconfigured_inc_dirs {
-            Some(d) => d.clone(),
-            None => std::env::var_os("AUTOCXX_INC").ok_or(Error::NoAutoCxxInc)?,
-        };
-        let inc_dirs = std::env::split_paths(&inc_dirs);
-        // TODO consider if we can or should look up the include path automatically
-        // instead of requiring callers always to set AUTOCXX_INC.
-
-        // On Windows, the canonical path begins with a UNC prefix that cannot be passed to
-        // the MSVC compiler, so dunce::canonicalize() is used instead of std::fs::canonicalize()
-        // See:
-        // * https://github.com/dtolnay/cxx/pull/41
-        // * https://github.com/alexcrichton/cc-rs/issues/169
-        inc_dirs
-            .map(|p| dunce::canonicalize(&p).map_err(|_| Error::CouldNotCanoncalizeIncludeDir(p)))
-            .collect()
-    }
-
-    fn make_bindgen_builder(&self) -> Result<bindgen::Builder> {
+    fn make_bindgen_builder(&self, inc_dirs: &[PathBuf]) -> bindgen::Builder {
         // TODO support different C++ versions
         let mut builder = bindgen::builder()
             .clang_args(&["-x", "c++", "-std=c++14"])
@@ -210,7 +186,7 @@ impl IncludeCpp {
             builder = builder.blacklist_item(item);
         }
 
-        for inc_dir in self.determine_incdirs()? {
+        for inc_dir in inc_dirs {
             // TODO work with OsStrs here to avoid the .display()
             builder = builder.clang_arg(format!("-I{}", inc_dir.display()));
         }
@@ -225,7 +201,7 @@ impl IncludeCpp {
                 .whitelist_var(a);
         }
 
-        Ok(builder)
+        builder
     }
 
     fn inject_header_into_bindgen(&self, mut builder: bindgen::Builder) -> bindgen::Builder {
@@ -254,11 +230,18 @@ impl IncludeCpp {
                     include!(#fname);
                 }
             }
-            State::Generated(itemmod, _) => itemmod.to_token_stream(),
+            State::Generated(gen_results) => gen_results.item_mod.to_token_stream(),
             State::NothingGenerated | State::ParseOnly => TokenStream2::new(),
         }
     }
 
+    pub fn include_dirs(&self) -> &Vec<PathBuf> {
+        match &self.state {
+            State::Generated(gen_results) => &gen_results.inc_dirs,
+            _ => panic!("Must call generate() before incdirs()"),
+        }
+    }
+
     fn parse_bindings(&self, bindings: bindgen::Bindings) -> Result<ItemMod> {
         // This bindings object is actually a TokenStream internally and we're wasting
         // effort converting to and from string. We could enhance the bindgen API
@@ -296,21 +279,35 @@ impl IncludeCpp {
     /// Along the way, the `bridge_converter` might tell us of additional
     /// C++ code which we should generate, e.g. wrappers to move things
     /// into and out of `UniquePtr`s.
-    pub fn generate(&mut self) -> Result<()> {
+    pub fn generate(&mut self, inc_dirs: &str) -> Result<()> {
+        // On Windows, the canonical path begins with a UNC prefix that cannot be passed to
+        // the MSVC compiler, so dunce::canonicalize() is used instead of std::fs::canonicalize()
+        // See:
+        // * https://github.com/dtolnay/cxx/pull/41
+        // * https://github.com/alexcrichton/cc-rs/issues/169
+        let inc_dirs: Vec<_> = std::env::split_paths(inc_dirs)
+            .map(|p| dunce::canonicalize(&p).map_err(|_| Error::CouldNotCanoncalizeIncludeDir(p)))
+            .collect();
+        let mut inc_dirs2 = Vec::new();
+        for x in inc_dirs {
+            let x = x?;
+            inc_dirs2.push(x); // TODO all yuck
+        }
+
         // If we are in parse only mode, do nothing. This is used for
         // doc tests to ensure the parsing is valid, but we can't expect
         // valid C++ header files or linkers to allow a complete build.
         match self.state {
             State::ParseOnly => return Ok(()),
             State::NotGenerated => {}
-            State::Generated(_, _) | State::NothingGenerated => panic!("Only call generate once"),
+            State::Generated(_) | State::NothingGenerated => panic!("Only call generate once"),
         }
 
         if self.config.type_database.allowlist_is_empty() {
             return Err(Error::NoGenerationRequested);
         }
 
-        let builder = self.make_bindgen_builder()?;
+        let builder = self.make_bindgen_builder(&inc_dirs2);
         let bindings = self
             .inject_header_into_bindgen(builder)
             .generate()
@@ -344,7 +341,11 @@ impl IncludeCpp {
             "New bindings:\n{}",
             rust_pretty_printer::pretty_print(&new_bindings.to_token_stream())
         );
-        self.state = State::Generated(new_bindings, additional_cpp_generator);
+        self.state = State::Generated(Box::new(GenerationResults {
+            item_mod: new_bindings,
+            additional_cpp_generator,
+            inc_dirs: inc_dirs2,
+        }));
         Ok(())
     }
 
@@ -355,8 +356,8 @@ impl IncludeCpp {
             State::ParseOnly => panic!("Cannot generate C++ in parse-only mode"),
             State::NotGenerated => panic!("Call generate() first"),
             State::NothingGenerated => {}
-            State::Generated(itemmod, additional_cpp_generator) => {
-                let rs = itemmod.into_token_stream();
+            State::Generated(gen_results) => {
+                let rs = gen_results.item_mod.to_token_stream();
                 let opt = cxx_gen::Opt::default();
                 let cxx_generated = cxx_gen::generate_header_and_cc(rs, &opt)?;
                 files.push(CppFilePair {
@@ -365,7 +366,7 @@ impl IncludeCpp {
                     implementation: cxx_generated.implementation,
                 });
 
-                match additional_cpp_generator.generate() {
+                match gen_results.additional_cpp_generator.generate() {
                     None => {}
                     Some(additional_cpp) => {
                         // TODO should probably replace pragma once below with traditional include guards.
@@ -383,11 +384,6 @@ impl IncludeCpp {
         };
         Ok(GeneratedCpp(files))
     }
-
-    /// Get the configured include directories.
-    pub fn include_dirs(&self) -> Result<Vec<PathBuf>> {
-        self.determine_incdirs()
-    }
 }
 
 #[cfg(test)]

engine/src/parse.rs

@@ -49,39 +49,27 @@ impl Display for ParseError {
 }
 
 /// Parse a Rust file, and spot any include_cpp macros within it.
-pub fn parse_file<P1: AsRef<Path>>(
-    rs_file: P1,
-    autocxx_inc: Option<&str>,
-) -> Result<ParsedFile, ParseError> {
+pub fn parse_file<P1: AsRef<Path>>(rs_file: P1) -> Result<ParsedFile, ParseError> {
     let mut source = String::new();
     let mut file = std::fs::File::open(rs_file).map_err(ParseError::FileOpen)?;
     file.read_to_string(&mut source)
         .map_err(ParseError::FileRead)?;
     let source = syn::parse_file(&source).map_err(ParseError::Syntax)?;
-    parse_file_contents(source, autocxx_inc)
+    parse_file_contents(source)
 }
 
-pub fn parse_token_stream(
-    ts: TokenStream,
-    autocxx_inc: Option<&str>,
-) -> Result<ParsedFile, ParseError> {
+pub fn parse_token_stream(ts: TokenStream) -> Result<ParsedFile, ParseError> {
     let file = syn::parse2(ts).map_err(ParseError::Syntax)?;
-    parse_file_contents(file, autocxx_inc)
+    parse_file_contents(file)
 }
 
-fn parse_file_contents(
-    source: syn::File,
-    autocxx_inc: Option<&str>,
-) -> Result<ParsedFile, ParseError> {
+fn parse_file_contents(source: syn::File) -> Result<ParsedFile, ParseError> {
     let mut results = Vec::new();
     for item in source.items {
         if let Item::Macro(ref mac) = item {
             if mac.mac.path.is_ident("include_cpp") {
-                let mut include_cpp = crate::IncludeCpp::new_from_syn(mac.mac.clone())
+                let include_cpp = crate::IncludeCpp::new_from_syn(mac.mac.clone())
                     .map_err(ParseError::MacroParseFail)?;
-                if let Some(autocxx_inc) = autocxx_inc {
-                    include_cpp.set_include_dirs(autocxx_inc);
-                }
                 results.push(Segment::Autocxx(include_cpp));
                 continue;
             }
@@ -97,6 +85,7 @@ fn parse_file_contents(
 /// the rest of the Rust code, such that it can be reconstituted if necessary.
 pub struct ParsedFile(Vec<Segment>);
 
+#[allow(clippy::large_enum_variant)]
 enum Segment {
     Autocxx(IncludeCpp),
     Other(Item),
@@ -124,9 +113,11 @@ impl ParsedFile {
             .collect()
     }
 
-    pub fn resolve_all(&mut self) -> Result<(), ParseError> {
+    pub fn resolve_all(&mut self, autocxx_inc: &str) -> Result<(), ParseError> {
         for include_cpp in self.get_autocxxes_mut() {
-            include_cpp.generate().map_err(ParseError::MacroParseFail)?
+            include_cpp
+                .generate(autocxx_inc)
+                .map_err(ParseError::MacroParseFail)?
         }
         Ok(())
     }