Skip to content

Support type aliases across bridges using different C++ namespaces #298

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 11 commits into from
Closed
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion gen/src/file.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,6 @@ fn parse_args(attr: &Attribute) -> Result<Namespace> {
if attr.tokens.is_empty() {
Ok(Namespace::none())
} else {
attr.parse_args()
attr.parse_args_with(Namespace::parse_bridge_attr_namespace)
}
}
15 changes: 14 additions & 1 deletion gen/src/write.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,9 @@ use crate::gen::{include, Opt};
use crate::syntax::atom::Atom::{self, *};
use crate::syntax::namespace::Namespace;
use crate::syntax::symbol::Symbol;
use crate::syntax::{mangle, Api, Enum, ExternFn, ExternType, Signature, Struct, Type, Types, Var};
use crate::syntax::{
mangle, Api, Enum, ExternFn, ExternType, Signature, Struct, Type, TypeAlias, Types, Var,
};
use proc_macro2::Ident;
use std::collections::HashMap;

Expand Down Expand Up @@ -42,6 +44,7 @@ pub(super) fn gen(
Api::Struct(strct) => write_struct_decl(out, &strct.ident),
Api::CxxType(ety) => write_struct_using(out, &ety.ident),
Api::RustType(ety) => write_struct_decl(out, &ety.ident),
Api::TypeAlias(alias) => write_alias(out, alias),
_ => {}
}
}
Expand Down Expand Up @@ -931,6 +934,16 @@ fn write_type(out: &mut OutFile, ty: &Type) {
}
}

fn write_alias(out: &mut OutFile, alias: &TypeAlias) {
if let Some(namespace) = &alias.namespace {
// Review TODO: Is this unwrap fine? i.e. is it ok to assume that, if
// the TypePath parsed, that it has at least one segment?
let remote_type = &alias.ty.path.segments.last().unwrap().ident;
let path = namespace.path_for_type(remote_type);
writeln!(out, "using {} = {};", alias.ident, path)
}
}

fn write_atom(out: &mut OutFile, atom: Atom) {
match atom {
Bool => write!(out, "bool"),
Expand Down
16 changes: 8 additions & 8 deletions macro/src/expand.rs
Original file line number Diff line number Diff line change
Expand Up @@ -657,16 +657,22 @@ fn expand_rust_function_shim_impl(
}

fn expand_type_alias(alias: &TypeAlias) -> TokenStream {
let doc = &alias.doc;
let ident = &alias.ident;
let ty = &alias.ty;
quote! {
#doc
pub type #ident = #ty;
}
}

fn expand_type_alias_verify(namespace: &Namespace, alias: &TypeAlias) -> TokenStream {
let namespace = alias.namespace.as_ref().unwrap_or(namespace);
// Review TODO: Is this unwrap fine? i.e. is it ok to assume that, if the
// TypePath parsed, that it has at least one segment?
let remote_type = &alias.ty.path.segments.last().unwrap().ident;
let type_id = type_id(namespace, remote_type);
let ident = &alias.ident;
let type_id = type_id(namespace, ident);
let begin_span = alias.type_token.span;
let end_span = alias.semi_token.span;
let begin = quote_spanned!(begin_span=> ::cxx::private::verify_extern_type::<);
Expand All @@ -678,13 +684,7 @@ fn expand_type_alias_verify(namespace: &Namespace, alias: &TypeAlias) -> TokenSt
}

fn type_id(namespace: &Namespace, ident: &Ident) -> TokenStream {
let mut path = String::new();
for name in namespace {
path += &name.to_string();
path += "::";
}
path += &ident.to_string();

let path = namespace.path_for_type(ident);
quote! {
::cxx::type_id!(#path)
}
Expand Down
7 changes: 5 additions & 2 deletions macro/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ use crate::syntax::file::Module;
use crate::syntax::namespace::Namespace;
use crate::syntax::qualified::QualifiedName;
use proc_macro::TokenStream;
use syn::parse::{Parse, ParseStream, Result};
use syn::parse::{Parse, ParseStream, Parser, Result};
use syn::parse_macro_input;

/// `#[cxx::bridge] mod ffi { ... }`
Expand All @@ -41,7 +41,10 @@ use syn::parse_macro_input;
pub fn bridge(args: TokenStream, input: TokenStream) -> TokenStream {
let _ = syntax::error::ERRORS;

let namespace = parse_macro_input!(args as Namespace);
let namespace = match Namespace::parse_bridge_attr_namespace.parse(args) {
Ok(ns) => ns,
Err(err) => return err.to_compile_error().into(),
};
let mut ffi = parse_macro_input!(input as Module);
ffi.namespace = namespace;

Expand Down
18 changes: 17 additions & 1 deletion syntax/attrs.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use crate::syntax::report::Errors;
use crate::syntax::Atom::{self, *};
use crate::syntax::{Derive, Doc};
use crate::syntax::{Derive, Doc, Namespace};
use proc_macro2::Ident;
use syn::parse::{ParseStream, Parser as _};
use syn::{Attribute, Error, LitStr, Path, Result, Token};
Expand All @@ -10,6 +10,7 @@ pub struct Parser<'a> {
pub doc: Option<&'a mut Doc>,
pub derives: Option<&'a mut Vec<Derive>>,
pub repr: Option<&'a mut Option<Atom>>,
pub namespace: Option<&'a mut Option<Namespace>>,
}

pub(super) fn parse_doc(cx: &mut Errors, attrs: &[Attribute]) -> Doc {
Expand Down Expand Up @@ -57,6 +58,16 @@ pub(super) fn parse(cx: &mut Errors, attrs: &[Attribute], mut parser: Parser) {
}
Err(err) => return cx.push(err),
}
} else if attr.path.is_ident("namespace") {
match parse_namespace_attribute.parse2(attr.tokens.clone()) {
Ok(namespace) => {
if let Some(ns) = &mut parser.namespace {
**ns = Some(Namespace::from(namespace));
continue;
}
}
Err(err) => return cx.push(err),
}
}
return cx.error(attr, "unsupported attribute");
}
Expand Down Expand Up @@ -99,3 +110,8 @@ fn parse_repr_attribute(input: ParseStream) -> Result<Atom> {
"unrecognized repr",
))
}

