Repository: google/autocxx (rust)

Issue:
Nicer syntax for `include_cpp`
The current syntax:

```rust
include_cpp!(Header("bob.h"), Allow("get_foo"), Allow("Bar"))
```

has four problems:
* It's ugly.
* It doesn't allow the name of the `ffi` mod to be specified.
* It doesn't allow specification of whether the `ffi` mod is `pub`, etc.
* There's no good way of attaching documentation to directives like `Allow`, etc.

I am thinking of switching to a syntax a bit more like `cxx`:

```rust
#[autocxx::bridge]
pub mod ffi {
   include!("bob.h") # or maybe include_cpp!
   allow!("get_foo")
   allow!("Bar")
}
```

_Hopefully_ I can persuade rustdoc to treat all the inner macros as document-able things. If not, they may become functions (which do nothing when called as actual functions, but can have docs attached). The problem there is that a single crate can either export a macro or functions. We'll see.

I should do this at the same time as #37 to have one major breaking change, though I don't think anyone's relying on the existing syntax yet.

The new syntax isn't perfect, since I wanted the `include_cpp!` line to act as much as possible like `#include` in C++. So thinking still in progress.

Pull request description:
Macro new syntax
Fixes #42. Compatibility break.

Patch:
diff --git a/Cargo.toml b/Cargo.toml
index e4447fa1..9ec28dd7 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -23,19 +23,12 @@ edition = "2018"
 keywords = ["ffi"]
 categories = ["development-tools::ffi", "api-bindings"]
 
-[lib]
-proc-macro = true
-
 [dependencies]
 autocxx-engine = { path="engine", version="0.3.3" }
-proc-macro-error = "1.0"
-
-[dependencies.syn]
-version = "1.0"
-features = [ "full" ]
+autocxx-macro = { path="macro", version="0.3.3" }
 
 [workspace]
-members = ["engine", "gen/cmd", "gen/build", "demo"]
+members = ["engine", "gen/cmd", "gen/build", "macro", "demo"]
 
 # [patch.crates-io]
 # cxx = { path="../cxx" }
diff --git a/demo/src/main.rs b/demo/src/main.rs
index 3593787a..6e897a60 100644
--- a/demo/src/main.rs
+++ b/demo/src/main.rs
@@ -12,9 +12,12 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-use autocxx::include_cxx;
+use autocxx::include_cpp;
 
