Repository: dtolnay/cxx (language: rust)

Issue:
Add ability to set symbol visibility
TL;DR:

I've needed to hack my generated `.cc` files to change:

```cpp
extern "C" {
void base$cxxbridge03$log_string(::rust::Str::Repr msg) noexcept {
  void (*log_string$)(::rust::Str) = log_string;
  log_string$(msg);
}
} // extern "C"
```

to

```cpp
extern "C" {
__attribute__((visibility("default"))) void base$cxxbridge03$log_string(::rust::Str::Repr msg) noexcept {
  void (*log_string$)(::rust::Str) = log_string;
  log_string$(msg);
}
```

I would like to add the ability to do that, possibly with an additional parameter to the `cxx::bridge` macro alongside the existing `namespace` parameter.

Here's why.

Our final build products are:
* C++ library `libservices.so`. This C++ depends upon APIs exported from...
* C++ library `libbase.so`

Each contains a blob of Rust code:
* Rust rlib `librust_services.rlib`
* Rust rlib `librust_base.rlib`

`librust_services.rlib` uses APIs exported publicly by `librust_base.rlib`. Specifically, the code in `librust_services.rlib` uses a `log()` API within `librust_base.rlib`, which in turn uses native code within the rest of `libbase.so`.

The only officially approved way to make these C++ binaries is by generating two Rust `staticlib`s:
* Rust staticlib `libservices_rust_deps.a`
* Rust staticlib `libbase_rust_deps.a`
and then linking them into the .so files with all the rest of the C++ code.

So we hope to achieve this call stack:
* Rust code in `librust_services.rlib` (which gets built into `libservices.so`). It calls
* Rust code in `librust_base.rlib`, which calls
* Native code elsewhere in `libbase.so`

So, where should the code in `librust_base.rlib` end up? Specifically this `log()` Rust API?

Ideally, that Rust API would _only_ be in `libbase.so` and so the Rust code within `libservices.so` would call other Rust code in `libbase.so`. But as far as I know, that's impossible. Rust doesn't provide a way to mark Rust code as exported from a `staticlib` - there is no equivalent to the C directive `__attribute__((visibility("default")))`. (Again, as far as I understand it) Rust will only attempt to keep C-facing APIs marked with `#[no_mangle]` directive; there's no way to ask that a given Rust API is exported from a `.so` which is downstream of a `staticlib`. (Maybe this merits an issue filed against rust - I'd be interested in opinions - this conflation of exporting vs naming seems to cause trouble now and again, e.g. https://github.com/rust-lang/rust/issues/54135#issuecomment-607935953).

Given that we can't export the Rust `log()` API from `libbase.so`, we need to build the Rust code into `libservices.so`. But now the Rust code has to depend upon the C++ implementations, which means we need to export them from `libbase.so`. Hence - the need for the generated C++ code to export the symbols from `libbase.so`.

Would you accept a PR to add this exporting ability?


Discussion / hints:
Thanks for the detailed explanation. I would accept a PR for a c++ code generator command line flag or an attribute (`#[cxx::bridge(namespace = base, dso_export)]`?) or whatever you think is more appropriate. Or is it something that would be reasonable to emit unconditionally?
Thanks! I've decided to poke the bear of the underlying issue first, and we'll see how that goes, but I fully expect a PR to land on your virtual desk at some point here.