fn parse_namespace_attribute(input: ParseStream) -> Result<Namespace> {
input.parse::<Token![=]>()?;
input.parse::<Namespace>()
}
7 changes: 5 additions & 2 deletions syntax/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,12 @@ use self::parse::kw;
use proc_macro2::{Ident, Span};
use syn::punctuated::Punctuated;
use syn::token::{Brace, Bracket, Paren};
use syn::{Expr, Lifetime, Token, Type as RustType};
use syn::{Expr, Lifetime, Token, TypePath};

pub use self::atom::Atom;
pub use self::derive::Derive;
pub use self::doc::Doc;
pub use self::namespace::Namespace;
pub use self::parse::parse_items;
pub use self::types::Types;

Expand Down Expand Up @@ -79,10 +80,12 @@ pub struct ExternFn {
}

pub struct TypeAlias {
pub doc: Doc,
pub namespace: Option<Namespace>,
pub type_token: Token![type],
pub ident: Ident,
pub eq_token: Token![=],
pub ty: RustType,
pub ty: TypePath,
pub semi_token: Token![;],
}

Expand Down
28 changes: 19 additions & 9 deletions syntax/namespace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,19 +24,29 @@ impl Namespace {
pub fn iter(&self) -> Iter<Ident> {
self.segments.iter()
}

pub fn path_for_type(&self, ident: &Ident) -> String {
let mut segments = self.iter().map(ToString::to_string).collect::<Vec<_>>();
segments.push(ident.to_string());
segments.join("::")
}

pub fn parse_bridge_attr_namespace(input: ParseStream) -> Result<Namespace> {
if input.is_empty() {
return Ok(Namespace::none());
}

input.parse::<kw::namespace>()?;
input.parse::<Token![=]>()?;
let ns = input.parse::<Namespace>()?;
input.parse::<Option<Token![,]>>()?;
Ok(ns)
}
}

impl Parse for Namespace {
fn parse(input: ParseStream) -> Result<Self> {
let mut segments = Vec::new();
if !input.is_empty() {
input.parse::<kw::namespace>()?;
input.parse::<Token![=]>()?;
segments = input
.call(QualifiedName::parse_quoted_or_unquoted)?
.segments;
input.parse::<Option<Token![,]>>()?;
}
let segments = QualifiedName::parse_quoted_or_unquoted(input)?.segments;
Ok(Namespace { segments })
}
}
Expand Down
17 changes: 15 additions & 2 deletions syntax/parse.rs
Original file line number Diff line number Diff line change
Expand Up @@ -394,11 +394,24 @@ fn parse_extern_verbatim(cx: &mut Errors, tokens: &TokenStream, lang: Lang) -> R
};
let ident: Ident = input.parse()?;
let eq_token: Token![=] = input.parse()?;
let ty: RustType = input.parse()?;
let ty: TypePath = input.parse()?;
let semi_token: Token![;] = input.parse()?;
attrs::parse_doc(cx, &attrs);

let mut doc = Doc::new();
let mut namespace = None;
attrs::parse(
cx,
&attrs,
attrs::Parser {
doc: Some(&mut doc),
namespace: Some(&mut namespace),
..Default::default()
},
);

Ok(TypeAlias {
doc,
namespace,
type_token,
ident,
eq_token,
Expand Down
10 changes: 5 additions & 5 deletions tests/BUCK
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@ rust_test(
rust_library(
name = "ffi",
srcs = [
"ffi/alias.rs",
"ffi/lib.rs",
"ffi/module.rs",
],
crate = "cxx_test_suite",
deps = [
Expand All @@ -21,8 +21,8 @@ cxx_library(
name = "impl",
srcs = [
"ffi/tests.cc",
":gen-alias-source",
":gen-lib-source",
":gen-module-source",
],
header_namespace = "cxx-test-suite",
headers = {
Expand All @@ -47,8 +47,8 @@ genrule(
)

genrule(
name = "gen-module-source",
srcs = ["ffi/module.rs"],
out = "module.rs.cc",
name = "gen-alias-source",
srcs = ["ffi/alias.rs"],
out = "alias.rs.cc",
cmd = "$(exe //:codegen) ${SRCS} > ${OUT}",
)
10 changes: 5 additions & 5 deletions tests/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@ rust_test(
rust_library(
name = "cxx_test_suite",
srcs = [
"ffi/alias.rs",
"ffi/lib.rs",
"ffi/module.rs",
],
deps = [
":impl",
Expand All @@ -22,8 +22,8 @@ cc_library(
name = "impl",
srcs = [
"ffi/tests.cc",
":gen-alias-source",
":gen-lib-source",
":gen-module-source",
],
hdrs = ["ffi/tests.h"],
include_prefix = "cxx-test-suite",
Expand Down Expand Up @@ -57,9 +57,9 @@ cc_library(
)

genrule(
name = "gen-module-source",
srcs = ["ffi/module.rs"],
outs = ["module.rs.cc"],
name = "gen-alias-source",
srcs = ["ffi/alias.rs"],
outs = ["alias.rs.cc"],
cmd = "$(location //:codegen) $< > $@",
tools = ["//:codegen"],
)
20 changes: 20 additions & 0 deletions tests/ffi/alias.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
// Rustfmt mangles the extern type alias.
// https://github.com/rust-lang/rustfmt/issues/4159
#[rustfmt::skip]
#[cxx::bridge(namespace = alias_tests)]
pub mod ffi {
extern "C" {
include!("cxx-test-suite/tests.h");

// Review TODO: Unquoted namespace here doesn't work, is that expected or a bug
// in my parsing?
#[namespace = "tests"]
type C = crate::ffi::C;

#[namespace = "tests"]
type SameC = crate::ffi::C;

fn c_return_unique_ptr() -> UniquePtr<C>;
fn c_take_unique_ptr(c: UniquePtr<SameC>);
}
}
2 changes: 1 addition & 1 deletion tests/ffi/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ fn main() {
return;
}

let sources = vec!["lib.rs", "module.rs"];
let sources = vec!["alias.rs", "lib.rs"];
cxx_build::bridges(sources)
.file("tests.cc")
.flag_if_supported(cxxbridge_flags::STD)
Expand Down
3 changes: 2 additions & 1 deletion tests/ffi/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
clippy::trivially_copy_pass_by_ref
)]

pub mod module;
pub mod alias;

use cxx::{CxxString, UniquePtr};
use std::fmt::{self, Display};
Expand Down Expand Up @@ -60,6 +60,7 @@ pub mod ffi {
fn c_take_str(s: &str);
fn c_take_sliceu8(s: &[u8]);
fn c_take_rust_string(s: String);
fn c_take_unique_ptr(c: UniquePtr<C>);
fn c_take_unique_ptr_string(s: UniquePtr<CxxString>);
fn c_take_unique_ptr_vector_u8(v: UniquePtr<CxxVector<u8>>);
fn c_take_unique_ptr_vector_f64(v: UniquePtr<CxxVector<f64>>);
Expand Down
13 changes: 0 additions & 13 deletions tests/ffi/module.rs

This file was deleted.

9 changes: 9 additions & 0 deletions tests/ffi/tests.h
Original file line number Diff line number Diff line change
Expand Up @@ -102,3 +102,12 @@ rust::Vec<rust::String> c_try_return_rust_vec_string();
const rust::Vec<uint8_t> &c_try_return_ref_rust_vec(const C &c);

} // namespace tests

namespace alias_tests {

// These aliases on the C++ side aren't under test, there's just no reason to
// duplicate these functions
using tests::c_return_unique_ptr;
using tests::c_take_unique_ptr;

} // namespace alias_tests
Loading