-include_cxx!(Header("input.h"), Allow("DoMath"),);
+include_cpp! {
+    #include "input.h"
+    allow!("DoMath")
+}
 
 fn main() {
     println!(
diff --git a/engine/Cargo.toml b/engine/Cargo.toml
index bb7fdfca..17186048 100644
--- a/engine/Cargo.toml
+++ b/engine/Cargo.toml
@@ -37,6 +37,7 @@ itertools = "0.9"
 cxx-gen = "0.6.2"
 dunce = "1.0.1"
 cc = { version = "1.0", optional = true }
+cxx = "0.5.5"
 
 [dependencies.syn]
 version = "1.0.39"
@@ -53,4 +54,3 @@ cc = "1.0"
 # by the trybuild test system...
 autocxx = { path=".." }
 link-cplusplus = "1.0"
-cxx = "0.5.5"
diff --git a/engine/src/lib.rs b/engine/src/lib.rs
index 62ce1561..4b4bc21c 100644
--- a/engine/src/lib.rs
+++ b/engine/src/lib.rs
@@ -49,6 +49,13 @@ pub use parse::{parse_file, parse_token_stream, ParseError, ParsedFile};
 
 pub use cxx_gen::HEADER;
 
+/// Re-export cxx such that clients can use the same version as
+/// us. This doesn't enable clients to avoid depending on the cxx
+/// crate too, unfortunately, since generated cxx::bridge code
+/// refers explicitly to ::cxx. See
+/// https://github.com/google/autocxx/issues/36
+pub use cxx;
+
 pub struct CppFilePair {
     pub header: Vec<u8>,
     pub implementation: Vec<u8>,
@@ -130,34 +137,41 @@ impl IncludeCpp {
         let mut exclude_utilities = false;
 
         while !input.is_empty() {
-            let ident: syn::Ident = input.parse()?;
-            if ident == "Header" {
-                let args;
-                syn::parenthesized!(args in input);
-                let hdr: syn::LitStr = args.parse()?;
-                inclusions.push(CppInclusion::Header(hdr.value()));
-            } else if ident == "Allow" || ident == "AllowPOD" {
-                let args;
-                syn::parenthesized!(args in input);
-                let allow: syn::LitStr = args.parse()?;
-                allowlist.push(allow.value());
-                if ident == "AllowPOD" {
-                    pod_types.push(TypeName::new_from_user_input(&allow.value()));
+            if input.parse::<Option<syn::Token![#]>>()?.is_some() {
+                let ident: syn::Ident = input.parse()?;
+                if ident != "include" {
+                    return Err(syn::Error::new(
+                        ident.span(),
+                        "expected include",
+                    ));
                 }
-            } else if ident == "ParseOnly" {
-                parse_only = true;
-            } else if ident == "ExcludeUtilities" {
-                exclude_utilities = true;
+                let hdr: syn::LitStr = input.parse()?;
+                inclusions.push(CppInclusion::Header(hdr.value()));
             } else {
-                return Err(syn::Error::new(
-                    ident.span(),
-                    "expected Header, Allow, AllowPOD or ExcludeUtilities",
-                ));
+                let ident: syn::Ident = input.parse()?;
+                input.parse::<Option<syn::Token![!]>>()?;
+                if ident == "allow" || ident == "allow_pod" {
+                    let args;
+                    syn::parenthesized!(args in input);
+                    let allow: syn::LitStr = args.parse()?;
+                    allowlist.push(allow.value());
+                    if ident == "allow_pod" {
+                        pod_types.push(TypeName::new_from_user_input(&allow.value()));
+                    }
+                } else if ident == "parse_only" {
+                    parse_only = true;
+                } else if ident == "exclude_utilities" {
+                    exclude_utilities = true;
+                } else {
+                    return Err(syn::Error::new(
+                        ident.span(),
+                        "expected allow, allow_pod or exclude_utilities",
+                    ));
+                }
             }
             if input.is_empty() {
                 break;
             }
-            input.parse::<syn::Token![,]>()?;
         }
 
         Ok(IncludeCpp {
diff --git a/engine/src/parse.rs b/engine/src/parse.rs
index ee8f90a1..f1a7882f 100644
--- a/engine/src/parse.rs
+++ b/engine/src/parse.rs
@@ -63,7 +63,7 @@ fn parse_file_contents(
     let mut results = Vec::new();
     for item in source.items {
         if let Item::Macro(ref mac) = item {
-            if mac.mac.path.is_ident("include_cxx") {
+            if mac.mac.path.is_ident("include_cpp") {
                 let mut include_cpp = crate::IncludeCpp::new_from_syn(mac.mac.clone())
                     .map_err(ParseError::MacroParseFail)?;
                 if let Some(autocxx_inc) = autocxx_inc {
diff --git a/macro/Cargo.toml b/macro/Cargo.toml
new file mode 100644
index 00000000..37bebcd6
--- /dev/null
+++ b/macro/Cargo.toml
@@ -0,0 +1,35 @@
+# Copyright 2020 Google LLC
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     https://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+[package]
+name = "autocxx-macro"
+version = "0.3.3"
+authors = ["Adrian Taylor <adetaylor@chromium.org>"]
+license = "MIT OR Apache-2.0"
+description = "Safe autogenerated interop between Rust and C++"
+repository = "https://github.com/google/autocxx"
+edition = "2018"
+keywords = ["ffi"]
+categories = ["development-tools::ffi", "api-bindings"]
+
+[lib]
+proc-macro = true
+
+[dependencies]
+autocxx-engine = { path="../engine", version="0.3.3" }
+proc-macro-error = "1.0"
+
+[dependencies.syn]
+version = "1.0"
+features = [ "full" ]
diff --git a/macro/src/lib.rs b/macro/src/lib.rs
new file mode 100644
index 00000000..48f67488
--- /dev/null
+++ b/macro/src/lib.rs
@@ -0,0 +1,29 @@
+// Copyright 2020 Google LLC
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//    https://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+use autocxx_engine::IncludeCpp;
+use proc_macro::TokenStream;
+use proc_macro_error::{abort_call_site, proc_macro_error};
+use syn::parse_macro_input;
+
+/// Implementation of the `include_cpp` macro. See documentation for `autocxx` crate.
+#[proc_macro_error]
+#[proc_macro]
+pub fn include_cpp_impl(input: TokenStream) -> TokenStream {
+    let mut include_cpp = parse_macro_input!(input as IncludeCpp);
+    match include_cpp.generate() {
+        Ok(_) => TokenStream::from(include_cpp.generate_rs()),
+        Err(e) => abort_call_site!(format!("{:?}", e)),
+    }
+}
diff --git a/publish-all.sh b/publish-all.sh
index 67f4541b..e001ef8a 100755
--- a/publish-all.sh
+++ b/publish-all.sh
@@ -4,13 +4,13 @@ set -e
 
 DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" >/dev/null 2>&1 && pwd )"
 
-DIRS="$DIR/engine $DIR $DIR/gen/build"
+DIRS="$DIR/engine $DIR/macro $DIR $DIR/gen/build"
 
 for CRATE in $DIRS; do
   pushd $CRATE
-  echo "Dry run: $CRATE"
-  cargo publish --dry-run
   echo "Publish: $CRATE"
   cargo publish
   popd
+  sleep 3 # sometimes crates.io takes a moment, and our
+          # crates are interdependent.
 done
diff --git a/src/lib.rs b/src/lib.rs
index 81bed48a..16e57f8e 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -12,10 +12,12 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-use autocxx_engine::IncludeCpp;
-use proc_macro::TokenStream;
-use proc_macro_error::{abort_call_site, proc_macro_error};
-use syn::parse_macro_input;
+
+// The crazy macro_rules magic in this file is thanks to dtolnay@
+// and is a way of attaching rustdoc to each of the possible directives
+// within the include_cpp outer macro. None of the directives actually
+// do anything - all the magic is handled entirely by
+// autocxx_macro::include_cpp_impl.
 
 /// Include some C++ headers in your Rust project.
 ///
@@ -33,11 +35,11 @@ use syn::parse_macro_input;
 ///
 /// Rust code:
 /// ```
-/// # use autocxx::include_cxx;
-/// include_cxx!(
-/// #   ParseOnly,
-///     Header("input.h"),
-///     Allow("do_math"),
+/// # use autocxx_macro::include_cpp_impl as include_cpp;
+/// include_cpp!(
+/// #   parse_only
+///     #include "input.h"
+///     allow("do_math")
 /// );
 ///
 /// # mod ffi { pub mod cxxbridge { pub fn do_math(a: u32) -> u32 { a+3 } } }
@@ -57,30 +59,35 @@ use syn::parse_macro_input;
 ///
 /// # Syntax
 ///
-/// Within the brackets of the `include_cpp!(...)` macro, you should provide
-/// a list of the following:
+/// Within the brackets of the `include_cxx!(...)` macro, you should provide
+/// a list of at least the following:
 ///
-/// * *Header(filename)*: a header filename to parse and include
-/// * *Allow(type or function name)*: a type or function name whose declaration
+/// * `#include "cpp_header.h"`: a header filename to parse and include
+/// * `allow("type_or_function_name")`: a type or function name whose declaration
 ///   should be made available to C++.
-/// * *AllowPOD(type name)*: a struct name whose declaration
-///   should be made available to C++, and whose fields are sufficient simple
-///   that they can allow being passed back and forth by value (POD = 'plain old
-///   data').
+///
+/// Other directives are possible as documented in this crate.
 ///
 /// # How to allow structs
 ///
-/// A C++ struct can be listed under 'Allow' or 'AllowPOD' (or may be implicitly
-/// 'Allowed' because it's a type referenced by something else you've allowed.)
+/// All C++ types can be owned within a [UniquePtr][autocxx_engine::cxx::UniquePtr]
+/// within Rust. To let this be possible, simply pass the names of these
+/// types within [allow] (or just [allow] any function which requires these types).
+///
+/// However, only _some_ C++ `struct`s can be owned _by value_ within Rust. Those
+/// types must be freely byte-copyable, because Rust is free to do that at
+/// any time. If you believe your `struct` meets those criteria, you can
+/// use [allow_pod] instead.
+///
+/// Use [allow] under normal circumstances, but [allow_pod] only for structs
+/// where you absolutely do need to pass them truly by value and have direct field access.
 ///
-/// The current plan is to use 'Allow' under normal circumstances, but
-/// 'AllowPOD' only for structs where you absolutely do need to pass them
-/// truly by value and have direct field access.
-/// Some structs can't be represented as POD, e.g. those containing `std::string`
-/// due to self-referential pointers. We will always handle such things using
-/// `UniquePtr` to an opaque type in Rust, but still allow calling existing C++
-/// APIs which take such things by value - we generate automatic
-/// unwrappers. This won't work in all cases.
+/// This doesn't just make a difference to the generated code for the type;
+/// it also makes a difference to any functions which take or return that type.
+/// If there's a C++ function which takes a struct by value, but that struct
+/// is not declared as POD-safe, then we'll generate wrapper functions to move
+/// that type into and out of [UniquePtr][autocxx_engine::cxx::UniquePtr]s.
+/// 
 ///
 /// # Generated code
 ///
@@ -108,13 +115,73 @@ use syn::parse_macro_input;
 /// will be generated on the Rust side. That means that you can't have two
 /// types or two functions with the same name within different namespaces. This
 /// is a temporary restriction.
+#[macro_export]
+macro_rules! include_cpp {
+    (
+        $(#$include:ident $lit:literal)*
+        $($mac:ident!($($arg:tt)*))*
+    ) => {
+        $($crate::$include!{__docs})*
+        $($crate::$mac!{__docs})*
+        $crate::include_cpp_impl! {
+            $(#include $lit)*
+            $($mac!($($arg)*))*
+        }
+    };
+}
+
+/// Include a C++ header. A directive to be included inside
+/// [include_cpp] - see [include_cpp] for details
+#[macro_export]
+macro_rules! include {
+    ($($tt:tt)*) => { $crate::usage!{$($tt)*} };
+}
 
-#[proc_macro_error]
-#[proc_macro]
-pub fn include_cxx(input: TokenStream) -> TokenStream {
-    let mut include_cpp = parse_macro_input!(input as IncludeCpp);
-    match include_cpp.generate() {
-        Ok(_) => TokenStream::from(include_cpp.generate_rs()),
-        Err(e) => abort_call_site!(format!("{:?}", e)),
-    }
+/// Generate Rust bindings for the given C++ type or function.
+/// A directive to be included inside
+/// [include_cpp] - see [include_cpp] for general information.
+/// See also [allow_pod].
+#[macro_export]
+macro_rules! allow {
+    ($($tt:tt)*) => { $crate::usage!{$($tt)*} };
 }
+
+/// Generate as "plain old data".
+/// Generate Rust bindings for the given C++ type such that
+/// it can be passed and owned by value in Rust. This only works
+/// for C++ types which have trivial move constructors and no
+/// destructor - you'll encounter a compile error otherwise.
+/// If your type doesn't match that description, use [allow]
+/// instead, and own the type using [UniquePtr][autocxx_engine::cxx::UniquePtr].
+/// A directive to be included inside
+/// [include_cpp] - see [include_cpp] for general information.
+#[macro_export]
+macro_rules! allow_pod {
+    ($($tt:tt)*) => { $crate::usage!{$($tt)*} };
+}
+
+/// Skip the normal generation of a `make_string` function
+/// and other utilities which we might generate normally.
+/// A directive to be included inside
+/// [include_cpp] - see [include_cpp] for general information.
+#[macro_export]
+macro_rules! exclude_utilities {
+    ($($tt:tt)*) => { $crate::usage!{$($tt)*} };
+}
+
+#[doc(hidden)]
+#[macro_export]
+macro_rules! usage {
+    (__docs) => {};
+    ($($tt:tt)*) => {
+        compile_error! {r#"usage:  include_cpp! {
+                   #include "path/to/header.h"
+                   allow!(...)
+                   allow_pod!(...)
+               }
+"#}
+    };
+}
+
+#[doc(hidden)]
+pub use autocxx_macro::include_cpp_impl;


Test patch:
diff --git a/engine/src/integration_tests.rs b/engine/src/integration_tests.rs
index 9999234f..ded5d38c 100644
--- a/engine/src/integration_tests.rs
+++ b/engine/src/integration_tests.rs
@@ -14,9 +14,10 @@
 
 use indoc::indoc;
 use log::info;
-use proc_macro2::TokenStream;
+use proc_macro2::{Span, TokenStream};
 use quote::quote;
 use quote::ToTokens;
+use syn::Token;
 use std::fs::File;
 use std::io::Write;
 use std::panic::RefUnwindSafe;
@@ -168,12 +169,12 @@ fn do_run_test(
     // clear how we're getting away with it, but quoting it doesn't work.
     let allowed_funcs = allowed_funcs.iter().map(|s| {
         quote! {
-            Allow(#s),
+            allow!(#s)
         }
     });
     let allowed_pods = allowed_pods.iter().map(|s| {
         quote! {
-            AllowPOD(#s),
+            allow_pod!(#s)
         }
     });
 
@@ -188,14 +189,15 @@ fn do_run_test(
         quote!()
     } else {
         quote!(
-            use autocxx::include_cxx;
+            use autocxx::include_cpp;
         )
     };
+    let hexathorpe = Token![#](Span::call_site());
     let unexpanded_rust = quote! {
         #include_bit
 
-        include_cxx!(
-            Header("input.h"),
+        include_cpp!(
+            #hexathorpe include "input.h"
             #(#allowed_funcs)*
             #(#allowed_pods)*
         );