Fix patch:
diff --git a/gen/cmd/src/main.rs b/gen/cmd/src/main.rs
index a20179f26..701f77fd9 100644
--- a/gen/cmd/src/main.rs
+++ b/gen/cmd/src/main.rs
@@ -35,6 +35,15 @@ struct Opt {
     #[structopt(long)]
     header: bool,
 
+    /// Optional annotation for implementations of C++ function
+    /// wrappers that may be exposed to Rust. You may for example
+    /// need to provide __declspec(dllexport) or
+    /// __attribute__((visibility("default"))) if Rust code from
+    /// one shared object or executable depends on these C++ functions
+    /// in another.
+    #[structopt(long)]
+    cxx_impl_annotations: Option<String>,
+
     /// Any additional headers to #include
     #[structopt(short, long)]
     include: Vec<String>,
@@ -49,6 +58,7 @@ fn main() {
 
     let gen = gen::Opt {
         include: opt.include,
+        cxx_impl_annotations: opt.cxx_impl_annotations,
     };
 
     match (opt.input, opt.header) {
diff --git a/gen/src/mod.rs b/gen/src/mod.rs
index b9d35d70d..77a0c6580 100644
--- a/gen/src/mod.rs
+++ b/gen/src/mod.rs
@@ -24,38 +24,81 @@ struct Input {
 pub(super) struct Opt {
     /// Any additional headers to #include
     pub include: Vec<String>,
+    /// Whether to set __attribute__((visibility("default")))
+    /// or similar annotations on function implementations.
+    pub cxx_impl_annotations: Option<String>,
 }
 
 pub(super) fn do_generate_bridge(path: &Path, opt: Opt) -> Vec<u8> {
     let header = false;
-    generate(path, opt, header)
+    generate_from_path(path, opt, header)
 }
 
 pub(super) fn do_generate_header(path: &Path, opt: Opt) -> Vec<u8> {
     let header = true;
-    generate(path, opt, header)
+    generate_from_path(path, opt, header)
 }
 
-fn generate(path: &Path, opt: Opt, header: bool) -> Vec<u8> {
+fn generate_from_path(path: &Path, opt: Opt, header: bool) -> Vec<u8> {
     let source = match fs::read_to_string(path) {
         Ok(source) => source,
         Err(err) => format_err(path, "", Error::Io(err)),
     };
-    match (|| -> Result<_> {
-        proc_macro2::fallback::force();
-        let ref mut errors = Errors::new();
-        let syntax = syn::parse_file(&source)?;
-        let bridge = find::find_bridge_mod(syntax)?;
-        let ref namespace = bridge.namespace;
-        let ref apis = syntax::parse_items(errors, bridge.module);
-        let ref types = Types::collect(errors, apis);
-        errors.propagate()?;
-        check::typecheck(errors, namespace, apis, types);
-        errors.propagate()?;
-        let out = write::gen(namespace, apis, types, opt, header);
-        Ok(out)
-    })() {
-        Ok(out) => out.content(),
+    match generate(&source, opt, header) {
+        Ok(out) => out,
         Err(err) => format_err(path, &source, err),
     }
 }
+
+fn generate(source: &str, opt: Opt, header: bool) -> Result<Vec<u8>> {
+    proc_macro2::fallback::force();
+    let ref mut errors = Errors::new();
+    let syntax = syn::parse_file(&source)?;
+    let bridge = find::find_bridge_mod(syntax)?;
+    let ref namespace = bridge.namespace;
+    let ref apis = syntax::parse_items(errors, bridge.module);
+    let ref types = Types::collect(errors, apis);
+    errors.propagate()?;
+    check::typecheck(errors, namespace, apis, types);
+    errors.propagate()?;
+    let out = write::gen(namespace, apis, types, opt, header);
+    Ok(out.content())
+}
+
+#[cfg(test)]
+mod tests {
+    use crate::gen::{generate, Opt};
+
+    const CPP_EXAMPLE: &'static str = r#"
+        #[cxx::bridge]
+        mod ffi {
+            extern "C" {
+                pub fn do_cpp_thing(foo: &str);
+            }
+        }
+        "#;
+
+    #[test]
+    fn test_cpp() {
+        let opts = Opt {
+            include: Vec::new(),
+            cxx_impl_annotations: None,
+        };
+        let output = generate(CPP_EXAMPLE, opts, false).unwrap();
+        let output = std::str::from_utf8(&output).unwrap();
+        // To avoid continual breakage we won't test every byte.
+        // Let's look for the major features.
+        assert!(output.contains("void cxxbridge03$do_cpp_thing(::rust::Str::Repr foo)"));
+    }
+
+    #[test]
+    fn test_annotation() {
+        let opts = Opt {
+            include: Vec::new(),
+            cxx_impl_annotations: Some("ANNOTATION".to_string()),
+        };
+        let output = generate(CPP_EXAMPLE, opts, false).unwrap();
+        let output = std::str::from_utf8(&output).unwrap();
+        assert!(output.contains("ANNOTATION void cxxbridge03$do_cpp_thing(::rust::Str::Repr foo)"));
+    }
+}
diff --git a/gen/src/write.rs b/gen/src/write.rs
index 46b6151c1..04d8e3e2a 100644
--- a/gen/src/write.rs
+++ b/gen/src/write.rs
@@ -86,13 +86,13 @@ pub(super) fn gen(
         out.begin_block("extern \"C\"");
         write_exception_glue(out, apis);
         for api in apis {
-            let (efn, write): (_, fn(_, _, _)) = match api {
+            let (efn, write): (_, fn(_, _, _, _)) = match api {
                 Api::CxxFunction(efn) => (efn, write_cxx_function_shim),
                 Api::RustFunction(efn) => (efn, write_rust_function_decl),
                 _ => continue,
             };
             out.next_section();
-            write(out, efn, types);
+            write(out, efn, types, &opt.cxx_impl_annotations);
         }
         out.end_block("extern \"C\"");
     }
@@ -399,7 +399,17 @@ fn write_exception_glue(out: &mut OutFile, apis: &[Api]) {
     }
 }
 
-fn write_cxx_function_shim(out: &mut OutFile, efn: &ExternFn, types: &Types) {
+fn write_cxx_function_shim(
+    out: &mut OutFile,
+    efn: &ExternFn,
+    types: &Types,
+    impl_annotations: &Option<String>,
+) {
+    if !out.header {
+        if let Some(annotation) = impl_annotations {
+            write!(out, "{} ", annotation);
+        }
+    }
     if efn.throws {
         write!(out, "::rust::Str::Repr ");
     } else {
@@ -560,7 +570,7 @@ fn write_function_pointer_trampoline(
     write_rust_function_shim_impl(out, &c_trampoline, f, types, &r_trampoline, indirect_call);
 }
 
-fn write_rust_function_decl(out: &mut OutFile, efn: &ExternFn, types: &Types) {
+fn write_rust_function_decl(out: &mut OutFile, efn: &ExternFn, types: &Types, _: &Option<String>) {
     let link_name = mangle::extern_fn(&out.namespace, efn);
     let indirect_call = false;
     write_rust_function_decl_impl(out, &link_name, efn, types, indirect_call);


Test patch: